Skip to content

Conversation

@Robdel12
Copy link
Contributor

@Robdel12 Robdel12 commented Oct 29, 2025

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

  • VizzlyClient: Lightweight HTTP client with auto-discovery
  • XCTest Extensions: Convenient helpers for XCUIApplication, XCUIElement, and XCTestCase
  • Auto-discovery: Searches for .vizzly/server.json in parent directories
  • Environment variables: VIZZLY_SERVER_URL, VIZZLY_BUILD_ID
  • Graceful degradation: Tests pass even if Vizzly is unavailable

Platform Support

  • iOS 13+
  • macOS 10.15+
  • Swift 5.9+

Automatic Metadata

The SDK automatically captures:

  • Platform (iOS/macOS)
  • Device model
  • Screen dimensions and scale
  • Element type (for component screenshots)

Usage

import XCTest
import Vizzly

class MyAppUITests: XCTestCase {
    let app = XCUIApplication()

    func testHomeScreen() {
        app.launch()
        
        // Simple screenshot
        app.vizzlyScreenshot(name: "home-screen")
        
        // With properties
        app.vizzlyScreenshot(
            name: "home-dark",
            properties: ["theme": "dark"],
            threshold: 5
        )
        
        // Element screenshot
        let navbar = app.navigationBars.firstMatch
        navbar.vizzlyScreenshot(name: "navbar")
    }
}

Documentation

  • README.md - Complete API reference
  • QUICKSTART.md - 5-minute getting started guide
  • INTEGRATION.md - Detailed iOS integration with CI/CD examples
  • Example/ExampleUITests.swift - 15+ real-world examples

Architecture

The Swift SDK follows the same lightweight philosophy as the Ruby client:

  • Zero dependencies (Foundation only)
  • Simple HTTP POST to local TDD server
  • Base64 image encoding
  • Synchronous requests (required for test context)
  • Same error handling patterns as other clients

Testing

✅ All 7 unit tests pass
✅ Swift package builds successfully
✅ CI workflow configured for macOS runners

swift build  # ✅ Builds successfully
swift test   # ✅ All tests pass

CI/CD

Added two workflows:

  1. ci.yml - Tests Swift package on multiple Xcode versions when clients/swift/** changes
  2. release-swift-client.yml - Release workflow for publishing new versions

Next Steps

  • Test with a real iOS app
  • Consider CocoaPods support
  • Add to main documentation site
  • Announce in community channels

Checklist

  • Core VizzlyClient implementation
  • XCTest integration extensions
  • Auto-discovery of TDD server
  • Comprehensive documentation
  • Example UI tests
  • Unit tests with 100% pass rate
  • CI/CD workflows
  • License and changelog

Related

Completes the multi-platform client SDK offering:

  • ✅ JavaScript/TypeScript (Vitest, Storybook, Static sites)
  • ✅ Ruby
  • ✅ Swift (this PR)

- 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
@claude
Copy link

claude bot commented Oct 29, 2025

📱 Swift SDK Review

Excellent 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.

✅ Strengths

Code Quality

  • Clean Architecture: The VizzlyClient singleton pattern is appropriate for test context
  • Graceful Error Handling: Excellent use of disable() to prevent test noise after first failure
  • Zero Dependencies: Pure Foundation implementation keeps the SDK lightweight
  • Synchronous HTTP: Smart use of DispatchSemaphore - necessary for XCTest context
  • Auto-Discovery: Robust file system traversal with proper parent directory search
  • Comprehensive Extensions: XCTest integration is intuitive and ergonomic

Documentation

  • Outstanding Documentation: The README, QUICKSTART, and INTEGRATION guides are thorough
  • Real-World Examples: ExampleUITests.swift provides 15+ practical patterns
  • CI/CD Examples: GitHub Actions and Fastlane integration examples are production-ready
  • Troubleshooting Section: Addresses common issues developers will encounter

Testing

  • Good Test Coverage: 7 unit tests cover initialization, configuration, and error states
  • Cross-Platform Tests: Tests work on both iOS and macOS

CI/CD Integration

  • Matrix Testing: Tests against multiple Xcode versions (15.2, 15.4)
  • Path-Based Triggers: Only runs when Swift files change
  • Release Workflow: Automated releases with Claude-generated changelogs

@claude
Copy link

claude bot commented Oct 29, 2025

🔍 Issues & Suggestions

1. 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:

  • ✅ Client initialization
  • ✅ Disabled client behavior
  • Auto-discovery logic (critical path not tested)
  • Error handling paths (network errors, server failures)
  • XCTest extensions (no tests for XCUIApplication.vizzlyScreenshot())
  • Property merging (automatic metadata + user properties)

Suggestion: Add tests for auto-discovery, properties merging, and network error handling.

@claude
Copy link

claude bot commented Oct 29, 2025

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:

  • CocoaPods installation not tested (line 37-39 in README)
  • Fastlane example has incorrect syntax (line 398-402 in README)
  • No mention of minimum Swift version (5.9) in README quickstart

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.

@claude
Copy link

claude bot commented Oct 29, 2025

🎯 Recommendations by Priority

Must Fix Before Merge:

  1. Thread Safety: Add synchronization or document single-thread requirement
  2. Test Coverage: Add auto-discovery tests at minimum

Should Fix Soon:

  1. Timeout Safety: Add timeout to semaphore.wait()
  2. XCTest Extension Tests: Test the public API developers will actually use

Nice to Have:

  1. CI caching for faster builds
  2. Property type safety documentation
  3. README corrections (CocoaPods, Fastlane)

🎉 Overall Assessment

Rating: 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
@blacksmith-sh blacksmith-sh bot deleted a comment from Robdel12 Oct 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants