diff --git a/.github/workflows/VerifyChanges.yaml b/.github/workflows/VerifyChanges.yaml index d9a7d72..9ead405 100644 --- a/.github/workflows/VerifyChanges.yaml +++ b/.github/workflows/VerifyChanges.yaml @@ -7,7 +7,7 @@ on: branches: ["main"] env: - XCODE_VERSION: 26.0.1 + XCODE_VERSION: 26.3 jobs: lint: @@ -15,12 +15,11 @@ jobs: runs-on: macos-26 steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@v6 - name: Select Xcode ${{ env.XCODE_VERSION }} - run: sudo xcode-select -s /Applications/Xcode_${{ env.XCODE_VERSION }}.app + run: sudo xcode-select -s /Applications/Xcode_"$XCODE_VERSION".app - name: Lint - run: | - Scripts/lint + run: Scripts/lint build-and-test: name: Build and Test (${{ matrix.platform }}) @@ -30,22 +29,14 @@ jobs: fail-fast: false matrix: include: -# - platform: iOS -# xcode_destination: "platform=iOS Simulator,name=GitHub_Actions_Simulator" -# simulator_device_type: "com.apple.CoreSimulator.SimDeviceType.iPhone-16-Pro" -# simulator_runtime: "com.apple.CoreSimulator.SimRuntime.iOS-26-0" + - platform: iOS + xcode_destination: "platform=iOS Simulator,name=iPhone 17 Pro" - platform: macOS xcode_destination: "platform=macOS,arch=arm64" -# simulator_device_type: "" -# simulator_runtime: "" -# - platform: tvOS -# xcode_destination: "platform=tvOS Simulator,name=GitHub_Actions_Simulator" -# simulator_device_type: "com.apple.CoreSimulator.SimDeviceType.Apple-TV-4K-3rd-generation-4K" -# simulator_runtime: "com.apple.CoreSimulator.SimRuntime.tvOS-26-0" -# - platform: watchOS -# xcode_destination: "platform=watchOS Simulator,name=GitHub_Actions_Simulator" -# simulator_device_type: "com.apple.CoreSimulator.SimDeviceType.Apple-Watch-Series-10-46mm" -# simulator_runtime: "com.apple.CoreSimulator.SimRuntime.watchOS-26-0" + - platform: tvOS + xcode_destination: "platform=tvOS Simulator,name=Apple TV 4K (3rd generation)" + - platform: watchOS + xcode_destination: "platform=watchOS Simulator,name=Apple Watch Series 11 (46mm)" env: DEV_BUILDS: DevBuilds/Sources @@ -58,13 +49,13 @@ jobs: steps: - name: Select Xcode ${{ env.XCODE_VERSION }} - run: sudo xcode-select -s /Applications/Xcode_${{ env.XCODE_VERSION }}.app + run: sudo xcode-select -s /Applications/Xcode_"$XCODE_VERSION".app - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@v6 - name: Checkout DevBuilds - uses: actions/checkout@v4 + uses: actions/checkout@v6 with: repository: DevKitOrganization/DevBuilds path: DevBuilds @@ -72,22 +63,21 @@ jobs: - name: Restore XCTestProducts if: github.event_name != 'push' id: cache-xctestproducts-restore - uses: actions/cache/restore@v4 + uses: actions/cache/restore@v5 with: path: ${{ env.XCODE_TEST_PRODUCTS_PATH }} - key: cache-xctestproducts-${{ github.workflow }}-${{ matrix.platform }}-${{ env.XCODE_VERSION }}-${{ github.sha }} + key: cache-xctestproducts-${{ github.workflow }}-${{ matrix.platform }}-${{ github.sha }} - uses: irgaly/xcode-cache@v1 if: steps.cache-xctestproducts-restore.outputs.cache-hit != 'true' with: - key: xcode-cache-deriveddata-${{ github.workflow }}-${{ matrix.platform }}-${{ env.XCODE_VERSION }}-${{ github.sha }} + key: xcode-cache-deriveddata-${{ env.XCODE_VERSION }}-${{ github.workflow }}-${{ matrix.platform }}-${{ github.sha }} restore-keys: | - xcode-cache-deriveddata-${{ github.workflow }}-${{ matrix.platform }}- - xcode-cache-deriveddata- + xcode-cache-deriveddata-${{ env.XCODE_VERSION }}-${{ github.workflow }}-${{ matrix.platform }}- + xcode-cache-deriveddata-${{ env.XCODE_VERSION }} deriveddata-directory: .build/DerivedData sourcepackages-directory: .build/DerivedData/SourcePackages swiftpm-package-resolved-file: Package.resolved - verbose: true - name: Build for Testing id: build-for-testing @@ -100,8 +90,8 @@ jobs: run: ${{ env.DEV_BUILDS }}/build_and_test.sh --action test-without-building - name: Save XCTestProducts - if: failure() && steps.cache-xctestproducts-restore.outputs.cache-hit != 'true' - uses: actions/cache/save@v4 + if: (cancelled() || failure()) && steps.cache-xctestproducts-restore.outputs.cache-hit != 'true' + uses: actions/cache/save@v5 with: path: ${{ env.XCODE_TEST_PRODUCTS_PATH }} key: ${{ steps.cache-xctestproducts-restore.outputs.cache-primary-key }} @@ -124,7 +114,7 @@ jobs: - name: Upload Logs and XCResults if: success() || failure() - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: Logs_and_XCResults-${{ matrix.platform }} path: | @@ -134,7 +124,7 @@ jobs: - name: Upload xccovPretty output if: github.event_name != 'push' - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: xccovPrettyOutput-${{ matrix.platform }} path: .build/xccovPretty-${{ matrix.platform }}.output @@ -150,7 +140,7 @@ jobs: steps: - name: Download xccovPretty output - uses: actions/download-artifact@v4 + uses: actions/download-artifact@v8 with: name: xccovPrettyOutput-macOS diff --git a/.swift-format b/.swift-format index 84042b2..6e99bc1 100644 --- a/.swift-format +++ b/.swift-format @@ -59,14 +59,14 @@ "ReplaceForEachWithForLoop": true, "ReturnVoidInsteadOfEmptyTuple": true, "TypeNamesShouldBeCapitalized": true, - "UseEarlyExits": true, + "UseEarlyExits": false, "UseExplicitNilCheckInConditions": true, "UseLetInEveryBoundCaseVariable": true, "UseShorthandTypeNames": true, "UseSingleLinePropertyGetter": true, "UseSynthesizedInitializer": true, "UseTripleSlashForDocumentationComments": true, - "UseWhereClausesInForLoops": true, + "UseWhereClausesInForLoops": false, "ValidateDocumentationComments": false }, "spacesAroundRangeFormationOperators": true, diff --git a/CLAUDE.md b/CLAUDE.md index b87a92b..a0cd582 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -26,13 +26,9 @@ The repository uses GitHub Actions for CI/CD with the workflow in `.github/workflows/VerifyChanges.yaml`. The workflow: - Lints code on PRs using `swift format` - - Builds and tests on macOS only (iOS, tvOS, and watchOS builds disabled due to poor - stability and performance in GitHub Actions) + - Builds and tests on iOS, macOS, tvOS, and watchOS - Generates code coverage reports using xccovPretty - - Requires Xcode 16.4 and macOS 15 runners - -**Note**: For comprehensive cross-platform testing, use `Scripts/test-all-platforms` locally -or the pre-push git hook which runs all platform tests before pushing. + - Uses Xcode 26.3 and macOS 26 runners ## Architecture Overview @@ -214,6 +210,36 @@ Follow the project's Markdown Style Guide: - **Terminology**: Use "function" over "method", "type" over "class" +## Code Formatting and Spacing + +The project follows strict spacing conventions for readability and consistency: + + - **2 blank lines between major sections** including: + - Between the last property declaration and first function declaration + - Between all function/computed property implementations at the same scope level + - Between top-level type declarations (class, struct, enum, protocol, extension) + - Before MARK comments that separate major sections + - **1 blank line** for minor separations: + - Between property declarations and nested type definitions + - Between all function definitions in protocols + - After headers in documentation + - After MARK comments that separate major sections + - **File endings**: All Swift files must end with exactly one blank line + + +## Documentation Style + +When writing Markdown documentation, reference `@Documentation/MarkdownStyleGuide.md` to +ensure consistent formatting, structure, and style across all project documentation. Key +standards: + + - **Line Length**: 100 characters maximum + - **Code Blocks**: Use 4-space indentation instead of fenced blocks + - **Lists**: Use `-` for bullets with proper indentation alignment + - **Spacing**: 2 blank lines between major sections, 1 blank line after headers + - **Terminology**: Use "function" over "method", "type" over "class" + + ## Development Notes - Follows Swift API Design Guidelines diff --git a/Documentation/MarkdownStyleGuide.md b/Documentation/MarkdownStyleGuide.md index d94f75c..1f6288b 100644 --- a/Documentation/MarkdownStyleGuide.md +++ b/Documentation/MarkdownStyleGuide.md @@ -1,7 +1,6 @@ # Markdown Style Guide -This document defines the Markdown formatting standards for documentation in the Shopper iOS -codebase. +This document defines the Markdown formatting standards for documentation in this project. ## General Formatting diff --git a/Documentation/TestMocks.md b/Documentation/TestMocks.md index 9d79a46..81e8fe5 100644 --- a/Documentation/TestMocks.md +++ b/Documentation/TestMocks.md @@ -213,8 +213,188 @@ For functions that might not be called in every test, provide default stub value > = .init() } +**CRITICAL - Stubs Accessed by Internal Async Tasks:** -### 3. Protocol Imports with @testable +When testing types that spawn internal `Task`s during initialization, ALL stubs accessed by those +tasks must be initialized in test setup, even if not directly tested. Failure to initialize stubs +will cause crashes when the internal task tries to access them. + +This applies to ANY mock (Observable or not) whose stubs are accessed by background tasks. + +#### Why This is Required: + +When a type spawns internal `Task`s during initialization, those tasks may access properties or +functions on injected dependencies. If those dependencies are mocks with force-unwrapped stubs, and +the stubs haven’t been initialized, the app will crash when the internal task tries to access them. + +**Example crash scenario:** + + // Any mock type (Observable or not) + @MainActor + final class MockDataSource: DataSource { + nonisolated(unsafe) var currentItemsStub: Stub! + var currentItems: [Item] { + currentItemsStub(id) // Crashes if stub is nil + } + } + + // Type spawns internal task during init + init(dataSource: any DataSource) { + self.dataSource = dataSource + Task { + // This accesses dataSource.currentItems, triggering the stub + let items = dataSource.currentItems + } + } + + // Test doesn’t initialize the stub + @Test + func myTest() { + let mockDataSource = MockDataSource() // currentItemsStub is nil + let processor = ItemProcessor(dataSource: mockDataSource) // Crashes in internal task + } + +#### Solution Pattern: Initialize in Test Setup + +Initialize all stubs that internal tasks will access: + + @MainActor + struct ItemProcessorTests: RandomValueGenerating { + var mockDataSource = MockDataSource() + + init() { + // CRITICAL: Initialize ALL stubs that the type’s internal tasks will access + // Even if this particular test doesn’t verify these stubs, they must be non-nil + mockDataSource.fetchItemsStub = ThrowingStub(defaultError: nil) + mockDataSource.currentItemsStub = Stub(defaultReturnValue: []) + } + + @Test + mutating func myTest() async throws { + // Create type that spawns internal tasks using mockDataSource + let processor = ItemProcessor(dataSource: mockDataSource) + // ... test logic ... + } + } + +#### How to Identify Required Stubs: + + 1. Look for `Task { }` blocks in the type’s initializer + 2. Trace what properties/functions those tasks access on dependencies + 3. Initialize stubs for all accessed properties/functions in test `init()` + 4. Run tests — crashes will identify any missed stubs + + +### 3. Prologue and Epilogue Closures for Execution Control + +For mock functions that need precise control over execution timing, use optional prologue and +epilogue closures that execute before and after the stub. + +#### Prologue Pattern + +Prologues execute before the stub is called: + + final class MockNetworkClient: NetworkClient { + nonisolated(unsafe) var sendRequestPrologue: (() async throws -> Void)? + nonisolated(unsafe) var sendRequestStub: ThrowingStub< + URLRequest, + Response, + any Error + >! + + + func sendRequest(_ request: URLRequest) async throws -> Response { + try await sendRequestPrologue?() + return try sendRequestStub(request) + } + } + +#### Epilogue Pattern + +Epilogues execute after the stub is called. Run the epilogue in a `Task` within a `defer` block: + + final class MockEventLogger: EventLogging { + nonisolated(unsafe) var logEventStub: Stub! + nonisolated(unsafe) var logEventEpilogue: (() async throws -> Void)? + + + func logEvent(_ event: some Event) { + defer { + if let epilogue = logEventEpilogue { + Task { try? await epilogue() } + } + } + logEventStub(event) + } + } + +#### Use Cases + +**Prologues** (execute before stub): + + - **Block before processing**: Delay the mock from executing its core behavior + - **Test pre-call state**: Verify conditions before the mock operation begins + - **Insert delays**: Add artificial timing before processing + - **Coordinate setup**: Ensure test conditions are ready before mock executes + +**Epilogues** (execute after stub): + + - **Signal completion**: Notify tests when the mock operation has finished + - **Test post-call state**: Verify conditions after the stub executes but before returning + - **Coordinate with background work**: Wait for fire-and-forget operations to complete + +#### Example: Blocking with AsyncStream + + let (signalStream, signaler) = AsyncStream.makeStream() + mockClient.sendRequestPrologue = { + await signalStream.first(where: { _ in true }) + } + mockClient.sendRequestStub = ThrowingStub(defaultReturnValue: .init()) + + // Start operation that calls the mock + instance.performAction() + + // Verify intermediate state while mock is blocked + await #expect(instance.isProcessing) + + // Signal completion to unblock + signaler.yield() + +#### Example: Signaling Completion with Epilogue + + let eventLogger = MockEventLogger() + eventLogger.logEventStub = Stub() + + let (signalStream, signaler) = AsyncStream.makeStream() + eventLogger.logEventEpilogue = { + signaler.yield() + } + + // Call function that triggers the mock (e.g., via event bus handler) + eventBus.post(SomeEvent()) + + // Wait for mock to complete + await signalStream.first { _ in true } + + // Verify the mock was called + #expect(eventLogger.logEventStub.calls.count == 1) + +#### Example: Adding Delays + + mockClient.sendRequestPrologue = { + try await Task.sleep(for: .milliseconds(100)) + } + +#### Benefits + + - Separates timing control (prologue/epilogue) from return values/errors (stub) + - Enables testing at different execution phases (before/after stub) + - More precise than arbitrary `Task.sleep()` delays in tests + - Eliminates race conditions from timing-based coordination + - Optional — tests can ignore if timing control isn’t needed + + +### 4. Protocol Imports with @testable Import protocols under test with `@testable` when accessing internal details: diff --git a/Documentation/TestingGuidelines.md b/Documentation/TestingGuidelines.md index 81c5877..141ff45 100644 --- a/Documentation/TestingGuidelines.md +++ b/Documentation/TestingGuidelines.md @@ -1,17 +1,18 @@ # Testing Guidelines for Claude Code -This file provides specific guidance for Claude Code when creating, updating, and maintaining -Swift tests. +This file provides specific guidance for Claude Code when creating, updating, and maintaining +tests in this repository. + ## Swift Testing Framework -**IMPORTANT**: This project uses **Swift Testing framework**, NOT XCTest. Do not apply XCTest +**IMPORTANT**: This project uses **Swift Testing framework**, NOT XCTest. Do not apply XCTest patterns or conventions. ### Key Differences from XCTest - **Use `@Test` attribute** instead of function name conventions - - **Use `#expect()` and `#require()`** instead of `XCTAssert*()` functions + - **Use `#expect()` and `#require()`** instead of `XCTAssert*()` functions - **Use `#expect(throws:)`** for testing error conditions instead of `XCTAssertThrows` - **No "test" prefixes** required on function names - **Struct-based test organization** instead of class-based @@ -19,9 +20,9 @@ patterns or conventions. ### Test Naming Conventions - **No "test" prefixes**: Swift Testing doesn't require "test" prefixes for function names - - **Descriptive names**: Use clear, descriptive names like `initialLoadingState()` instead of + - **Descriptive names**: Use clear, descriptive names like `initialLoadingState()` instead of `testInitialLoadingState()` - - **Protocol-specific naming**: For protocols with concrete implementations, name tests after + - **Protocol-specific naming**: For protocols with concrete implementations, name tests after the concrete type (e.g., `StandardAuthenticationRemoteNotificationHandlerTests`) ### Unit Test Structure Conventions @@ -29,9 +30,12 @@ patterns or conventions. Unit tests typically have 3 portions; setup, exercise, and expect. - **Setup**: Create the inputs or mocks necessary to exercise the unit under test - - **Exercise**: Exercise the unit under test, usually by invoking a function using the inputs prepared during "Setup". + - **Exercise**: Exercise the unit under test, usually by invoking a function using the inputs + prepared during "Setup". - **Expect**: Expect one or more results to be true, using Swift Testing expressions. - - More complicated tests may repeat the "exercise" and "expect" steps. + - **IMPORTANT**: Each test should have exactly ONE unit-under-test. Only mark the invocation of + the unit-under-test with an "// exercise" comment. Additional function calls during setup or + verification should use "// set up" or "// expect" comments respectively. - The beginning of each step should be clearly marked with a code comment, like: - // set up the test by preparing a mock authenticator - // exercise the test by initializing the data source @@ -40,19 +44,21 @@ Unit tests typically have 3 portions; setup, exercise, and expect. ### Mock Testing Strategy - - **Focus on verification**: Test that mocks are called correctly, not custom mock + - **Focus on verification**: Test that mocks are called correctly, not custom mock implementations - - **Use standard mocks**: All tests with injectable dependencies should use the same mock + - **Verify parameter passing**: Always check that mocks receive expected arguments, not just + call counts + - **Use standard mocks**: All tests with injectable dependencies should use the same mock types, not custom mock implementations. - - **Always mock dependencies**: Even when not testing the mocked behavior, always use mocks - to supply dependencies to objects under test. - - **Minimal Stubbing**: Only stub functions that are relevant to the code under test, or - required for the test to execute successfully. + - **Always mock dependencies**: Even when not testing the mocked behavior, always use mocks + to supply dependencies to objects under test. + - **Minimal Stubbing**: Only stub functions that are relevant to the code under test, or + required for the test to execute successfully. - Do *NOT* leave comments when stubs are omitted because they are irrelevant. ### ThrowingStub Usage -**CRITICAL**: DevTesting's `ThrowingStub` has very specific initialization patterns that +**CRITICAL**: DevTesting's `ThrowingStub` has very specific initialization patterns that differ from regular `Stub`. Using incorrect initializers will cause compilation errors. #### Correct ThrowingStub Patterns: @@ -60,25 +66,23 @@ differ from regular `Stub`. Using incorrect initializers will cause compilation // For success cases: ThrowingStub(defaultReturnValue: value) - // For error cases: + // For error cases: ThrowingStub(defaultError: error) - // For cases where the value could be success or error: - ThrowingStub(defaultResult: result) - - // For cases where the return type is Void and you don’t want to throw an error: + // For void return types that don't throw: ThrowingStub(defaultError: nil) #### Common Mistakes to Avoid: - - ❌ `ThrowingStub(throwingError: error)` - This doesn't exist + - ❌ `ThrowingStub(throwingError: error)` - This doesn't exist - ❌ `ThrowingStub()` with separate configuration - Must provide default in initializer ### Mock Object Patterns Follow established patterns from `@Documentation/TestMocks.md`: - - **Stub-based architecture**: Use `Stub` and `ThrowingStub` + - **Stub-based architecture**: Use `Stub` and + `ThrowingStub` - **Thread safety**: Mark stub properties with `nonisolated(unsafe)` - **Protocol conformance**: Mock the protocol, not the concrete implementation - **Argument structures**: For complex parameters, create dedicated argument structures @@ -86,23 +90,23 @@ Follow established patterns from `@Documentation/TestMocks.md`: Example mock structure: final class MockProtocolName: ProtocolName { - nonisolated(unsafe) var methodStub: Stub! - - func method(input: InputType) -> OutputType { - methodStub(input) + nonisolated(unsafe) var functionStub: Stub! + + func function(input: InputType) -> OutputType { + functionStub(input) } - - nonisolated(unsafe) var throwingMethodStub: ThrowingStub! - - func throwingMethod(input: InputType) throws -> OutputType { - try throwingMethodStub(input) + + nonisolated(unsafe) var throwingFunctionStub: ThrowingStub! + + func throwingFunction(input: InputType) throws -> OutputType { + try throwingFunctionStub(input) } } ### Random Value Generation with Swift Testing -**IMPORTANT**: Swift Testing uses immutable test structs, but `RandomValueGenerating` requires +**IMPORTANT**: Swift Testing uses immutable test structs, but `RandomValueGenerating` requires `mutating` functions. This creates a specific pattern that must be followed. #### Correct Pattern for Random Value Generation: @@ -110,7 +114,7 @@ Example mock structure: @MainActor struct MyTests: RandomValueGenerating { var randomNumberGenerator = makeRandomNumberGenerator() - + @Test mutating func myTest() throws { let randomValue = randomAlphanumericString() @@ -127,13 +131,12 @@ Example mock structure: #### Dedicated Random Value Extensions: - - **Dedicated files**: Create `RandomValueGenerating+[ModuleName].swift` files for random value + - **Dedicated files**: Create `RandomValueGenerating+[ModuleName].swift` files for random value generation - - **Centralized functions**: Move random value creation functions to these dedicated extension + - **Centralized functions**: Move random value creation functions to these dedicated extension files - - **Consistent patterns**: Follow existing patterns from other modules (e.g., - `RandomValueGenerating+AppPlatform.swift`) - - **Proper imports**: Include necessary `@testable import` statements for modules being + - **Consistent patterns**: Follow existing patterns from other modules + - **Proper imports**: Include necessary `@testable import` statements for modules being extended Example structure: @@ -151,17 +154,18 @@ Example structure: } } + ## File Organization ### Test Files - - **Naming pattern**: `[ClassName]Tests.swift` in corresponding Tests directories + - **Naming pattern**: `[TypeName]Tests.swift` in corresponding Tests directories - **Location**: Place in `Tests/[ModuleName]/[Category]/` directories - - **One test file per class**: Each class should have its own dedicated test file - - **Organize tests by function**: The tests for the function under test should be organized - together, preceded by a `// MARK: - [FunctionName]` comment. + - **One test file per type**: Each type should have its own dedicated test file + - **Organize tests by function**: The tests for the function under test should be organized + together, preceded by a `// MARK: - [FunctionName]` comment. -### Mock Files +### Mock Files - **Naming pattern**: `Mock[ProtocolName].swift` - **Location**: Place in `Tests/[ModuleName]/Testing Support/` directories @@ -169,7 +173,7 @@ Example structure: ### Random Value Extensions - - **Naming pattern**: `RandomValueGenerating+[ModuleName].swift` + - **Naming pattern**: `RandomValueGenerating+[ModuleName].swift` - **Location**: Place in `Tests/[ModuleName]/Testing Support/` directories - **Module-specific**: Create extensions for each module's unique types @@ -179,17 +183,21 @@ Example structure: - **Regular imports**: Use regular imports for testing frameworks and utilities - **Specific imports**: Import only what's needed to keep dependencies clear + ## Test Coverage Guidelines +### Protocols + - **Skip protocol-only tests**: Don't test pure protocols with no implementation + ### Function Coverage - **Each function/getter**: Should have at least one corresponding test function - - **Multiple scenarios**: Create additional test functions to cover edge cases and error + - **Multiple scenarios**: Create additional test functions to cover edge cases and error conditions - **Error paths**: Test both success and failure scenarios for throwing functions ### Error Handling in Tests -**CRITICAL**: Test functions that use `#expect(throws:)` must be marked with `throws`, +**CRITICAL**: Test functions that use `#expect(throws:)` must be marked with `throws`, otherwise you'll get "Errors thrown from here are not handled" compilation errors. #### Correct Pattern: @@ -204,60 +212,226 @@ otherwise you'll get "Errors thrown from here are not handled" compilation error #### Common Mistake: // ❌ This will cause compilation error - @Test + @Test func myTestThatExpectsErrors() { #expect(throws: SomeError.self) { try somethingThatThrows() } } +### Boolean Expressions in Expectations + +Do not compare boolean expressions to `== true` or `== false`. Use the boolean directly or +negate it with `!`. The exception is when the expression is an optional `Bool?`, where the +comparison is needed to unwrap and disambiguate the value. + + // ✅ Good + #expect(instance.isLoading) + #expect(!instance.isLoading) + + // ✅ Good - optional Bool? requires comparison + #expect(instance.optionalFlag == true) + #expect(instance.optionalFlag == false) + + // ❌ Bad - unnecessary comparison + #expect(instance.isLoading == true) + #expect(instance.isLoading == false) + +### Use `#require` Instead of Optional Chaining + +When accessing optional values in tests, use `#require` to unwrap them rather than optional +chaining. This ensures the test fails with a clear message at the point of unwrapping rather +than silently skipping assertions. + + // ✅ Good - fails clearly if nil + let value = try #require(instance.optionalProperty) + #expect(value.name == "expected") + + // ❌ Bad - silently passes if nil + #expect(instance.optionalProperty?.name == "expected") + +### Safe Array Access in Tests + +Never subscript arrays directly based on a prior `#expect` count check. If the expectation +fails, the test continues and the subscript will crash. Use `#require` or a guard to safely +verify bounds before indexing. + + // ✅ Good - test fails safely if count is wrong + #expect(items.count == 2) + let first = try #require(items.first) + #expect(first.name == "expected") + + // ✅ Good - compare mapped arrays to avoid indexing entirely + #expect(items.map(\.name) == ["expected", "other"]) + + // ✅ Good - also safe with explicit bounds check + guard items.count == 2 else { + Issue.record("Expected 2 items, got \(items.count)") + return + } + #expect(items[0].name == "expected") + #expect(items[1].name == "other") + + // ❌ Bad - crashes if count expectation fails + #expect(items.count == 2) + #expect(items[0].name == "expected") + #expect(items[1].name == "other") + +### Unconditional Test Failures + +When a code path should never be reached in a passing test (e.g., a pattern match fails), use +`Issue.record(...)` to record an unconditional failure rather than `#expect(Bool(false), ...)`. +`Issue.record` is the Swift Testing idiomatic API for this, and it produces cleaner failure +output. + +#### Correct Pattern: + + guard case .someCase(let value) = result else { + Issue.record("Expected .someCase, got \(result)") + return + } + #expect(value == expectedValue) + +#### Common Mistake to Avoid: + + // ❌ Avoid - not idiomatic Swift Testing + guard case .someCase(let value) = result else { + #expect(Bool(false), "Expected .someCase, got \(result)") + return + } + +#### Key Points: + + - **Import Testing**: `Issue.record` is part of the `Testing` module — ensure it is imported + - **Provide a descriptive message**: Include what was expected and what was received + - **Use `return` after recording**: Since `Issue.record` does not stop execution, always + return explicitly to prevent further test code from running with invalid state + ### Main Actor Considerations - - **Test isolation**: Mark test structs and methods with `@MainActor` when testing + - **Test isolation**: Mark test structs and functions with `@MainActor` when testing MainActor-isolated code - **Mock conformance**: Ensure mocks properly handle MainActor isolation requirements - **Async testing**: Use proper async/await patterns for testing async code ### Dependency Injection Testing - - **Mock all dependencies**: Create mocks for all injected dependencies to ensure proper + - **Mock all dependencies**: Create mocks for all injected dependencies to ensure proper isolation - **Verify interactions**: Test that dependencies are called with correct parameters - - **State verification**: Check both mock call counts and state changes in the system under + - **State verification**: Check both mock call counts and state changes in the system under test + +## Reducing Repetition in Tests + +### Shared Setup in `init` + +When most tests in a struct need the same mocks or dependencies, create them in the struct's +`init` rather than repeating the setup in every test. This keeps individual tests focused on +what makes them unique. + + @MainActor + struct ItemProcessorTests: RandomValueGenerating { + var randomNumberGenerator = makeRandomNumberGenerator() + + let mockService: MockService + let mockDelegate: MockDelegate + let processor: ItemProcessor + + init() { + mockService = MockService() + mockService.fetchItemsStub = ThrowingStub(defaultReturnValue: []) + + mockDelegate = MockDelegate() + mockDelegate.didUpdateStub = Stub() + + processor = ItemProcessor( + dependencies: .init(service: mockService), + delegate: mockDelegate + ) + } + + @Test + func loadItemsCallsService() async throws { + // exercise the processor — no setup needed, init handled it + await processor.loadItems() + + // expect the service was called + #expect(mockService.fetchItemsStub.calls.count == 1) + } + + @Test + func loadItemsWithCustomData() async throws { + // set up only what differs from the default + let items = [Item(name: "Custom")] + mockService.fetchItemsStub = ThrowingStub(defaultReturnValue: items) + + // exercise + await processor.loadItems() + + // expect + #expect(processor.items.map(\.name) == ["Custom"]) + } + } + +### Helper Functions for Common Test Objects + +When multiple tests construct similar objects with minor variations, extract a helper function. +Keep helpers private to the test file and give them clear names. + + extension ItemProcessorTests { + private func makeItem( + name: String = "Default", + price: Float64 = 9.99 + ) -> Item { + Item(name: name, price: price) + } + } + +### Keep Test-Specific Setup in the Test + +Shared setup should cover the common baseline. Anything unique to a specific test scenario +belongs in that test's "set up" section so readers can see the full context without jumping +to `init` or helpers. + + ## Common Testing Patterns +**IMPORTANT**: Avoid using `Task.sleep()` for test coordination whenever possible. Instead, use +precise synchronization mechanisms like `AsyncStream`, `confirmation`, mock prologues/epilogues, +or returning tasks. Arbitrary delays make tests slower and less reliable. + ### Testing Initialization @Test func initializationSetsCorrectDefaults() { - let instance = ClassUnderTest() - + let instance = SystemUnderTest() + #expect(instance.property == expectedDefault) } ### Testing Dependency Calls - @Test - mutating func methodCallsDependency() { + @Test + mutating func functionCallsDependency() { let mock = MockDependency() - mock.methodStub = Stub() - - let instance = ClassUnderTest(dependency: mock) + mock.functionStub = Stub() + + let instance = SystemUnderTest(dependency: mock) instance.performAction() - - #expect(mock.methodStub.calls.count == 1) + + #expect(mock.functionStub.calls.count == 1) } ### Testing Error Scenarios @Test - mutating func methodThrowsWhenDependencyFails() { + mutating func functionThrowsWhenDependencyFails() { let mock = MockDependency() let error = MockError(description: "Test error") - mock.methodStub = ThrowingStub(defaultError: error) - - let instance = ClassUnderTest(dependency: mock) - + mock.functionStub = ThrowingStub(defaultError: error) + + let instance = SystemUnderTest(dependency: mock) + #expect(throws: MockError.self) { try instance.performAction() } @@ -266,63 +440,278 @@ otherwise you'll get "Errors thrown from here are not handled" compilation error ### Testing Async Operations @Test - mutating func asyncMethodCompletesSuccessfully() async throws { + mutating func asyncFunctionCompletesSuccessfully() async throws { let mock = MockDependency() - mock.asyncMethodStub = Stub(defaultReturnValue: expectedResult) - - let instance = ClassUnderTest(dependency: mock) + mock.asyncFunctionStub = Stub(defaultReturnValue: expectedResult) + + let instance = SystemUnderTest(dependency: mock) let result = await instance.performAsyncAction() - + #expect(result == expectedResult) - #expect(mock.asyncMethodStub.calls.count == 1) + #expect(mock.asyncFunctionStub.calls.count == 1) + } + +### Using Confirmation to Verify Callbacks + +Swift Testing's `confirmation` API ensures that specific code paths execute. Use it to verify +that callbacks, handlers, or closures are invoked: + + @Test + mutating func handlerIsInvoked() async throws { + let text = randomBasicLatinString() + let customID = randomUUID() + + await confirmation { handlerCalled in + let linkedText = RichTextContent.LinkedText(id: customID, text: text) { + defer { handlerCalled() } + return .handled + } + + #expect(linkedText.text == text) + #expect(linkedText.id == customID) + #expect(linkedText.openHandler() == .handled) + } + } + +**Key points:** + + - Call the confirmation callback (e.g., `handlerCalled()`) when the expected code path + executes + - The test will fail if the callback is never invoked + - Use `defer` in handlers to ensure confirmation happens even if early returns occur + +### Testing Synchronous Interfaces with Asynchronous Work + +When testing code with a synchronous interface that performs asynchronous work internally +(e.g., posting to an event bus, dispatching to a queue), use `AsyncStream` to coordinate +test expectations rather than arbitrary sleep durations: + + @Test + mutating func functionPostsEventAsynchronously() async throws { + // set up the test with mocks and event observer + let observer = ContextualBusEventObserver(context: ()) + eventBus.addObserver(observer) + + let instance = SystemUnderTest(eventBus: eventBus) + + // set up signaling mechanism for async completion + let (signalStream, signaler) = AsyncStream.makeStream() + await confirmation("event posted") { confirm in + observer.addHandler(for: SomeEvent.self) { event, _ in + // expect the event has correct data + #expect(event.eventData.value == expectedValue) + confirm() + signaler.yield() + } + + // exercise the test by calling the synchronous function + instance.performSynchronousFunction() + + // wait for the asynchronous work to complete + await signalStream.first { _ in true } + } + } + +#### Key Points for Synchronous Interfaces with Async Work: + + - **Use `AsyncStream.makeStream()`**: Create a signal stream and signaler to coordinate + completion + - **Signal after confirmation**: Call `signaler.yield()` after calling `confirm()` in the + handler + - **Wait for signal**: Use `await signalStream.first { _ in true }` instead of + `Task.sleep(for:)` + - **Avoid arbitrary delays**: This pattern waits precisely for the async work to complete, + making tests faster and more reliable + - **Common use cases**: Event bus posting, background queue dispatch, timer-based operations + +#### Alternative: Returning Tasks for Testability + +When possible, if a function spawns a `Task` and doesn't need to return a value, consider +returning the task itself to simplify testing. Mark the result as `@discardableResult` so +callers can ignore it in production code: + + @discardableResult + func performBackgroundWork() -> Task { + Task { + // Perform async work... + await someAsyncOperation() + } } -### Testing Async State Changes with Confirmations +This allows tests to await the task completion directly without needing stream-based +coordination: + + @Test + mutating func backgroundWorkCompletes() async throws { + let instance = SystemUnderTest() + + // exercise the test and await the returned task + let task = instance.performBackgroundWork() + await task.value + + // expect the work completed successfully + #expect(instance.workCompletedFlag) + } -When testing async state changes that occur through observation (like SwiftUI's `withObservationTracking`), use Swift Testing's `confirmation` API to properly wait for and verify the changes: +**When to use this pattern:** + + - Function spawns a `Task` internally for background work + - Return value is not otherwise needed + - Tests need to verify completion or side effects + +**When to use `AsyncStream` instead:** + + - Cannot modify the function signature (e.g., protocol requirements, public API constraints) + - Function doesn't spawn a task directly (e.g., posts to event bus, uses callbacks) + - Multiple async operations occur that need individual verification + +### Testing Observable Type State Changes + +**PREFERRED PATTERN**: When testing `@Observable` types that update asynchronously through +internal tasks, use the `Observations` helper from DevFoundation. This pattern is simpler and +more reliable than manual observation tracking. + +#### Preferred: Using Observations Helper + +The `Observations` helper directly observes property changes and waits for specific conditions: @Test @MainActor - mutating func stateChangesAsynchronously() async throws { + mutating func propertyUpdatesAsynchronously() async throws { + // set up the test with a task that the type will await + let pendingTask = Task { + return response + } + + // exercise: create instance that spawns internal Task + let instance = SystemUnderTest( + dependencies: dependencies, + pendingTask: pendingTask + ) + + // wait for internal task to process initial state + _ = await Observations({ instance.title }).first { @Sendable _ in true } + + // expect that title was updated by internal task + #expect(instance.title == "Initial Title") + } + +#### Key Points for Observations Pattern: + + - **Use for internal async tasks**: When the type spawns internal `Task`s that update state + - **Observe the property directly**: Pass a closure that reads the property + - **Wait for specific conditions**: Use `.first { condition }` to wait for expected state + - **Mark closure as `@Sendable`**: Required for concurrency safety + - **Simpler than `withObservationTracking`**: No need for manual stream coordination + +#### Alternative: Manual Observation Tracking + +When you need manual control over the observation lifecycle, use `withObservationTracking`: + + @Test @MainActor + mutating func observableStateChangesAsynchronously() async throws { // set up the test by creating the object and mocked dependencies - let instance = ClassUnderTest() + let instance = SystemUnderTest() let mockDependency = MockDependency() instance.dependency = mockDependency - + // set up observation and confirmation for async state change + let (signalStream, signaler) = AsyncStream.makeStream() try await confirmation { stateChanged in withObservationTracking { _ = instance.observableState } onChange: { stateChanged() + signaler.yield() } - + // exercise the test by triggering the state change try instance.performActionThatChangesState() - - // allow time for async state change to occur - try await Task.sleep(for: .seconds(0.5)) + + // await a signal to know the state change occurred + await signalStream.first { @MainActor _ in true } } - + // expect the final state to be correct #expect(instance.observableState == expectedFinalState) } -#### Key Points for Async State Testing: +**When to use `withObservationTracking`:** + + - Need to verify observation behavior itself (not just state changes) + - Need precise control over when observation starts/stops + - Testing custom observation mechanisms + +**Otherwise, prefer `Observations` helper** for simpler, more focused tests. + +### Testing with Mock Prologue and Epilogue Closures + +When testing code that calls mock functions, you can use prologue and epilogue closures to +control execution timing. Prologues execute before the stub, epilogues execute after. See +`@Documentation/TestMocks.md` for the mock implementation patterns. + +#### Pattern: Blocking Mock Execution + + @Test + mutating func testIntermediateStateWhileProcessing() async throws { + // set up the test with a blocking prologue + let (signalStream, signaler) = AsyncStream.makeStream() + mockClient.sendRequestPrologue = { + await signalStream.first(where: { _ in true }) + } + mockClient.sendRequestStub = ThrowingStub(defaultReturnValue: .init()) + + let instance = SystemUnderTest(client: mockClient) + + // exercise the test by triggering the async operation + instance.performAction() + + // expect intermediate state while mock is blocked + await #expect(instance.isProcessing) + + // signal completion to unblock the mock + signaler.yield() + } + +#### Pattern: Signaling Completion with Epilogue + + @Test + mutating func eventHandlerLogsToTelemetry() async throws { + // set up the test with epilogue for coordination + let eventLogger = MockEventLogger() + eventLogger.logEventStub = Stub() + + let (signalStream, signaler) = AsyncStream.makeStream() + eventLogger.logEventEpilogue = { + signaler.yield() + } + + // exercise the test by posting event + eventBus.post(SomeEvent()) + + // wait for async handler to complete logging + await signalStream.first { _ in true } + + // expect the event was logged + #expect(eventLogger.logEventStub.calls.count == 1) + } + +#### Key Points + + - **Prologues execute before stub**: Control timing before mock processes input + - **Epilogues execute after stub**: Signal completion or coordinate post-execution state + - **Eliminates arbitrary delays**: Replace `Task.sleep` with precise synchronization + - **Test intermediate states**: Verify system behavior at different execution phases + - **Optional by default**: Tests can ignore if timing control isn't needed + - **Separate concerns**: Prologue/epilogue handle timing, stub handles return values/errors - - **Use `confirmation`**: Wrap observation tracking with `confirmation { callback in }` to properly wait for async changes - - **Call callback on change**: Invoke the confirmation callback in the `onChange` closure - - **Allow processing time**: Use `Task.sleep(for:)` after triggering the action to allow async processing - - **Mark test as async**: Use `async throws` and `await` for the confirmation - - **Verify final state**: Check the final state after the confirmation completes ## Integration with Existing Documentation This testing documentation supplements the main project documentation: - **Test Mocks**: See `@Documentation/TestMocks.md` for detailed mock object patterns - - **Dependency Injection**: See `@Documentation/DependencyInjection.md` for dependency + - **Dependency Injection**: See `@Documentation/DependencyInjection.md` for dependency patterns - - **MVVM Testing**: See `@Documentation/MVVMForSwiftUI.md` for view model testing approaches -When in doubt, follow existing patterns from similar tests in the codebase and reference the +When in doubt, follow existing patterns from similar tests in the codebase and reference the established documentation for architectural guidance. diff --git a/Scripts/format b/Scripts/format index d762974..a450349 100755 --- a/Scripts/format +++ b/Scripts/format @@ -8,6 +8,5 @@ REPO_ROOT="$(dirname "$SCRIPT_DIR")" # Run swift format with --in-place to fix formatting issues swift format --in-place --recursive \ - "$REPO_ROOT/Packages/" \ "$REPO_ROOT/Sources/" \ "$REPO_ROOT/Tests/" diff --git a/Scripts/install-git-hooks b/Scripts/install-git-hooks index e29caa6..56d6935 100755 --- a/Scripts/install-git-hooks +++ b/Scripts/install-git-hooks @@ -6,78 +6,51 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" # Go to the repository root (one level up from Scripts) REPO_ROOT="$(dirname "$SCRIPT_DIR")" -# Check if we're in a git repository -if [ ! -d "$REPO_ROOT/.git" ]; then +# Check if we're in a git repository and get the git directory +# This works for both regular repos and worktrees +cd "$REPO_ROOT" +if ! git rev-parse --git-dir > /dev/null 2>&1; then echo "Error: Not in a git repository" exit 1 fi -mkdir -p "$REPO_ROOT/.git/hooks" - -# Function to install the pre-commit hook -install_pre_commit_hook() { - local pre_commit_hook="$REPO_ROOT/.git/hooks/pre-commit" - - echo "Installing pre-commit hook..." - - cat > "$pre_commit_hook" << 'EOF' -#!/bin/bash - -# Get the directory where this hook is located -HOOK_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" - -# Go to the repository root (two levels up from .git/hooks) -REPO_ROOT="$(dirname "$(dirname "$HOOK_DIR")")" - -# Run the lint script -echo "Running lint check..." -if ! "$REPO_ROOT/Scripts/lint"; then - echo "Lint check failed. Please fix formatting issues before committing." +# Get the common git directory (shared across all worktrees) +GIT_COMMON_DIR="$(git rev-parse --git-common-dir)" +if [ ! -d "$GIT_COMMON_DIR" ]; then + echo "Error: Could not find git directory" exit 1 fi -echo "Lint check passed." -EOF - - chmod +x "$pre_commit_hook" - echo "Pre-commit hook installed successfully!" -} +mkdir -p "$GIT_COMMON_DIR/hooks" # Function to install the pre-push hook install_pre_push_hook() { - local pre_push_hook="$REPO_ROOT/.git/hooks/pre-push" + local pre_push_hook="$GIT_COMMON_DIR/hooks/pre-push" echo "Installing pre-push hook..." cat > "$pre_push_hook" << 'EOF' #!/bin/bash -# Get the directory where this hook is located -HOOK_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" - -# Go to the repository root (two levels up from .git/hooks) -REPO_ROOT="$(dirname "$(dirname "$HOOK_DIR")")" +# Resolve the repository root using git, which correctly handles worktrees +REPO_ROOT="$(git rev-parse --show-toplevel)" -# Run the test-all-platforms script -echo "Running tests on all platforms..." -if ! "$REPO_ROOT/Scripts/test-all-platforms"; then - echo "Platform tests failed. Please fix issues before pushing." +# Run the lint script +echo "Running lint check..." +if ! "$REPO_ROOT/Scripts/lint"; then + echo "Lint check failed. Please fix formatting issues before pushing." exit 1 fi -echo "All platform tests passed." +echo "Lint check passed." EOF chmod +x "$pre_push_hook" echo "Pre-push hook installed successfully!" } -# Install the pre-commit hook -install_pre_commit_hook - # Install the pre-push hook install_pre_push_hook echo "All git hooks installed successfully!" -echo "The pre-commit hook will run 'Scripts/lint' before each commit." -echo "The pre-push hook will run 'Scripts/test-all-platforms' before each push." +echo "The pre-push hook will run 'Scripts/lint' before each push." diff --git a/Scripts/test-all-platforms b/Scripts/test-all-platforms deleted file mode 100755 index ae40af3..0000000 --- a/Scripts/test-all-platforms +++ /dev/null @@ -1,53 +0,0 @@ -#!/bin/bash - -set -euo pipefail - -# Colors for output -RED='\033[0;31m' -GREEN='\033[0;32m' -YELLOW='\033[1;33m' -NC='\033[0m' # No Color - -# Function to print colored output -print_status() { - echo -e "${YELLOW}[$(date +'%H:%M:%S')] $1${NC}" -} - -print_success() { - echo -e "${GREEN}✓ $1${NC}" -} - -print_error() { - echo -e "${RED}✗ $1${NC}" -} - -# Platforms to test -PLATFORMS=( - "iOS Simulator,name=iPhone 17 Pro" - "macOS" - "tvOS Simulator,name=Apple TV 4K" - "watchOS Simulator,name=Apple Watch Series 10" -) - -SCHEME="DevFoundation-Package" - -print_status "Starting tests on all platforms..." -echo - -for platform in "${PLATFORMS[@]}"; do - platform_name=$(echo "$platform" | cut -d',' -f1) - print_status "Testing on $platform_name..." - - if xcodebuild test -scheme "$SCHEME" -destination "platform=$platform"; then - print_success "$platform_name tests passed" - echo - else - print_error "$platform_name tests failed" - exit 1 - fi -done - -# Summary -echo "==========================" -print_success "All platform tests passed!" -exit 0 diff --git a/Tests/DevFoundationTests/Concurrency/WithTimeoutTests.swift b/Tests/DevFoundationTests/Concurrency/WithTimeoutTests.swift index 34e81ce..dc3c242 100644 --- a/Tests/DevFoundationTests/Concurrency/WithTimeoutTests.swift +++ b/Tests/DevFoundationTests/Concurrency/WithTimeoutTests.swift @@ -18,7 +18,7 @@ struct WithTimeoutTests: RandomValueGenerating { mutating func operationCompletesBeforeTimeout() async throws { let expectedResult = randomAlphanumericString() - let result = try await withTimeout(.milliseconds(500)) { + let result = try await withTimeout(.seconds(60)) { try await Task.sleep(for: .milliseconds(150)) return expectedResult } @@ -32,8 +32,8 @@ struct WithTimeoutTests: RandomValueGenerating { let result = randomInt(in: .min ... .max) await #expect(throws: CancellationError.self) { - _ = try await withTimeout(.milliseconds(500)) { - try await Task.sleep(for: .milliseconds(1000)) + _ = try await withTimeout(.milliseconds(100)) { + try await Task.sleep(for: .seconds(10)) return result } } @@ -69,9 +69,9 @@ struct WithTimeoutTests: RandomValueGenerating { _ = await confirmation { (operationStarted) in await confirmation(expectedCount: 0) { (operationFinished) in await #expect(throws: CancellationError.self) { - try await withTimeout(.milliseconds(10)) { + try await withTimeout(.milliseconds(100)) { operationStarted() - try await Task.sleep(for: .milliseconds(500)) + try await Task.sleep(for: .seconds(10)) operationFinished() } }