From 53d437459dcfa62c62c39615e58819e777f71a60 Mon Sep 17 00:00:00 2001 From: Prachi Gauriar Date: Fri, 20 Mar 2026 01:05:23 -0400 Subject: [PATCH] Update scripts, docs, and workflows to latest and greatest - Re-enable non-macOS builds - Update lint rules to be slightly more permissive - Update GitHub Actions workflow to be a little more sophisticated - Clean up scripts - Update docs and guides --- .claude/settings.local.json | 7 + .github/workflows/VerifyChanges.yaml | 63 +-- .swift-format | 4 +- CLAUDE.md | 137 ++--- Documentation/MarkdownStyleGuide.md | 7 +- Documentation/TestMocks.md | 282 +++++++++-- Documentation/TestingGuidelines.md | 717 +++++++++++++++++++++++++++ README.md | 10 +- Scripts/install-git-hooks | 63 +-- Scripts/lint | 1 - Scripts/test-all-platforms | 68 --- 11 files changed, 1102 insertions(+), 257 deletions(-) create mode 100644 .claude/settings.local.json create mode 100644 Documentation/TestingGuidelines.md delete mode 100755 Scripts/test-all-platforms diff --git a/.claude/settings.local.json b/.claude/settings.local.json new file mode 100644 index 0000000..ded820a --- /dev/null +++ b/.claude/settings.local.json @@ -0,0 +1,7 @@ +{ + "permissions": { + "allow": [ + "Bash(git:*)" + ] + } +} diff --git a/.github/workflows/VerifyChanges.yaml b/.github/workflows/VerifyChanges.yaml index 0d2e7aa..791ab6c 100644 --- a/.github/workflows/VerifyChanges.yaml +++ b/.github/workflows/VerifyChanges.yaml @@ -7,17 +7,17 @@ on: branches: ["main"] env: - XCODE_VERSION: 26.0.1 + XCODE_VERSION: 26.3 jobs: lint: name: Lint - runs-on: macos-15 + 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 @@ -29,39 +29,32 @@ jobs: fail-fast: false matrix: include: -# - platform: iOS -# xcode_destination: "platform=iOS Simulator,name=iPhone 16 Pro" -# xcode_project: "DevKeychain.xcodeproj" -# xcode_scheme: "DevKeychainApp" + - platform: iOS + xcode_destination: "platform=iOS Simulator,name=iPhone 17 Pro" - platform: macOS xcode_destination: "platform=macOS,arch=arm64" - xcode_project: "" - xcode_scheme: "DevKeychain" -# - platform: tvOS -# xcode_destination: "platform=tvOS Simulator,name=Apple TV 4K (3rd generation)" -# xcode_project: "" -# xcode_scheme: "DevKeychain" -# - platform: watchOS -# xcode_destination: "platform=watchOS Simulator,name=Apple Watch Series 10 (46mm)" -# xcode_project: "" -# xcode_scheme: "DevKeychain" + - 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 + OTHER_XCBEAUTIFY_FLAGS: --renderer github-actions XCCOV_PRETTY_VERSION: 1.2.0 - XCODE_PROJECT: ${{ matrix.xcode_project }} - XCODE_SCHEME: ${{ matrix.xcode_scheme }} + XCODE_SCHEME: DevKeychain XCODE_DESTINATION: ${{ matrix.xcode_destination }} - XCODE_TEST_PRODUCTS_PATH: .build/DevFoundation.xctestproducts + XCODE_TEST_PRODUCTS_PATH: .build/DevKeychain.xctestproducts 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 @@ -69,7 +62,7 @@ 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 }}-${{ github.sha }} @@ -77,15 +70,13 @@ jobs: - uses: irgaly/xcode-cache@v1 if: steps.cache-xctestproducts-restore.outputs.cache-hit != 'true' with: - key: xcode-cache-deriveddata-${{ github.workflow }}-${{ matrix.platform }}-${{ 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: | - **/*.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved - Package.resolved + swiftpm-package-resolved-file: Package.resolved - name: Build for Testing id: build-for-testing @@ -98,8 +89,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 }} @@ -122,7 +113,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: | @@ -132,7 +123,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 @@ -148,7 +139,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 f1c36e6..4a10b3b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -10,37 +10,23 @@ DevKeychain is a Swift package providing a modern, type-safe interface to Apple' services. It supports version 26 of Apple's OSes and requires a Swift 6.2+ toolchain. -## Common Development Commands +## Development Commands ### Building and Testing - # Build the package - swift build - - # Run all tests - swift test - - # Run specific test - swift test --filter - - # Build and test with code coverage - swift test --enable-code-coverage + - **Build**: `swift build` + - **Test all**: `swift test` + - **Test specific**: `swift test --filter ` + - **Test with coverage**: `swift test --enable-code-coverage` ### Code Quality - # Format code (requires swift-format) - swift-format --in-place --recursive Sources/ Tests/ - - # Check formatting - swift-format --recursive Sources/ Tests/ + - **Lint**: `Scripts/lint` (uses `swift format lint --recursive --strict`) + - **Format**: `Scripts/format` (uses `swift format --in-place --recursive`) ### Git Hooks - # Install pre-commit hooks (if Scripts directory exists) - Scripts/install-git-hooks - - # Run linting manually - Scripts/lint + - **Install**: `Scripts/install-git-hooks` (installs pre-push hook that runs lint) ## Architecture Overview @@ -123,23 +109,80 @@ Uses **Swift Testing** framework with comprehensive mocking: The project uses GitHub Actions for continuous integration: - **Linting**: Automatically checks code formatting on all pull requests using `swift format` - - **Testing**: Runs tests on macOS (iOS, tvOS, and watchOS testing are disabled in CI due to - reliability issues) + - **Testing**: Builds and tests on iOS, macOS, tvOS, and watchOS - **Coverage**: Generates code coverage reports using xccovPretty - -For comprehensive cross-platform testing, developers should run `Scripts/test-all-platforms` -locally or rely on the pre-push git hook which automatically runs all platform tests before -pushing changes. + - Uses Xcode 26.3 and macOS 26 runners ## Platform Requirements - Swift 6.2+ toolchain required - - Xcode 26.0 for CI/CD + - Xcode 26.3 for CI/CD - Apple platforms only (iOS/macOS/tvOS/visionOS/watchOS version 26) - Uses modern Swift concurrency features +## Testing and Mocking Standards + +### Test Mock Architecture + +The codebase uses a consistent stub-based mocking pattern built on the DevTesting framework: + +#### Core Mock Patterns + + - **Stub-based mocks**: All mocks use `Stub` or + `ThrowingStub` + - **Force-unwrapped stubs**: Stub properties are declared with `!` — tests must configure them + - **Swift 6 concurrency**: All stub properties marked `nonisolated(unsafe)` + - **Argument structures**: Complex parameters use dedicated structures (e.g., + `LogErrorArguments`) + +#### Mock Organization + + - **File naming**: `Mock[ProtocolName].swift` + - **Type naming**: `Mock[ProtocolName]` + - **Stub properties**: `[functionName]Stub` + - **Location**: `Tests/DevKeychainTests/Testing Helpers/` + +#### Test Patterns + + - Use `@Test` with Swift Testing framework + - Use `#expect()` and `#require()` for assertions + - Always configure stubs before use to avoid crashes + - Leverage DevTesting's call tracking for verification + + +## Documentation Standards + +Follow the project's Markdown Style Guide: + + - **Line length**: 100 characters max + - **Code blocks**: Use 4-space indentation, not fenced blocks + - **Lists**: Use `-` for bullets, align continuation lines with text + - **Spacing**: 2 blank lines between major sections, 1 after headers + - **Terminology**: Use "function" over "method", "type" over "class" + +When writing Markdown documentation, reference `@Documentation/MarkdownStyleGuide.md` to +ensure consistent formatting, structure, and style across all project documentation. + + +## 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 + + ## Scripts Directory The `Scripts/` directory contains utility scripts for development workflow automation: @@ -147,37 +190,19 @@ The `Scripts/` directory contains utility scripts for development workflow autom ### Available Scripts **`Scripts/install-git-hooks`**: - - Installs pre-commit and pre-push git hooks for automated quality checks - - Creates `.git/hooks/pre-commit` that calls `Scripts/lint` for formatting validation - - Creates `.git/hooks/pre-push` that calls `Scripts/test-all-platforms` for comprehensive testing - - Prevents commits with formatting issues and pushes with test failures - - Run once per repository to set up automated code quality and testing workflow + - Installs a pre-push git hook that runs `Scripts/lint` for formatting validation + - Works correctly with git worktrees + - Run once per repository to set up automated code quality workflow **`Scripts/lint`**: - - Runs swift-format lint validation with strict mode enabled + - Runs `swift format lint` validation with strict mode enabled - Checks `App/`, `Sources/`, and `Tests/` directories recursively - Returns non-zero exit code if formatting issues are found - - Used by pre-commit hooks and can be run manually for code quality verification - -**`Scripts/test-all-platforms`**: - - Runs comprehensive tests across all supported Apple platforms - - Tests on iOS Simulator (iPhone 16 Pro), macOS, tvOS Simulator (Apple TV 4K), and watchOS - Simulator (Apple Watch Series 10) - - Uses different project configurations: DevKeychainApp scheme for iOS, DevKeychain scheme for - other platforms - - Provides colored output with timestamps and clear success/failure indicators - - Returns non-zero exit code if any platform tests fail - - Essential for local cross-platform validation since CI only tests macOS - -### Usage Patterns - - - Run `Scripts/install-git-hooks` after cloning the repository for complete automation - - Pre-commit hooks automatically run `Scripts/lint` before each commit - - Pre-push hooks automatically run `Scripts/test-all-platforms` before each push - - Manual operations: - - Code formatting check: `Scripts/lint` - - Cross-platform testing: `Scripts/test-all-platforms` - - All scripts work from any directory by calculating repository root path + - Used by pre-push hook and can be run manually for code quality verification + +**`Scripts/format`**: + - Runs `swift format --in-place` to automatically fix formatting issues + - Formats `App/`, `Sources/`, and `Tests/` directories recursively ## Key Files for Understanding diff --git a/Documentation/MarkdownStyleGuide.md b/Documentation/MarkdownStyleGuide.md index d94f75c..da25aa8 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 @@ -12,9 +11,9 @@ Keep all lines under **100 characters**. Break long sentences and paragraphs at to stay within this limit. ✅ Good: - Faucibus consectetur lacinia nostra eros conubia nibh inceptos hendrerit, ante blandit + Faucibus consectetur lacinia nostra eros conubia nibh inceptos hendrerit, ante blandit vulputate imperdiet amet porttitor torquent mattis. - + ❌ Bad: Faucibus consectetur lacinia nostra eros conubia nibh inceptos hendrerit, ante blandit vulputate imperdiet amet porttitor torquent mattis. diff --git a/Documentation/TestMocks.md b/Documentation/TestMocks.md index 3e9ab80..afca4c9 100644 --- a/Documentation/TestMocks.md +++ b/Documentation/TestMocks.md @@ -1,28 +1,52 @@ # Test Mock Documentation -This document outlines the patterns and conventions for writing test mocks in the Shopper iOS -codebase. +This document outlines the patterns and conventions for writing test mocks in Swift. + +Its helpful to read the [Dependency Injection guide](DependencyInjection.md) before reading this +guide, as it introduces core principles for how we think about dependency injection. ## Overview -The codebase uses a consistent approach to mocking based on the DevTesting framework's `Stub` type. -All mocks follow standardized patterns that make them predictable, testable, and maintainable. +We use a consistent approach to mocking based on the DevTesting package's `Stub` and `ThrowingStub` +types. All mocks follow standardized patterns that make them predictable, testable, and +maintainable. + + +## When to Mock vs. Use Types Directly + +Create mock protocols when + + - The type has **non-deterministic behavior** (network calls, file I/O, time-dependent operations) + - You need to **control or observe the behavior** in tests + - The type's behavior **varies across environments** + +Use types directly when + + - The type has **deterministic, predictable behavior** + - Testing with the real implementation provides **sufficient coverage** + - Creating abstractions adds **complexity without testing benefits** + +It's worth pointing out that the following foundational types should be used directly. + + - **`NotificationCenter`**: Posting and observing notifications is predictable + - **`UserDefaults`**: Simple key-value storage with consistent behavior + - **`Bundle`**: Resource loading behavior is consistent and testable + - **`EventBus`**: Synchronous event dispatching with deterministic outcomes ## Core Mock Patterns ### 1. Stub-Based Architecture -All mocks use `DevTesting`’s `Stub` or `ThrowingStub` types +All mocks use `DevTesting`'s `Stub` or `ThrowingStub` types for function and property implementations: import DevTesting final class MockService: ServiceProtocol { - nonisolated(unsafe) - var performActionStub: Stub! + nonisolated(unsafe) var performActionStub: Stub! func performAction(_ input: String) -> Bool { @@ -48,8 +72,7 @@ When functions have multiple parameters, create dedicated argument structures: } - nonisolated(unsafe) - var logErrorStub: Stub! + nonisolated(unsafe) var logErrorStub: Stub! func logError(_ error: some Error, attributes: [String : any Encodable]) { @@ -70,11 +93,8 @@ For services that only expose properties (like `MockAppServices`), each property stub: final class MockAppServices: PlatformAppServices { - nonisolated(unsafe) - var stylesheetStub: Stub! - - nonisolated(unsafe) - var telemetryEventLoggerStub: Stub! + nonisolated(unsafe) var stylesheetStub: Stub! + nonisolated(unsafe) var telemetryEventLoggerStub: Stub! var stylesheet: Stylesheet { @@ -98,6 +118,7 @@ For protocols with associated types, create generic mocks: var eventData: EventData } + extension MockTelemetryEvent: Equatable where EventData: Equatable { } extension MockTelemetryEvent: Hashable where EventData: Hashable { } @@ -128,20 +149,27 @@ For testing error scenarios, use simple enum-based errors: ## Mock Organization -### File Structure +### File Structure and Organization +#### Directory Structure: Tests/ - ├── AppPlatformTests/ - │ └── Testing Support/ - │ ├── MockAppServices.swift - │ ├── MockBootstrapper.swift - │ └── MockSubapp.swift - └── TelemetryTests/ - └── Testing Support/ - ├── MockTelemetryDestination.swift - ├── MockTelemetryEvent.swift - └── MockError.swift - + ├── [PackageName]Tests/ # Package-specific tests + │ ├── Unit Tests/ + │ │ └── [ModuleName] # Feature-specific tests + │ │ └── [ProtocolName]Tests.swift + │ └── Testing Support/ # Mock objects and test utilities + │ ├── Mock[ProtocolName].swift # Mock implementations + │ ├── MockError.swift # Test-specific error types + │ └── RandomValueGenerating+[ModuleName].swift # Random value extensions + +#### File Placement Guidelines: + +- **Unit Test files**: Place in `Tests/[PackageName]/Unit Tests/`, matching the path of the source file in + the directory structure under `Sources/` +- **Mock objects**: Always place in `Tests/[PackageName]/Testing Support/` directories +- **One mock per file**: Each protocol should have its own dedicated mock file +- **Sharing mocks**: Do not share mocks between Packages. If the same Mock is needed across + multiple packages, duplicate it. ### Naming Conventions @@ -164,8 +192,7 @@ When mocking types with custom initializers, use static stubs: } - nonisolated(unsafe) - static var initStub: Stub! + nonisolated(unsafe) static var initStub: Stub! init(appConfiguration: AppConfiguration, subappServices: any SubappServices) async { @@ -180,12 +207,194 @@ For functions that might not be called in every test, provide default stub value final class MockSubapp: Subapp { // Initialize to non-nil to avoid crashes in tests that don't configure this stub - nonisolated(unsafe) - var installTelemetryBusEventHandlersStub: Stub = .init() + nonisolated(unsafe) var installTelemetryBusEventHandlersStub: Stub< + TelemetryBusEventObserver, + Void + > = .init() } +**CRITICAL - Stubs Accessed by Internal Async Tasks:** + +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 + -### 3. Protocol Imports with @testable +### 4. Protocol Imports with @testable Import protocols under test with `@testable` when accessing internal details: @@ -256,12 +465,11 @@ Import protocols under test with `@testable` when accessing internal details: 1. **Always configure stubs**: Force-unwrapped stubs will crash if not configured 2. **Use argument structures**: Simplifies complex parameter verification - 3. **Maintain protocol fidelity**: Mocks should behave like real implementations - 4. **Leverage DevTesting**: Use the framework's call tracking and verification capabilities - 5. **Keep mocks simple**: Avoid complex logic in mock implementations - 6. **Group related mocks**: Place mocks in appropriate Testing Support directories - 7. **Follow naming conventions**: Consistent naming improves maintainability - 8. **Use Swift Testing**: Leverage `@Test`, `#expect()`, and `#require()` for assertions + 3. **Leverage DevTesting**: Use the package's call tracking and verification capabilities + 4. **Keep mocks simple**: Avoid complex logic in mock implementations + 5. **Group related mocks**: Place mocks in appropriate Testing Support directories + 6. **Follow naming conventions**: Consistent naming improves maintainability + 7. **Use Swift Testing**: Leverage `@Test`, `#expect()`, and `#require()` for assertions ## Thread Safety diff --git a/Documentation/TestingGuidelines.md b/Documentation/TestingGuidelines.md new file mode 100644 index 0000000..141ff45 --- /dev/null +++ b/Documentation/TestingGuidelines.md @@ -0,0 +1,717 @@ +# Testing Guidelines for Claude Code + +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 +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(throws:)`** for testing error conditions instead of `XCTAssertThrows` + - **No "test" prefixes** required on function names + - **Struct-based test organization** instead of class-based + +### 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 + `testInitialLoadingState()` + - **Protocol-specific naming**: For protocols with concrete implementations, name tests after + the concrete type (e.g., `StandardAuthenticationRemoteNotificationHandlerTests`) + +### Unit Test Structure 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". + - **Expect**: Expect one or more results to be true, using Swift Testing expressions. + - **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 + - // expect that the loadUser stub is invoked once + - If two sections overlap, only mention the most relevant information. + +### Mock Testing Strategy + + - **Focus on verification**: Test that mocks are called correctly, not custom mock + implementations + - **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. + - Do *NOT* leave comments when stubs are omitted because they are irrelevant. + +### ThrowingStub Usage + +**CRITICAL**: DevTesting's `ThrowingStub` has very specific initialization patterns that +differ from regular `Stub`. Using incorrect initializers will cause compilation errors. + +#### Correct ThrowingStub Patterns: + + // For success cases: + ThrowingStub(defaultReturnValue: value) + + // For error cases: + ThrowingStub(defaultError: error) + + // For void return types that don't throw: + ThrowingStub(defaultError: nil) + +#### Common Mistakes to Avoid: + + - ❌ `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` + - **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 + +Example mock structure: + + final class MockProtocolName: ProtocolName { + nonisolated(unsafe) var functionStub: Stub! + + func function(input: InputType) -> OutputType { + functionStub(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 +`mutating` functions. This creates a specific pattern that must be followed. + +#### Correct Pattern for Random Value Generation: + + @MainActor + struct MyTests: RandomValueGenerating { + var randomNumberGenerator = makeRandomNumberGenerator() + + @Test + mutating func myTest() throws { + let randomValue = randomAlphanumericString() + // ... test logic + } + } + +#### Key Requirements: + + - **Test struct must conform to `RandomValueGenerating`** + - **Include `var randomNumberGenerator = makeRandomNumberGenerator()` property** + - **Mark test functions as `mutating`** when using random value generation + - **Test struct can be immutable** for tests that don't use random values + +#### Dedicated Random Value Extensions: + + - **Dedicated files**: Create `RandomValueGenerating+[ModuleName].swift` files for random value + generation + - **Centralized functions**: Move random value creation functions to these dedicated extension + files + - **Consistent patterns**: Follow existing patterns from other modules + - **Proper imports**: Include necessary `@testable import` statements for modules being + extended + +Example structure: + + import DevTesting + import Foundation + + @testable import ModuleName + + extension RandomValueGenerating { + mutating func randomModuleSpecificType() -> ModuleType { + return ModuleType( + property: randomAlphanumericString() + ) + } + } + + +## File Organization + +### Test Files + + - **Naming pattern**: `[TypeName]Tests.swift` in corresponding Tests directories + - **Location**: Place in `Tests/[ModuleName]/[Category]/` directories + - **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 + + - **Naming pattern**: `Mock[ProtocolName].swift` + - **Location**: Place in `Tests/[ModuleName]/Testing Support/` directories + - **Protocol-based**: Mock the protocol interface, not concrete implementations + +### Random Value Extensions + + - **Naming pattern**: `RandomValueGenerating+[ModuleName].swift` + - **Location**: Place in `Tests/[ModuleName]/Testing Support/` directories + - **Module-specific**: Create extensions for each module's unique types + +### Import Patterns + + - **Testable imports**: Use `@testable import ModuleName` for modules under test + - **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 + 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`, +otherwise you'll get "Errors thrown from here are not handled" compilation errors. + +#### Correct Pattern: + + @Test + func myTestThatExpectsErrors() throws { + #expect(throws: SomeError.self) { + try somethingThatThrows() + } + } + +#### Common Mistake: + + // ❌ This will cause compilation error + @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 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 + 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 + 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 = SystemUnderTest() + + #expect(instance.property == expectedDefault) + } + +### Testing Dependency Calls + + @Test + mutating func functionCallsDependency() { + let mock = MockDependency() + mock.functionStub = Stub() + + let instance = SystemUnderTest(dependency: mock) + instance.performAction() + + #expect(mock.functionStub.calls.count == 1) + } + +### Testing Error Scenarios + + @Test + mutating func functionThrowsWhenDependencyFails() { + let mock = MockDependency() + let error = MockError(description: "Test error") + mock.functionStub = ThrowingStub(defaultError: error) + + let instance = SystemUnderTest(dependency: mock) + + #expect(throws: MockError.self) { + try instance.performAction() + } + } + +### Testing Async Operations + + @Test + mutating func asyncFunctionCompletesSuccessfully() async throws { + let mock = MockDependency() + mock.asyncFunctionStub = Stub(defaultReturnValue: expectedResult) + + let instance = SystemUnderTest(dependency: mock) + let result = await instance.performAsyncAction() + + #expect(result == expectedResult) + #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() + } + } + +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 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 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 = 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() + + // 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) + } + +**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 + + +## 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 + patterns + +When in doubt, follow existing patterns from similar tests in the codebase and reference the +established documentation for architectural guidance. diff --git a/README.md b/README.md index 1f00683..f7ccbff 100644 --- a/README.md +++ b/README.md @@ -21,9 +21,8 @@ interfaces are fully documented and tested. We aim for overall test coverage ove To set up the development environment: 1. Run `Scripts/install-git-hooks` to install git hooks that automatically check code - formatting on commits and run comprehensive tests before pushing. + formatting before pushing. 2. Use `Scripts/lint` to manually check code formatting at any time. - 3. Use `Scripts/test-all-platforms` to run tests on all supported platforms locally. ## Continuous Integration @@ -31,14 +30,9 @@ To set up the development environment: DevKeychain uses GitHub Actions for continuous integration. The CI pipeline: - **Linting**: Automatically checks code formatting on all pull requests using `swift format` - - **Testing**: Runs tests on macOS (iOS, tvOS, and watchOS testing are disabled in CI due to - reliability issues) + - **Testing**: Builds and tests on iOS, macOS, tvOS, and watchOS - **Coverage**: Generates code coverage reports using xccovPretty -For comprehensive cross-platform testing, developers should run `Scripts/test-all-platforms` -locally or rely on the pre-push git hook which automatically runs all platform tests before -pushing changes. - ## Bugs and Feature Requests 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/lint b/Scripts/lint index fc8a054..1901415 100755 --- a/Scripts/lint +++ b/Scripts/lint @@ -8,4 +8,3 @@ REPO_ROOT="$(dirname "$SCRIPT_DIR")" # Run swift format lint with full paths to preserve user's PWD swift format lint --recursive --strict "$REPO_ROOT/App/" "$REPO_ROOT/Sources/" "$REPO_ROOT/Tests/" - diff --git a/Scripts/test-all-platforms b/Scripts/test-all-platforms deleted file mode 100755 index bd6d901..0000000 --- a/Scripts/test-all-platforms +++ /dev/null @@ -1,68 +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 16 Pro" - "macOS" - "tvOS Simulator,name=Apple TV 4K" - "watchOS Simulator,name=Apple Watch Series 10" -) - -DEFAULT_SCHEME="DevKeychain" -IOS_PROJECT="DevKeychain.xcodeproj" -IOS_SCHEME="DevKeychainApp" -FAILED_PLATFORMS=() - -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..." - - # Set project specifier based on platform - if [[ "$platform_name" == "iOS Simulator" ]]; then - PROJECT_SPECIFIER="-project $IOS_PROJECT -scheme $IOS_SCHEME" - else - PROJECT_SPECIFIER="-scheme $DEFAULT_SCHEME" - fi - - if xcodebuild test $PROJECT_SPECIFIER -destination "platform=$platform"; then - print_success "$platform_name tests passed" - else - print_error "$platform_name tests failed" - FAILED_PLATFORMS+=("$platform_name") - fi - echo -done - -# Summary -echo "==========================" -if [ ${#FAILED_PLATFORMS[@]} -eq 0 ]; then - print_success "All platform tests passed!" - exit 0 -else - print_error "Tests failed on: ${FAILED_PLATFORMS[*]}" - exit 1 -fi