Swift Testing Code Review

Swift Testing Code Review

Hard gates

Complete in order before recording Swift Testing review findings. Stack with review-verification-protocol for universal review rules.

1. Scope: You have an explicit list of .swift paths under review (or a user-named single file). Pass: Paths captured in working notes or one line: No Swift files in scope — then stop with no findings. 2. Swift Testing surface: For each path you treat as Swift Testing code, confirm import Testing or @Test / #expect / #require / @Suite appears in that file (open or search). Pass: At least one match per critiqued file, or you exclude that file from Swift Testing review with a one-line reason (e.g. XCTest-only). 3. Evidence + protocol: Load review-verification-protocol before asserting any issue. Pass: Each finding meets that skill’s anchor rules; any violated Review Checklist item cites [FILE:LINE] evidence. If you report zero issues, state Protocol applied; no Swift Testing issues (or equivalent) in the review summary.

Quick Reference

Review Checklist

• [ ] Expressions embedded directly in #expect (not pre-computed booleans)
• [ ] #require used only for preconditions, #expect for assertions
• [ ] Error tests check specific types (not generic (any Error).self)
• [ ] Parameterized tests with pairs use zip() (not Cartesian product)
• [ ] No logic mirroring implementation in parameterized expected values
• [ ] Async sequences tested with confirmation(expectedCount:)
• [ ] Completion handlers use withCheckedContinuation, not confirmation
• [ ] .serialized applied only where necessary (shared resources)
• [ ] Sibling serialized suites nested under parent if mutually exclusive
• [ ] No assumption of state persistence between @Test functions
• [ ] Disabled tests have explanations and bug links

When to Load References

• Reviewing #expect or #require usage -> expect-macro.md
• Reviewing @Test with arguments or traits -> parameterized.md
• Reviewing confirmation or async testing -> async-testing.md
• Reviewing @Suite or test organization -> organization.md

Review Questions

1. Could pre-computed booleans in #expect lose diagnostic context? 2. Is #require stopping tests prematurely instead of revealing all failures? 3. Are multi-argument parameterized tests creating accidental Cartesian products? 4. Could zip() silently drop test cases due to unequal array lengths? 5. Are completion handlers incorrectly tested with confirmation?

Async Testing

Critical Anti-Patterns

1. Using confirmation for Completion Handlers

// BAD - Closure exits before callback fires
@Test func badCallbackTest() async {
    await confirmation { confirm in
        networkService.fetch { _ in
            confirm()  // Never reached in time!
        }
    }
}

// GOOD - Use withCheckedContinuation instead
@Test func goodCallbackTest() async {
    await withCheckedContinuation { continuation in
        networkService.fetch { result in
            #expect(result.isSuccess)
            continuation.resume()
        }
    }
}

2. Unsafe Counter Variables

// BAD - Counter variable causes Swift 6 concurrency error
@Test func badStreamTest() async {
    var count = 0  // Unsafe in concurrent context
    for await _ in generator {
        count += 1
    }
    #expect(count == 10)
}

// GOOD - Thread-safe confirmation
@Test func goodStreamTest() async {
    await confirmation(expectedCount: 10) { confirm in
        for await _ in generator {
            confirm()
        }
    }
}

3. Tasks Execute Immediately Assumption

// BAD - Task hasn't executed yet
@Test func badTaskTest() {
    sut.refreshData()  // Creates Task internally
    #expect(mockRepo.loadCallCount == 1)  // Still 0!
}

// GOOD - Wait for task completion
@Test func goodTaskTest() async {
    mockRepo.stubResponse = .success([])
    let exp = expectation(description: #function)
    mockRepo.didLoad = { exp.fulfill() }
    sut.refreshData()
    await fulfillment(of: [exp], timeout: 1)
    #expect(mockRepo.loadCallCount == 1)
}

4. Using sleep() in Tests

// BAD - Slow, flaky, arbitrary timing
@Test func badSleepTest() async throws {
    startLongOperation()
    try await Task.sleep(for: .seconds(2))  // Arbitrary delay
    #expect(operationCompleted)
}

// GOOD - Use proper async/await or confirmations
@Test func goodAsyncTest() async throws {
    let result = try await performOperation()
    #expect(result.isSuccess)
}

5. Blocking with DispatchSemaphore/DispatchGroup

// BAD - Risk of deadlock, especially on main thread
@Test func badBlockingTest() async {
    let semaphore = DispatchSemaphore(value: 0)
    Task {
        await asyncOperation()
        semaphore.signal()
    }
    semaphore.wait()  // Deadlock risk!
}

// GOOD - Use structured async/await
@Test func goodAsyncTest() async {
    await asyncOperation()
    #expect(result.isSuccess)
}

confirmation API Usage

The confirmation API verifies callbacks/events occur a specific number of times.

// Default: expects exactly one confirmation
await confirmation { confirm in
    for await event in eventStream {
        confirm()
    }
}

// Custom count
await confirmation(expectedCount: 10) { confirm in
    for await _ in generator {
        confirm()
    }
}

// Verify something never happens
await confirmation(expectedCount: 0) { confirm in
    // If confirm() is called, test fails
}

Critical: All confirm() calls MUST execute before the confirmation closure returns (eager evaluation). Unlike XCTest's XCTestExpectation with fulfillment(of:timeout:), confirmations do not suspend waiting for future events.

For completion-handler APIs:

• Option A: Convert to async/await and await the async work inside the confirmation closure
• Option B: Use withCheckedContinuation (shown in Anti-Pattern #1 above)
• Option C: For callback-style tests that need waiting, use XCTest's fulfillment(of:timeout:) instead

Time Limits

// Test-level time limit
@Test(.timeLimit(.minutes(1)))
func loadNames() async {
    let viewModel = ViewModel()
    await viewModel.loadNames()
    #expect(viewModel.names.isEmpty == false)
}

// Suite-level (shorter of suite/test wins)
@Suite(.timeLimit(.minutes(2)))
struct NetworkTests {
    @Test(.timeLimit(.minutes(1)))  // 1 minute wins
    func fastTest() async { }
}

Best Practices

• Use async/await directly when available
• Use `confirmation` for async sequences and streams
• Use `withCheckedContinuation` for completion handler APIs
• Store Task references for testing unstructured concurrency
• Use `withKnownIssue` for flaky tests
• Use `.timeLimit` trait for tests with external dependencies

Review Questions

1. Are completion handlers being tested with withCheckedContinuation, not confirmation? 2. Are async sequences tested with confirmation(expectedCount:)? 3. Are mutable counters in concurrent contexts replaced with confirmations? 4. Are unstructured Tasks being awaited before assertions? 5. Is sleep() being used instead of proper async patterns?

#expect Macro

Critical Anti-Patterns

1. Computing Booleans Outside #expect

// BAD - Loses expression capture and diagnostic context
let passed = user.age >= 18 && user.hasVerifiedEmail
#expect(passed)
// Failure shows: Expectation failed: passed

// GOOD - Full expression capture
#expect(user.age >= 18, "User must be adult")
#expect(user.hasVerifiedEmail, "Email verification required")
// Failure shows: (user.age → 16) >= 18

2. Overusing #require Instead of #expect

// BAD - Stops at first failure, hides other issues
@Test func testUserProfile() throws {
    let user = try #require(fetchUser())
    try #require(user.name == "Alice")  // Stops here if fails
    try #require(user.isActive)          // Never checked
}

// GOOD - #require only for preconditions, #expect for assertions
@Test func testUserProfile() throws {
    let user = try #require(fetchUser())  // Required to proceed
    #expect(user.name == "Alice")  // Soft check - continues
    #expect(user.isActive)          // Also checked
}

3. Mixing XCTest and Swift Testing

// BAD - Frameworks incompatible
@Test func testMixedFrameworks() {
    XCTAssertEqual(value, expected)  // WRONG
    #expect(otherValue == expected)
}

// GOOD - Use one framework per test
@Test func testWithSwiftTesting() {
    #expect(value == expected)
    #expect(otherValue == expected)
}

4. Generic Error Testing

// BAD - Overly generic, masks specific failures
#expect(throws: (any Error).self) { try validate(input) }

// GOOD - Specific error case
#expect(throws: ValidationError.invalidFormat) { try validate(input) }

// GOOD - Custom validation for associated values
#expect(performing: { try validate(input) }, throws: { error in
    guard let validationError = error as? ValidationError,
          validationError.code == 400 else { return false }
    return true
})

5. Force Unwrap After nil Check

// BAD - Assertion followed by force unwrap
#expect(optionalValue != nil)
let value = optionalValue!

// GOOD - Combine unwrap and assertion
let value = try #require(optionalValue)

Best Practices

• Embed expressions directly in #expect for full diagnostic capture
• Use `#expect` for assertions (soft fail, continues), `#require` for preconditions (hard fail, stops)
• Include descriptive messages when failure reason isn't obvious
• Test specific error cases rather than generic (any Error).self
• Implement `CustomTestStringConvertible` for complex types to improve failure messages

Migration from XCTest

Review Questions

1. Are expressions embedded directly in #expect for full capture? 2. Is #require used only for essential preconditions, not all assertions? 3. Are error tests checking specific error types, not generic (any Error).self? 4. Do complex types implement CustomTestStringConvertible? 5. Are assertion messages provided when failure reason isn't self-evident?

Test Organization

Critical Anti-Patterns

1. Expecting State Persistence Between Tests

// BAD - Each test gets fresh instance, state doesn't persist
@Suite(.serialized)
struct StatefulTests {
    var value = 0

    @Test mutating func step1() { value = 42 }
    @Test func step2() { #expect(value == 42) }  // Fails! Fresh instance
}

// GOOD - Use init() for setup
struct Tests {
    let value: Int
    init() { value = 42 }
    @Test func verify() { #expect(value == 42) }
}

// GOOD - Combine into one test for dependent steps
@Test func completeFlow() {
    var value = 0
    value = 42
    #expect(value == 42)
}

2. Serializing Everything

// BAD - Slow, defeats parallelism
@Suite(.serialized)
struct AllTests {
    // 200 tests that could run in parallel
}

// GOOD - Only serialize what needs it
struct AllTests {
    struct FastTests { }  // Parallel by default
    @Suite(.serialized) struct DatabaseTests { }  // Only these
}

3. Incorrect Nested Serialization

// BAD - Suites run in parallel with each other!
@Suite(.serialized) struct Suite1 { @Test func a() {} }
@Suite(.serialized) struct Suite2 { @Test func b() {} }

// GOOD - Nested under parent
@Suite(.serialized) struct DatabaseSuites {
    @Suite struct Suite1 { @Test func a() {} }
    @Suite struct Suite2 { @Test func b() {} }  // Waits for Suite1
}

4. Using Static State for Sharing

// BAD - Race conditions with parallel tests
struct UnsafeTests {
    static var shared = ""

    @Test func create() { Self.shared = "value" }
    @Test func check() { #expect(Self.shared == "value") }  // Race!
}

// GOOD - Fresh instance per test
final class SafeTests {
    let database: Database
    init() throws { database = try Database(path: UUID().uuidString) }
    deinit { try? database.cleanup() }

    @Test func query() { }  // Own database instance
}

5. Silent Test Skip Without Explanation

// BAD - No explanation why disabled
@Test(.disabled())
func flakyTest() {}

// GOOD - Reason and bug link
@Test(.disabled("Waiting for backend fix"), .bug("PROJ-123"))
func flakyTest() {}

@Suite Fundamentals

Any type containing @Test functions is implicitly a suite. Use explicit @Suite for:

• Display names: @Suite("User Validation Tests")
• Traits: @Suite(.serialized)
• Nested organization

@Suite("Dessert Tests")
struct DessertTests {
    @Suite struct WarmDesserts {
        @Test func applePieCrustLayers() { }
    }
    @Suite struct ColdDesserts {
        @Test func cheesecakeBakingStrategy() { }
    }
}

Supported types: struct (preferred), class (for deinit teardown), actor NOT supported: enum (cannot contain tests directly)

Tags

Declare tags as extensions on Tag:

extension Tag {
    @Tag static var unitTests: Self
    @Tag static var integrationTests: Self
    @Tag static var networking: Self
}

Apply to tests or suites (traits cascade to nested items):

@Suite(.tags(.database))
struct DatabaseTests {
    @Test func testInsert() { }  // Inherits .database
    @Test(.tags(.critical)) func testTransaction() { }  // .database + .critical
}

Parallel Execution

Swift Testing runs all tests in parallel by default. Key implications:

• Each @Test gets its own fresh suite instance
• Global/static state causes race conditions
• Use .serialized only when necessary (shared resources, external services)

Lifecycle

final class DatabaseServiceTests {
    let sut: DatabaseService
    let tempDirectory: URL

    init() throws {  // Setup - runs before EACH test
        self.tempDirectory = FileManager.default.temporaryDirectory
            .appendingPathComponent(UUID().uuidString)
        self.sut = DatabaseService(database: TestDatabase(storageURL: tempDirectory))
    }

    deinit {  // Teardown - runs after EACH test (requires class)
        try? FileManager.default.removeItem(at: tempDirectory)
    }
}

Review Questions

1. Are tests assuming state persists between test functions? 2. Is .serialized applied only where necessary, not everywhere? 3. Are sibling .serialized suites nested under a parent if they must be mutually exclusive? 4. Is static/global state avoided in tests? 5. Do disabled tests have explanations and bug links?

Parameterized Tests

Critical Anti-Patterns

1. Accidental Cartesian Product

// BAD - Creates 25 tests (5x5) when you need 5 pairs
@Test(arguments: [18, 30, 50, 70, 80], [77.0, 73, 65, 61, 55])
func verifyNormalHeartRate(age: Int, bpm: Double) { }

// GOOD - Creates exactly 5 paired tests with zip
@Test(arguments: zip([18, 30, 50, 70, 80], [77.0, 73, 65, 61, 55]))
func verifyNormalHeartRate(age: Int, bpm: Double) { }

2. Logic Mirroring Implementation

// BAD - Test logic mirrors implementation, masks bugs
@Test(arguments: Day.allCases)
func greeting(day: Day) {
    #expect(greeting(of: day) == "Happy \(day.rawValue)!")
}

// GOOD - Explicit expected values
@Test(arguments: [
    (Day.monday, "Happy Monday!"),
    (Day.tuesday, "Happy Tuesday!")
])
func greeting(day: Day, expected: String) {
    #expect(greeting(of: day) == expected)
}

3. Silent Drops with zip()

// BAD - If arrays have different lengths, extras are silently dropped
@Test(arguments: zip(Ingredient.allCases, Dish.allCases))
func cook(_ ingredient: Ingredient, into dish: Dish) { }
// If Ingredient has 5 cases but Dish has 4, one ingredient goes untested!

// GOOD - Explicit array ensures complete coverage
@Test(arguments: [
    (Ingredient.rice, Dish.onigiri),
    (Ingredient.potato, Dish.fries),
    (Ingredient.tomato, Dish.salad)
])
func cook(_ ingredient: Ingredient, into dish: Dish) { }

4. CaseIterable Order Dependency

// BAD - Breaks if enum cases are reordered
@Test(arguments: zip(Status.allCases, ["P", "A", "C"]))
func statusCode(status: Status, code: String) { }

// GOOD - Explicit mapping immune to reordering
@Test(arguments: [
    (Status.pending, "P"),
    (Status.active, "A"),
    (Status.completed, "C")
])
func statusCode(status: Status, code: String) { }

5. For-Loops Instead of Parameterized Tests

// BAD - Stops at first failure, unclear which value failed
@Test func doesNotContainNuts() throws {
    for flavor in [Flavor.vanilla, .chocolate] {
        try #require(!flavor.containsNuts)
    }
}

// GOOD - Each value is independent test case
@Test(arguments: [Flavor.vanilla, .chocolate])
func doesNotContainNuts(flavor: Flavor) throws {
    try #require(!flavor.containsNuts)
}

6. Missing .serialized for Shared Resources

// BAD - Random failures when server limits connections
@Test(arguments: [1, 2, 3, 4, 5])
func uploadFile(id: Int) async {
    await server.upload(fileId: id)
}

// GOOD - Sequential execution for resource-constrained tests
@Test(.serialized, arguments: [1, 2, 3, 4, 5])
func uploadFile(id: Int) async {
    await server.upload(fileId: id)
}

Best Practices

• Use explicit tuple arrays over zip(allCases, allCases) for clarity
• Use `zip()` only when intentionally pairing two sequences
• Prefer `#expect` over #require in parameterized tests to see all failures
• Implement `CustomTestStringConvertible` for readable test names
• Use `.serialized` when tests share limited resources

Available Traits

Review Questions

1. Are multi-argument tests using zip() for pairs, or accidentally creating Cartesian products? 2. Do parameterized tests use explicit expected values, or mirror implementation logic? 3. Could unequal-length zip() silently drop test cases? 4. Are tests that access shared resources using .serialized? 5. Is CustomTestStringConvertible implemented for complex parameter types?