-
Notifications
You must be signed in to change notification settings - Fork 0
📱 Add Swift SDK for iOS/macOS visual testing #76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Core VizzlyClient with HTTP communication and auto-discovery - XCTest integration extensions for XCUIApplication and XCUIElement - Automatic metadata capture (platform, device, viewport) - iOS 13+ and macOS 10.15+ support - Comprehensive documentation and examples - Unit tests with 100% pass rate
- Add Swift client testing to main CI workflow - Add release workflow for Swift client - Test on macOS with multiple Xcode versions - Integrate with existing path filtering system
📱 Swift SDK ReviewExcellent work on the Swift SDK implementation! This is a well-architected, production-ready addition to the Vizzly client ecosystem. The code follows Swift best practices and maintains consistency with the project's philosophy. ✅ StrengthsCode Quality
Documentation
Testing
CI/CD Integration
|
🔍 Issues & Suggestions1. Thread Safety Concern (Medium Priority)Location: VizzlyClient.swift:102-146 The VizzlyClient singleton is accessed from potentially multiple test threads, but mutable state (isDisabled, hasWarned) lacks synchronization. In parallel test execution, race conditions could cause multiple warning messages, inconsistent disabled state, or crashes from concurrent URLSession access. Suggestion: Add a serial queue for state access or document that the SDK is not thread-safe and should only be called from the main test thread. 2. Memory Leak Potential (Low Priority)Location: VizzlyClient.swift:145 The semaphore.wait() call has no timeout. If the server hangs or network stalls indefinitely, the test thread blocks forever. While the URLSession has a 30s timeout, that may not fire in all edge cases. Suggestion: Add a timeout to semaphore.wait() to prevent indefinite blocking. 3. Property Type Safety (Low Priority)Location: VizzlyClient.swift:48, XCTestExtensions.swift:27-29 Properties are [String: Any], requiring runtime type checking. No compile-time safety for property values - developers could pass non-serializable types. Suggestion: Consider using Codable or limiting to [String: String] for better safety, or add documentation about acceptable types. 4. Missing Tests (Medium Priority)Current test coverage gaps:
Suggestion: Add tests for auto-discovery, properties merging, and network error handling. |
5. CI Workflow Inefficiency (Low Priority)Location: .github/workflows/ci.yml:406-421 The Swift tests job doesn't cache build artifacts, resulting in slower CI builds on every run. Suggestion: Add Xcode build cache using actions/cache to improve CI performance. 6. Release Workflow Security (Medium Priority)Location: .github/workflows/release-swift-client.yml:78-111 The workflow uses Claude Code to generate changelogs, which could potentially be exploited if commit messages contain injection attacks. Suggestion: Add input sanitization or use a more constrained changelog generation approach. Consider using conventional-changelog or similar tools. 7. Documentation Improvements (Low Priority)Minor issues:
8. Base64 Encoding Optimization (Low Priority)Location: VizzlyClient.swift:64 Large screenshots (e.g., iPad Pro full screen) could be 5-10MB. Base64 encoding increases size by ~33%. Not critical since this matches other clients' behavior, but consider documenting the size implications. |
🎯 Recommendations by PriorityMust Fix Before Merge:
Should Fix Soon:
Nice to Have:
🎉 Overall AssessmentRating: 9/10 - This is excellent work! The implementation is clean, follows Swift idioms, and integrates seamlessly with XCTest. The documentation quality is outstanding - among the best I've seen for a client SDK. The only significant concerns are thread safety and test coverage gaps. Once those are addressed, this is production-ready. Architecture Praise: The consistency with other Vizzly clients (Ruby, JS) is impressive. The thin client, smart server philosophy is maintained perfectly. Documentation Praise: The three-tiered documentation (README → QUICKSTART → INTEGRATION) is exactly what developers need. The example tests are comprehensive and practical. Great job! 🚀 |
- Update to Xcode 16.2 and 16.4 (available on macos-latest) - Previously referenced Xcode 15.2/15.4 which aren't installed
Overview
This PR adds a complete Swift SDK for Vizzly, enabling visual regression testing in iOS and macOS UI tests.
What's Included
Core SDK
XCUIApplication,XCUIElement, andXCTestCase.vizzly/server.jsonin parent directoriesVIZZLY_SERVER_URL,VIZZLY_BUILD_IDPlatform Support
Automatic Metadata
The SDK automatically captures:
Usage
Documentation
Architecture
The Swift SDK follows the same lightweight philosophy as the Ruby client:
Testing
✅ All 7 unit tests pass
✅ Swift package builds successfully
✅ CI workflow configured for macOS runners
CI/CD
Added two workflows:
clients/swift/**changesNext Steps
Checklist
Related
Completes the multi-platform client SDK offering: