Add Swift Testing framework auto-detection and mixed XCTest/Testing support for Wasm #78
Conversation
…ightdigit/MistKit.git subrepo: subdir: "MistKit" merged: "7ae45ca" upstream: origin: "git@github.com:brightdigit/MistKit.git" branch: "swift-build-1.5.0-beta.2" commit: "7ae45ca" git-subrepo: version: "0.4.9" origin: "https://github.com/Homebrew/brew" commit: "1383417817"
…rightdigit/SyndiKit.git subrepo: subdir: "SyndiKit" merged: "dcabb15" upstream: origin: "git@github.com:brightdigit/SyndiKit.git" branch: "swift-build-v1.5.0-beta.1" commit: "dcabb15" git-subrepo: version: "0.4.9" origin: "https://github.com/Homebrew/brew" commit: "1383417817"
Implements automatic testing framework detection for WebAssembly tests by scanning test sources for import statements. Fixes issue #77 where Swift Testing tests in .xctest binaries were not being discovered. Changes: - Add wasm-testing-library input parameter (auto/swift-testing/xctest/both/none) - Add wasm-swift-test-flags input parameter for additional test runner flags - Add detection step that scans Tests/ directory for framework imports - Update WasmKit execution logic to use detected mode with switch/case - Update Wasmtime execution logic to use detected mode with switch/case - Update CLAUDE.md documentation with new parameters and usage examples Testing framework detection: - Scans for "import Testing" → swift-testing mode - Scans for "import XCTest" → xctest mode - Both found → both mode (runs tests twice) - Neither found → none mode (no flags) Backward compatible: Default 'auto' mode preserves existing behavior while fixing .xctest binaries containing Swift Testing tests. Fixes #77 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Introduces comprehensive test package combining Swift Testing and XCTest frameworks to validate "both" mode detection across all platforms. Package structure: - 3 library targets (Calculator, Validator, Utils) with dependencies - 4 test targets: 2 Swift Testing + 2 XCTest - 17 total tests (9 Swift Testing + 8 XCTest) CI coverage: - test-mixed-framework-macos (11 configurations) - test-mixed-framework-ubuntu (10 configurations) - test-mixed-framework-windows (4 configurations) - test-mixed-framework-android (1 configuration) - test-mixed-framework-wasm (4 configurations) Critical for validating issue #77 fix: Wasm builds with mixed frameworks should auto-detect "both" mode and run tests twice (once per framework). Local validation: - Package builds successfully (swift build) - All 17 tests pass (swift test) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
subrepo: subdir: "MistKit" merged: "f71e8d0" upstream: origin: "git@github.com:brightdigit/MistKit.git" branch: "swift-build-1.5.0-beta.2" commit: "f71e8d0" git-subrepo: version: "0.4.9" origin: "https://github.com/Homebrew/brew" commit: "1383417817"
subrepo: subdir: "SyndiKit" merged: "2ebbba7" upstream: origin: "git@github.com:brightdigit/SyndiKit.git" branch: "swift-build-v1.5.0-beta.1" commit: "2ebbba7" git-subrepo: version: "0.4.9" origin: "https://github.com/Homebrew/brew" commit: "1383417817"
Implements test result parsing capabilities to extract test statistics and validate test counts across all platforms, ensuring MixedFrameworkPackage reports exactly 17 tests (9 Swift Testing + 8 XCTest). New Features: - Created parse-test-output.sh script to extract test counts from output - Added parse-test-results input to enable test output parsing - Added expected-test-count input to validate actual vs expected counts - Added fail-on-parse-error input to control parsing error behavior - Added 5 new output variables (test-total, test-passed, test-failed, test-skipped, test-parsing-status) Platform Support: - SwiftPM (macOS, Ubuntu, Windows): Auto-detects Swift Testing vs XCTest, handles mixed frameworks by summing both - Xcode (all Apple platforms): Parses xcodebuild and xcbeautify output - Wasm (including "both" mode): Correctly sums Swift Testing + XCTest from separate test runs - Android: Set to not-applicable (output not capturable from external composite action, documented limitation) Implementation Details: - Text-based parsing using regex patterns (most consistent across platforms) - Uses tail -1 to get final summary lines (handles per-suite + total output) - Wasm "both" mode captures two outputs and merges for parsing - Exit codes: 0=success, 1=parsing failed, 2=validation failed - Validation failures always fail workflow regardless of fail-on-parse-error CI Updates: - Updated all 4 MixedFrameworkPackage job families (macOS, Ubuntu, Windows, Wasm) - All jobs now validate exactly 17 tests with expected-test-count parameter - Added verification steps to display and validate test counts Relates to #77 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…builds Fixes two critical issues discovered during CI testing: 1. Container Temp Directory Issue: - Added mkdir -p check for $RUNNER_TEMP before capturing test output - Falls back to /tmp if $RUNNER_TEMP cannot be created - Applies to SPM, Xcode, and Wasm test execution steps - Resolves "Output file not found" error in Docker containers 2. Xcode Multi-Target Parsing: - Updated parse_xcodebuild() to detect mixed frameworks - Sums all non-zero test counts instead of using tail -1 - Handles multiple "Executed X tests" lines from different test targets - Resolves "Test count mismatch. Expected: 17, Actual: 0" error Technical Details: - Docker containers (swift:6.2-jammy) may not have $RUNNER_TEMP writable - Xcode runs each test target separately, producing multiple summaries - Each target's "Executed X tests" line must be summed - Wrapper suites output "Executed 0 tests" which was being counted Relates to #77 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR adds cross-platform mixed-framework CI testing (macOS, Ubuntu, Windows, Android, Wasm), new action inputs and Wasm testing-framework auto-detection, a test-output capture option, a validation script to aggregate/validate test counts, and a sample MixedFrameworkPackage with both XCTest and Swift Testing suites. Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI Workflow
participant Action as swift-build Action
participant Detector as Wasm Framework Detector
participant Runner as Test Runner (SPM / Xcode / WasmKit / Wasmtime)
participant Storage as Artifact Storage
participant Validator as validate-test-output.sh
CI->>Action: start job with matrices & inputs (wasm-testing-library, save-test-output)
Action->>Detector: run detection step (scan Tests/* for imports) / accept explicit input
Detector-->>Action: detected mode (swift-testing / xctest / both / none)
Action->>Runner: execute tests per platform/runtime with appropriate flags
Runner->>Storage: tee and save test output to files (conditional)
CI->>Validator: download aggregated test outputs across platforms
Validator->>Validator: parse XCTest & Swift Testing counts and validate
Validator-->>CI: exit status and summary
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Review: Swift Testing Framework Support for WasmThis PR adds comprehensive testing support for mixed Swift Testing + XCTest frameworks across all platforms, particularly for WebAssembly. Here's my detailed review: ✅ Strengths1. Excellent Test Coverage & Architecture
2. Comprehensive Wasm Testing Strategy
3. Documentation Excellence
4. Test Result Parsing
|
This commit fixes two critical test result parsing bugs that caused CI
failures for MixedFrameworkPackage validation:
1. Fix parse_wasm_both() to sum all non-zero XCTest results
- Previously used tail -1 which could return "Executed 0 tests" from
wrapper suites
- Now filters out zeros and sums all actual test counts
- Applies same logic as parse_xcodebuild() fix from previous commit
- Resolves: "ERROR: Cannot parse XCTest section in mixed output"
2. Replace $RUNNER_TEMP with working directory paths
- export in one step doesn't persist to parser step (different shells)
- Changed all $RUNNER_TEMP/test-output.txt to ./test-output.txt
- Working directory is always accessible across steps
- Resolves: "ERROR: Output file not found: /__w/_temp/test-output.txt"
Changes:
- scripts/parse-test-output.sh: Update parse_wasm_both() to sum non-zero
XCTest counts (lines 187-221)
- action.yml: Remove temp directory creation code, replace all $RUNNER_TEMP
paths with working directory relative paths
Testing approach: Text parsing everywhere (Option B)
- Simplest solution, just fixing edge cases
- Consistent across all platforms
- Alternative JUnit XML approach not viable for Wasm (swift test --swift-sdk
fails with Foundation Bundle errors)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Pull Request Review: Swift Testing for WasmSummaryThis PR adds comprehensive support for Swift Testing framework detection and execution in WebAssembly builds. The implementation includes automatic framework detection, mixed framework support (Swift Testing + XCTest), and test result parsing for Wasm builds. Code Quality: ✅ ExcellentStrengths
Best Practices: ✅ Strong
Potential Issues:
|
Pull Request Review: Swift Testing Framework Detection for WASMOverviewThis PR implements automatic Swift Testing framework detection for WebAssembly builds, addressing issue #77. The implementation adds new input parameters and testing capabilities for WASM builds. Code Quality & Best Practices✅ Positive Aspects
Potential Issues & Concerns
|
Add comprehensive test output examples from MixedFrameworkPackage covering: - Standard swift test (native macOS with mixed frameworks) - xcodebuild test output (verbose build pipeline, ~263KB) - WASM with Swift Testing (WasmKit runtime, --testing-library flag) - WASM with XCTest (WasmKit runtime, no testing library flag) - WASM standard build output - WASM embedded build failure (Foundation not available) These fixtures serve as reference implementations for test result parsing validation, covering both XCTest and Swift Testing output formats across different build systems (swift test, xcodebuild, WASM runtimes). Key differences documented: - Timestamp formats (native vs WASM epoch timestamps) - Unicode symbol variations between frameworks - Testing framework detection requirements - Build output verbosity levels Files can be used as test fixtures for validating output parsing logic in the swift-build action. [skip ci] Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Creates validate-test-totals.sh script to parse and verify test counts from SwiftPM, Xcode, and Wasm test outputs. Ensures all 17 tests (8 XCTest + 9 Swift Testing) are discovered across all platforms. The script handles different output formats: - Swift test: Combined XCTest + Swift Testing output - Xcodebuild: Multiple test bundle summaries that need to be summed - Wasm: Separate outputs for XCTest and Swift Testing frameworks Updates test-outputs/README.md to document expected test counts and validation script usage. [skip ci] Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Integrates test count validation into CI workflow to ensure all 17 tests (8 XCTest + 9 Swift Testing) execute successfully across all platforms. Changes: - Add scripts/validate-test-output.sh: stdin-only validation script with three modes (combined, xctest-only, swift-testing-only) - Add save-test-output parameter to action.yml: optional flag to save test output to files (defaults to false, preserving existing behavior) - Modify action.yml test execution steps: conditionally capture output via tee for SPM, Xcode, and Wasm builds when save-test-output=true - Enable output saving for all MixedFrameworkPackage test jobs in workflow: macOS, Ubuntu, Windows, Android, and Wasm - Add artifact uploads after each test job with 1-day retention - Add validate-mixed-framework-test-counts job: final validation job that downloads all artifacts and validates test counts (runs even if tests fail via if: always()) Benefits: - Catches regressions where tests might be accidentally skipped - Centralized validation logic (no per-job repetition) - Platform-specific handling (Wasm validates split XCTest/Swift Testing) - Clear error messages showing expected vs actual counts - No impact on existing workflows (save-test-output defaults to false) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Pull Request Review: Swift Testing Framework Detection for Wasm BuildsOverviewThis PR adds automatic detection and support for mixed testing frameworks (Swift Testing + XCTest) in WebAssembly builds. This is a valuable enhancement that makes the action more flexible and easier to use. ✅ Code Quality & Best PracticesStrengths:
Suggestions:
🐛 Potential Bugs & IssuesCritical:
Minor:
⚡ Performance Considerations
🔒 Security Concerns
🧪 Test CoverageExcellent test coverage:
Suggestions:
📝 Documentation QualityStrengths:
Suggestions:
🎯 RecommendationsHigh Priority:
Medium Priority:
Low Priority:
✨ Overall AssessmentThis is a well-designed and valuable feature that significantly improves the developer experience for Wasm testing. The automatic detection is particularly elegant. With the critical bug fix for output file collisions and improved grep patterns, this will be production-ready. Approval Status: ✅ Approve with minor changes The implementation is solid, tests are comprehensive, and documentation is excellent. The identified issues are fixable and do not block the core functionality. 📊 Metrics
|
Changes:
- Update action.yml to save test output to $RUNNER_TEMP instead of working-directory
- Update workflow artifact paths to use ${{ runner.temp }}
- Update input parameter description to reflect RUNNER_TEMP usage
Benefits:
- Cleaner: doesn't pollute working directory with test output files
- Auto-cleanup: GitHub Actions cleans RUNNER_TEMP automatically
- Consistent: same temp directory location across all platforms
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Code Review: Swift Testing Framework Detection for WASM BuildsThis PR successfully implements automatic testing framework detection for WebAssembly builds, addressing issue #77. ✅ Strengths1. Comprehensive Test Coverage
2. Well-Documented
3. Backward Compatibility
|
Simplify test validation by uploading artifacts from only one representative job per platform instead of all matrix combinations. Changes: - macOS: Only upload from macos-15 (SPM + Xcode macos type) - Ubuntu: Only upload from Swift 6.2 on noble (non-nightly) - Windows: Only upload from windows-2022 - Wasm: Only upload from Ubuntu + wasm + WasmKit default - Android: Skip validation entirely (can't capture output from delegated action) - Validation job: Remove android dependency, update comments This reduces artifact volume while still validating all 17 tests run correctly across each platform type. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
leogdion
left a comment
leogdion
left a comment
There was a problem hiding this comment.
we don't need both new validation scripts. we just need one for the ci
test/MixedFrameworkPackage/test/MixedFrameworkPackage/test-outputs/README.md
Outdated
Show resolved
Hide resolved
Pull Request Review: Swift Testing Support for WasmOverviewThis PR adds comprehensive support for Swift Testing framework detection and execution in WebAssembly (Wasm) builds, including a new ✅ Strengths1. Well-Architected Testing Framework DetectionThe
2. Comprehensive Test CoverageThe new
3. Robust Validation Script
4. Excellent Documentation
5. Proper Separation of Test OutputsFor Wasm tests, the action creates separate output files (
|
- Make test counts configurable via script arguments instead of hardcoded - Remove redundant validation script and test fixture files - Simplify Windows CI to windows-2025 only (reduces 4 jobs to 1) - Remove unnecessary wasm-swift-flags from MixedFrameworkPackage - Add environment variables for expected test counts in validation job Verified locally that MixedFrameworkPackage Wasm tests pass without any compiler flags when using wasmkit with --dir flag for filesystem access. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Pull Request Review: Swift Testing Support for WasmI have completed a comprehensive review of this PR which adds automatic Swift Testing framework detection for WebAssembly builds. Overall, this is excellent work with solid implementation and thorough testing. Strengths1. Excellent Feature Design
2. Robust Implementation
3. Documentation Quality
4. Code Quality
Issues and Recommendations1. Security: Input Validation (Medium Priority)Issue: The wasm-swift-test-flags parameter is directly interpolated into shell commands without sanitization at action.yml lines 1712 and 1846. Impact: While GitHub Actions inputs are trusted sources (require repo write access), this could be problematic if building reusable workflows that accept external inputs. Recommendation: Add basic validation for wasm-swift-test-flags to prevent command injection characters. 2. Test Coverage: Missing Edge Cases (Low Priority)Issue: Test coverage for detection logic could be more comprehensive. Missing test cases include empty Tests/ directory, commented-out imports, and string literals containing imports. Recommendation: Add regex anchoring to the grep pattern at lines 1521 and 1529 to only match actual import statements. 3. Documentation: Parameter Interaction (Low Priority)Issue: The interaction between wasm-testing-library and save-test-output parameters is not clearly documented. Wasm creates separate output files unlike other platforms. Recommendation: Add a note in CLAUDE.md explaining Wasm test output file behavior. 4. Performance: Grep Efficiency (Very Low Priority)Issue: Running grep twice over the entire Tests/ directory could be slow for large test suites. Impact is minimal for typical projects. 5. Code: Duplication Between Runtimes (Low Priority)Issue: Test execution logic is duplicated between Wasmtime (lines 1706-1760) and WasmKit (lines 1840-1894). This creates a maintenance burden. Minor Observations
Test Coverage AssessmentExcellent coverage across all platforms and configurations. Test validation is comprehensive with proper expected counts (8 XCTest + 9 Swift Testing). Recommendations SummaryMust Fix (Before Merge): None - the PR is production-ready as-is. Should Fix (Shortly After):
Nice to Have (Future):
Final VerdictRecommendation: APPROVE with minor suggestions This is a well-designed, thoroughly tested feature that significantly improves the Wasm testing experience. The automatic framework detection is smart, the implementation is robust, and the test coverage is comprehensive. The identified issues are minor and do not block merging. Quality Rating: 4.5/5
Great work on this feature! |
…s tests Updates Windows test configuration to use stable Swift 6.2-RELEASE instead of development snapshot, addressing PR review feedback. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
subrepo: subdir: "SyndiKit" merged: "c901ea0" upstream: origin: "git@github.com:brightdigit/SyndiKit.git" branch: "swift-build-v1.5.0-beta.1" commit: "c901ea0" git-subrepo: version: "0.4.9" origin: "https://github.com/Homebrew/brew" commit: "1383417817"
|
@claude suggest a better title or commit message for this PR |
|
Claude finished @leogdion's task in 54s —— View job Analyzing PR for Better Title and Commit Message
Recommended PR Title OptionsBased on the PR changes, here are three title options (from most to least preferred): Option 1: Feature-focused (Recommended)Why: Uses conventional commit format, clearly describes the main feature, and specifies the platform scope. Option 2: Detailed feature descriptionWhy: More descriptive, mentions both auto-detection and mixed framework support, but doesn't use conventional commit format. Option 3: User-focusedWhy: Emphasizes the "auto-detect" UX improvement, mentions both frameworks supported. Recommended Commit MessageIf you squash merge, here's a comprehensive commit message: Why These Recommendations?
Alternative: Keep it SimpleIf you prefer a shorter, simpler approach: Title: Commit Message: | |
leogdion
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/swift-test.yml:
- Around line 1233-1288: The Wasm matrix entries in the
test-mixed-framework-wasm job are missing the required wasm-swift-flags, causing
Wasm builds (MixedFrameworkPackage scheme, Utils.swift imports Foundation) to
fail; update every matrix include entry under strategy.matrix.include to add the
wasm-swift-flags key with the flags: -Xcc -D_WASI_EMULATED_SIGNAL -Xcc
-D_WASI_EMULATED_MMAN -Xlinker --import-memory so that ${{
matrix.wasm-swift-flags }} is populated for all Wasm runs (including wasm,
wasm-embedded, macos wasm, and wasmtime fallback entries).
🧹 Nitpick comments (5)
test/MixedFrameworkPackage/.gitignore (1)
1-8: Standard patterns look good; consider adding.cache/for local testing.The gitignore covers standard Swift/SwiftPM/Xcode artifacts appropriately. However, based on learnings about local testing workflows that use
--cache-path .cache, you might want to add.cache/to avoid committing locally cached build artifacts.📝 Optional addition
.DS_Store /.build +/.cache /Packages /*.xcodeproj xcuserdata/ DerivedData/ .swiftpm/ Package.resolved.github/workflows/swift-test.yml (2)
527-549: Windows job uses development snapshot - consider using release branch.The job pins to
swift-6.2-branchwith a development snapshot build. For stability in CI validation, consider using the release branch configuration.💡 Suggestion: Use release branch for more stable CI
- uses: ./ with: working-directory: test/MixedFrameworkPackage scheme: MixedFrameworkPackage-Package - windows-swift-version: swift-6.2-branch - windows-swift-build: 6.2-DEVELOPMENT-SNAPSHOT-2025-09-06-a + windows-swift-version: swift-6.1-release + windows-swift-build: 6.1-RELEASE
1316-1338: Address shellcheck warnings: quote variables and useread -r.Static analysis flagged issues with unquoted variables and missing
-rflag on read. These are valid concerns for robustness.🔧 Proposed fixes for shell script robustness
- name: Validate combined outputs (SPM, Xcode, Ubuntu, Windows) run: | echo "Validating combined test outputs..." - find test-outputs -type f -name "test-output.txt" | while read file; do + find test-outputs -type f -name "test-output.txt" | while read -r file; do echo "Validating: $file" - cat "$file" | ./scripts/validate-test-output.sh combined $EXPECTED_XCTEST $EXPECTED_SWIFT_TESTING + cat "$file" | ./scripts/validate-test-output.sh combined "$EXPECTED_XCTEST" "$EXPECTED_SWIFT_TESTING" done - name: Validate Wasm XCTest outputs run: | echo "Validating Wasm XCTest outputs..." - find test-outputs -type f -name "wasm-xctest-output.txt" | while read file; do + find test-outputs -type f -name "wasm-xctest-output.txt" | while read -r file; do echo "Validating: $file" - cat "$file" | ./scripts/validate-test-output.sh xctest-only $EXPECTED_XCTEST + cat "$file" | ./scripts/validate-test-output.sh xctest-only "$EXPECTED_XCTEST" done - name: Validate Wasm Swift Testing outputs run: | echo "Validating Wasm Swift Testing outputs..." - find test-outputs -type f -name "wasm-swift-testing-output.txt" | while read file; do + find test-outputs -type f -name "wasm-swift-testing-output.txt" | while read -r file; do echo "Validating: $file" - cat "$file" | ./scripts/validate-test-output.sh swift-testing-only $EXPECTED_SWIFT_TESTING + cat "$file" | ./scripts/validate-test-output.sh swift-testing-only "$EXPECTED_SWIFT_TESTING" doneaction.yml (1)
1693-1766: Consider extracting test execution logic to reduce duplication.The Wasmtime test execution block (lines 1693-1766) has significant duplication with the WasmKit block (lines 1827-1900). Both implement identical switch/case logic for
TEST_MODE.This is a nice-to-have refactor - the current implementation is functional and readable, but a helper function could reduce maintenance burden.
test/MixedFrameworkPackage/Tests/CalculatorXCTests/CalculatorXCTests.swift (1)
7-10: Consider addingtearDownfor completeness.While not strictly necessary due to ARC, adding a
tearDownmethod to nil out thecalculatoris a common XCTest best practice for symmetry and explicit cleanup.♻️ Optional cleanup
override func setUp() { super.setUp() calculator = Calculator() } + + override func tearDown() { + calculator = nil + super.tearDown() + }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.github/workflows/swift-test.ymlCLAUDE.mdaction.ymlscripts/validate-test-output.shtest/MixedFrameworkPackage/.gitignoretest/MixedFrameworkPackage/Package.swifttest/MixedFrameworkPackage/Sources/Calculator/Calculator.swifttest/MixedFrameworkPackage/Sources/Utils/Utils.swifttest/MixedFrameworkPackage/Sources/Validator/Validator.swifttest/MixedFrameworkPackage/Tests/CalculatorSwiftTestingTests/CalculatorTests.swifttest/MixedFrameworkPackage/Tests/CalculatorXCTests/CalculatorXCTests.swifttest/MixedFrameworkPackage/Tests/UtilsXCTests/UtilsXCTests.swifttest/MixedFrameworkPackage/Tests/ValidatorSwiftTestingTests/ValidatorTests.swift
🧰 Additional context used
📓 Path-based instructions (1)
**/{Package.swift,*.swift}
📄 CodeRabbit inference engine (CLAUDE.md)
For Swift packages tested locally, use swift build with --build-tests, --cache-path .cache, and --force-resolved-versions flags; then run swift test with --enable-code-coverage, --cache-path .cache, and --force-resolved-versions flags
Files:
test/MixedFrameworkPackage/Sources/Utils/Utils.swifttest/MixedFrameworkPackage/Sources/Validator/Validator.swifttest/MixedFrameworkPackage/Sources/Calculator/Calculator.swifttest/MixedFrameworkPackage/Tests/CalculatorSwiftTestingTests/CalculatorTests.swifttest/MixedFrameworkPackage/Tests/ValidatorSwiftTestingTests/ValidatorTests.swifttest/MixedFrameworkPackage/Tests/UtilsXCTests/UtilsXCTests.swifttest/MixedFrameworkPackage/Tests/CalculatorXCTests/CalculatorXCTests.swifttest/MixedFrameworkPackage/Package.swift
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: Wasm projects require explicit wasm-swift-flags input parameter configuration including WASI emulation flags and memory configuration flags; WASI emulation flags (-Xcc -D_WASI_EMULATED_SIGNAL, -Xcc -D_WASI_EMULATED_MMAN, -Xlinker flags) are required for projects using Foundation/CoreFoundation; memory flags (--initial-memory, --max-memory) are required for test suites processing large datasets
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: For WebAssembly builds, use WasmKit runtime by default (bundled with Swift 6.2.3+) for instant test execution without external downloads; optionally configure wasmtime-version parameter for Wasmtime fallback with specific version numbers in X.Y.Z format (no 'latest' support)
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: Use contains-code-coverage output from the build action to conditionally run coverage collection steps; returns 'true' for SwiftPM and Xcode builds with tests enabled, 'false' for Wasm builds, Android builds, and build-only mode
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: Applies to **/{Package.swift,*.swift} : For Swift packages tested locally, use swift build with --build-tests, --cache-path .cache, and --force-resolved-versions flags; then run swift test with --enable-code-coverage, --cache-path .cache, and --force-resolved-versions flags
📚 Learning: 2026-01-14T14:18:28.247Z
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: Applies to **/{Package.swift,*.swift} : For Swift packages tested locally, use swift build with --build-tests, --cache-path .cache, and --force-resolved-versions flags; then run swift test with --enable-code-coverage, --cache-path .cache, and --force-resolved-versions flags
Applied to files:
test/MixedFrameworkPackage/.gitignore.github/workflows/swift-test.ymlaction.ymltest/MixedFrameworkPackage/Package.swiftCLAUDE.md
📚 Learning: 2026-01-14T14:18:28.247Z
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: For Android testing, delegate to skiptools/swift-android-actionv2 which supports Swift 6.2+, API levels 28+; use android-run-tests: false for ARM macOS runners where emulator support is unavailable
Applied to files:
.github/workflows/swift-test.ymlaction.ymlCLAUDE.md
📚 Learning: 2026-01-14T14:18:28.247Z
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: For WebAssembly builds, use WasmKit runtime by default (bundled with Swift 6.2.3+) for instant test execution without external downloads; optionally configure wasmtime-version parameter for Wasmtime fallback with specific version numbers in X.Y.Z format (no 'latest' support)
Applied to files:
.github/workflows/swift-test.ymlaction.ymlCLAUDE.md
📚 Learning: 2026-01-14T14:18:28.247Z
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: For Xcode testing on macOS, use xcodebuild test with appropriate -scheme, -sdk, -destination, and -enableCodeCoverage YES parameters; adjust SDK and destination for iOS Simulator, watchOS, tvOS, and visionOS platforms
Applied to files:
.github/workflows/swift-test.ymlaction.yml
📚 Learning: 2026-01-14T14:18:28.247Z
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: Use contains-code-coverage output from the build action to conditionally run coverage collection steps; returns 'true' for SwiftPM and Xcode builds with tests enabled, 'false' for Wasm builds, Android builds, and build-only mode
Applied to files:
.github/workflows/swift-test.ymlaction.ymlCLAUDE.md
📚 Learning: 2026-01-14T14:18:28.247Z
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: Wasm projects require explicit wasm-swift-flags input parameter configuration including WASI emulation flags and memory configuration flags; WASI emulation flags (-Xcc -D_WASI_EMULATED_SIGNAL, -Xcc -D_WASI_EMULATED_MMAN, -Xlinker flags) are required for projects using Foundation/CoreFoundation; memory flags (--initial-memory, --max-memory) are required for test suites processing large datasets
Applied to files:
action.ymlCLAUDE.md
📚 Learning: 2026-01-14T14:18:28.247Z
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: GitHub Action wasm-swift-flags input parameter is interpolated directly into shell commands without sanitization; ensure values are properly validated before passing if used in reusable workflows accepting external inputs; never pass untrusted user input directly to this parameter
Applied to files:
action.ymlCLAUDE.md
📚 Learning: 2026-01-14T14:18:28.247Z
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: WebAssembly builds do NOT support code coverage; use the contains-code-coverage output to conditionally skip coverage collection for Wasm builds
Applied to files:
action.ymlCLAUDE.md
🧬 Code graph analysis (4)
test/MixedFrameworkPackage/Sources/Validator/Validator.swift (1)
test/MixedFrameworkPackage/Sources/Utils/Utils.swift (1)
trimWhitespace(4-6)
test/MixedFrameworkPackage/Tests/CalculatorSwiftTestingTests/CalculatorTests.swift (2)
test/MixedFrameworkPackage/Tests/CalculatorXCTests/CalculatorXCTests.swift (5)
testAddition(12-15)testSubtraction(17-20)testMultiplication(22-25)testDivision(27-30)testDivisionByZero(32-34)test/MixedFrameworkPackage/Sources/Calculator/Calculator.swift (4)
add(4-6)subtract(8-10)multiply(12-14)divide(16-19)
test/MixedFrameworkPackage/Tests/ValidatorSwiftTestingTests/ValidatorTests.swift (1)
test/MixedFrameworkPackage/Sources/Validator/Validator.swift (2)
validateEmail(6-9)validateAge(11-13)
test/MixedFrameworkPackage/Tests/CalculatorXCTests/CalculatorXCTests.swift (1)
test/MixedFrameworkPackage/Sources/Calculator/Calculator.swift (4)
add(4-6)subtract(8-10)multiply(12-14)divide(16-19)
🪛 actionlint (1.7.10)
.github/workflows/swift-test.yml
318-318: property "download-platform" is not defined in object type {devicename: string; osversion: number; runs-on: string; type: string; use-xcbeautify: bool; xcode: string}
(expression)
325-325: property "build-only" is not defined in object type {devicename: string; osversion: number; runs-on: string; type: string; use-xcbeautify: bool; xcode: string}
(expression)
1274-1274: property "wasm-swift-flags" is not defined in object type {build-only: bool; container: string; runs-on: string; type: string; wasmtime-version: string; xcode: string}
(expression)
1317-1317: shellcheck reported issue in this script: SC2086:info:4:60: Double quote to prevent globbing and word splitting
(shellcheck)
1317-1317: shellcheck reported issue in this script: SC2086:info:4:77: Double quote to prevent globbing and word splitting
(shellcheck)
1317-1317: shellcheck reported issue in this script: SC2162:info:2:59: read without -r will mangle backslashes
(shellcheck)
1325-1325: shellcheck reported issue in this script: SC2086:info:4:63: Double quote to prevent globbing and word splitting
(shellcheck)
1325-1325: shellcheck reported issue in this script: SC2162:info:2:66: read without -r will mangle backslashes
(shellcheck)
1333-1333: shellcheck reported issue in this script: SC2086:info:4:70: Double quote to prevent globbing and word splitting
(shellcheck)
1333-1333: shellcheck reported issue in this script: SC2162:info:2:73: read without -r will mangle backslashes
(shellcheck)
🪛 Shellcheck (0.11.0)
scripts/validate-test-output.sh
[warning] 45-45: YELLOW appears unused. Verify use (or export if used externally).
(SC2034)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: Test Single Target Package (macOS) (watchos, macos-26, /Applications/Xcode_26.2.app, Apple Watch ...
- GitHub Check: Test Single Target Package (Wasm) (macos-26, /Applications/Xcode_26.2.app, wasm, false, -Xcc -D_W...
- GitHub Check: Test Single Target Package (macOS) (watchos, macos-26, /Applications/Xcode_26.2.app, Apple Watch ...
- GitHub Check: Test Single Target Package (Wasm) (macos-26, /Applications/Xcode_26.2.app, wasm, false, -Xcc -D_W...
- GitHub Check: Test Single Target Package (macOS) (watchos, macos-26, /Applications/Xcode_26.2.app, Apple Watch ...
- GitHub Check: Test Single Target Package (Wasm) (macos-26, /Applications/Xcode_26.2.app, wasm, false, -Xcc -D_W...
- GitHub Check: Test Single Target Package (macOS) (watchos, macos-26, /Applications/Xcode_26.2.app, Apple Watch ...
- GitHub Check: Test Single Target Package (Wasm) (macos-26, /Applications/Xcode_26.2.app, wasm, false, -Xcc -D_W...
- GitHub Check: Test Single Target Package (macOS) (watchos, macos-26, /Applications/Xcode_26.2.app, Apple Watch ...
- GitHub Check: Test Single Target Package (Wasm) (macos-26, /Applications/Xcode_26.2.app, wasm, false, -Xcc -D_W...
- GitHub Check: Test Single Target Package (macOS) (watchos, macos-26, /Applications/Xcode_26.2.app, Apple Watch ...
- GitHub Check: Test Single Target Package (Wasm) (macos-26, /Applications/Xcode_26.2.app, wasm, false, -Xcc -D_W...
- GitHub Check: Test Single Target Package (macOS) (watchos, macos-26, /Applications/Xcode_26.2.app, Apple Watch ...
- GitHub Check: Test Single Target Package (Wasm) (macos-26, /Applications/Xcode_26.2.app, wasm, false, -Xcc -D_W...
- GitHub Check: Test Single Target Package (macOS) (watchos, macos-26, /Applications/Xcode_26.2.app, Apple Watch ...
- GitHub Check: Test Single Target Package (Wasm) (macos-26, /Applications/Xcode_26.2.app, wasm, false, -Xcc -D_W...
- GitHub Check: Test Single Target Package (macOS) (watchos, macos-26, /Applications/Xcode_26.2.app, Apple Watch ...
- GitHub Check: Test Single Target Package (Wasm) (macos-26, /Applications/Xcode_26.2.app, wasm, false, -Xcc -D_W...
- GitHub Check: Test Single Target Package (macOS) (watchos, macos-26, /Applications/Xcode_26.2.app, Apple Watch ...
- GitHub Check: Test Single Target Package (Wasm) (macos-26, /Applications/Xcode_26.2.app, wasm, false, -Xcc -D_W...
🔇 Additional comments (26)
.github/workflows/swift-test.yml (4)
290-330: LGTM - New macOS mixed-framework test job.The job structure follows the established pattern from other macOS test jobs. The matrix is appropriately minimal (one SwiftPM and one iOS configuration) as per the past review comment about not needing all platforms for testing output.
Minor note: The
download-platformandbuild-onlyproperties referenced in lines 318 and 325 are not defined in the matrix include entries, but this is acceptable since they default to falsy values which is the intended behavior.
428-465: LGTM - Ubuntu mixed-framework test job follows established patterns.The job correctly mirrors the multi-target Ubuntu job structure with appropriate Swift version exclusions for nightly builds. The conditional upload (line 460) ensures only one artifact is created per platform.
790-823: LGTM - Android mixed-framework job is well-structured.The job follows the established pattern for Android testing with appropriate disk space cleanup. Single matrix entry is sufficient for mixed-framework validation.
1290-1351: LGTM - Validation job provides comprehensive cross-platform verification.The validation job correctly:
- Depends on all mixed-framework test jobs
- Uses
if: always()to run even if some tests failed- Downloads artifacts by pattern and validates each mode separately
- Provides helpful summary output
action.yml (6)
207-227: LGTM - New Wasm testing inputs are well-documented.The
wasm-testing-libraryandwasm-swift-test-flagsinputs follow the established documentation pattern with clear descriptions, valid values, and security considerations mentioned.
299-308: LGTM -save-test-outputinput enables test validation workflows.The input is well-documented with clear file locations and use cases. The default
'false'maintains backward compatibility.
1498-1555: LGTM - Wasm testing framework detection logic is sound.The detection step correctly:
- Handles the
automode by scanning for import statements- Provides clear logging for each detected framework
- Falls back gracefully when Tests/ directory is missing
- Outputs the appropriate mode for downstream consumption
1827-1901: LGTM - WasmKit test execution with framework detection.The implementation correctly handles all testing modes (swift-testing, xctest, both, none) with appropriate output capture when enabled. The exit code handling for 'both' mode properly fails if either framework's tests fail.
1965-2005: LGTM - Xcode build step updated for test output saving.The changes properly integrate output saving with xcbeautify piping using
tee, and handle both xcbeautify and non-xcbeautify paths correctly.
1399-1416: LGTM - SPM build step updated for test output saving.The implementation correctly configures output file path and uses
teewithset -o pipefailto capture output while preserving exit codes.scripts/validate-test-output.sh (5)
1-47: LGTM - Script header and color setup are well-structured.The script has clear documentation, proper
set -euo pipefailfor safety, and correct terminal detection for color support.The
YELLOWvariable (line 40/45) is defined but unused - this is intentional for future warning messages and can be safely ignored.
90-97: LGTM - Swift Testing parser correctly sums all test runs.The function handles multiple test runs by summing all matches, which is the correct behavior for multi-target packages.
106-143: LGTM - Combined validation provides clear feedback.The
validate_combinedfunction correctly validates both counts and total, with helpful color-coded output for each component.
193-311: LGTM - Main function with comprehensive argument handling.The entry point correctly:
- Validates required arguments per mode
- Handles the flexible argument position for swift-testing-only (lines 237-238)
- Provides helpful usage messages
- Routes to appropriate validators
55-81: The edge case handling is correct and robust.The
parse_xctest_countfunction properly handles all-zero test counts in both code paths. When using the simple path,grep -v '^0$'filters zeros and returns empty, but the fallback${count:-0}safely converts it to "0". In the xcodebuild path,awk '{sum+=$1} END {print sum+0}'correctly sums to 0 without filtering. No unexpected behavior occurs in either scenario.CLAUDE.md (2)
80-94: LGTM - Testing Framework Detection documentation is clear and accurate.The documentation correctly explains the auto-detection behavior and provides a practical example showing both
wasm-testing-libraryandwasm-swift-test-flagsusage.
126-137: LGTM - Input documentation aligned with action.yml.The new inputs (
wasm-testing-library,wasm-swift-test-flags) are accurately documented with all valid values and security considerations appropriately extended to cover both flag parameters.test/MixedFrameworkPackage/Sources/Utils/Utils.swift (1)
1-11: LGTM - Simple utility struct for test package.The
Utilsstruct provides basic string utilities using standard Foundation APIs. The implementation is correct and appropriate for a test fixture demonstrating multi-target package testing.Note:
trimWhitespaceuses.whitespaceswhich excludes newlines. If newline trimming is needed, consider.whitespacesAndNewlines. For test fixture purposes, current behavior is fine.test/MixedFrameworkPackage/Sources/Validator/Validator.swift (1)
1-14: LGTM!The
Validatorstruct provides a clean public API with appropriate visibility. The email validation is intentionally simple (checking for@and.presence after trimming), which is suitable for this test package's purpose of validating mixed-framework detection. The age validation correctly uses an inclusive range check.test/MixedFrameworkPackage/Sources/Calculator/Calculator.swift (1)
1-20: LGTM!The
Calculatorimplementation is correct for its purpose as a test fixture. Thedividemethod properly handles division by zero by returningnil. Integer overflow is not handled, but this is acceptable for a test package designed to validate framework detection rather than production arithmetic.test/MixedFrameworkPackage/Tests/CalculatorSwiftTestingTests/CalculatorTests.swift (1)
1-40: LGTM!Well-structured Swift Testing suite with good coverage of all
Calculatoroperations. The test values complement the XCTest variant (different inputs), and the#expectassertions correctly validate the expected behavior including thenilresult for division by zero.test/MixedFrameworkPackage/Tests/CalculatorXCTests/CalculatorXCTests.swift (1)
4-40: LGTM!Solid XCTest suite with comprehensive coverage including edge cases. The
setUppattern correctly initializes the calculator before each test. The edge case testingInt.max + 0is a safe boundary check.test/MixedFrameworkPackage/Tests/UtilsXCTests/UtilsXCTests.swift (1)
11-16: Test expectations are correct. TheUtils.isEmptyimplementation does trim whitespace before checking emptiness, as the test assumes. All assertions intestIsEmpty()accurately reflect the expected behavior of theisEmptymethod.test/MixedFrameworkPackage/Package.swift (2)
6-8: Consider cross-platform compatibility for this test fixture.The package is restricted to macOS v13+, but the PR objective mentions testing on Ubuntu, Windows, Android, and Wasm. If this package is intended to validate mixed-framework detection across all these platforms, you may want to either:
- Remove the
platformsconstraint entirely to allow broader compatibility, or- Add additional platform minimums (e.g.,
.iOS(.v16),.tvOS(.v16),.watchOS(.v9))If the macOS-only restriction is intentional for this specific test fixture, this comment can be disregarded.
14-39: LGTM! Well-structured test target organization.The separation of Swift Testing and XCTest targets is clear and properly demonstrates the mixed-framework scenario. The naming convention (
*SwiftTestingTestsvs*XCTests) makes the framework distinction immediately apparent, which will help validate the auto-detection logic.test/MixedFrameworkPackage/Tests/ValidatorSwiftTestingTests/ValidatorTests.swift (1)
1-35: LGTM! Comprehensive test coverage with correct assertions.The test suite properly exercises both email and age validation with good boundary coverage:
- Email tests cover valid formats, whitespace handling, and various invalid patterns
- Age tests cover boundaries (0, 150), mid-range, and out-of-range values
The Swift Testing syntax (
@Suite,@Test,#expect) is used correctly.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| test-mixed-framework-wasm: | ||
| name: Test Mixed-Framework Package (Wasm) | ||
| runs-on: ${{ matrix.runs-on }} | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| include: | ||
| # WasmKit (default) - Ubuntu with Wasm | ||
| - runs-on: ubuntu-latest | ||
| container: swift:6.2-jammy | ||
| type: wasm | ||
| build-only: false | ||
|
|
||
| # WasmKit (default) - Ubuntu with Wasm-embedded | ||
| - runs-on: ubuntu-latest | ||
| container: swift:6.2-jammy | ||
| type: wasm-embedded | ||
| build-only: false | ||
|
|
||
| # WasmKit (default) - macOS 26 with Wasm | ||
| - runs-on: macos-26 | ||
| xcode: "/Applications/Xcode_26.2.app" | ||
| type: wasm | ||
| build-only: false | ||
|
|
||
| # Wasmtime fallback - Ubuntu with WASM | ||
| - runs-on: ubuntu-latest | ||
| container: swift:6.2-jammy | ||
| type: wasm | ||
| wasmtime-version: '27.0.0' | ||
| build-only: false | ||
| container: ${{ matrix.container }} | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: ./ | ||
| with: | ||
| working-directory: test/MixedFrameworkPackage | ||
| scheme: MixedFrameworkPackage-Package | ||
| type: ${{ matrix.type }} | ||
| xcode: ${{ matrix.xcode }} | ||
| skip-package-resolved: "true" | ||
| wasm-swift-flags: ${{ matrix.wasm-swift-flags }} | ||
| wasmtime-version: ${{ matrix.wasmtime-version }} | ||
| build-only: ${{ matrix.build-only }} | ||
| save-test-output: 'true' | ||
|
|
||
| - name: Upload test output | ||
| # Only upload for Ubuntu + wasm + WasmKit (no wasmtime-version) | ||
| if: matrix.build-only != true && matrix.runs-on == 'ubuntu-latest' && matrix.type == 'wasm' && !matrix.wasmtime-version | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: test-output-wasm | ||
| path: | | ||
| ${{ runner.temp }}/wasm-xctest-output.txt | ||
| ${{ runner.temp }}/wasm-swift-testing-output.txt | ||
| retention-days: 1 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if MixedFrameworkPackage uses Foundation or other libraries requiring WASI flags
echo "Checking for Foundation imports in MixedFrameworkPackage:"
rg -n "import Foundation" test/MixedFrameworkPackage/ --type swift || echo "No Foundation imports found"
echo ""
echo "Checking for CoreFoundation imports:"
rg -n "import CoreFoundation" test/MixedFrameworkPackage/ --type swift || echo "No CoreFoundation imports found"
echo ""
echo "Checking Swift files in MixedFrameworkPackage:"
fd . test/MixedFrameworkPackage/ --type f --extension swiftRepository: brightdigit/swift-build
Length of output: 837
Add missing wasm-swift-flags to Wasm matrix entries.
MixedFrameworkPackage imports Foundation (line 1 of Utils.swift), which requires WASI emulation flags for Wasm builds. All matrix entries should define wasm-swift-flags with the required flags. Update each matrix entry to include:
wasm-swift-flags: -Xcc -D_WASI_EMULATED_SIGNAL -Xcc -D_WASI_EMULATED_MMAN -Xlinker --import-memoryCurrently, ${{ matrix.wasm-swift-flags }} resolves to an empty string, which will cause the Wasm builds to fail.
🧰 Tools
🪛 actionlint (1.7.10)
1274-1274: property "wasm-swift-flags" is not defined in object type {build-only: bool; container: string; runs-on: string; type: string; wasmtime-version: string; xcode: string}
(expression)
🤖 Prompt for AI Agents
In @.github/workflows/swift-test.yml around lines 1233 - 1288, The Wasm matrix
entries in the test-mixed-framework-wasm job are missing the required
wasm-swift-flags, causing Wasm builds (MixedFrameworkPackage scheme, Utils.swift
imports Foundation) to fail; update every matrix include entry under
strategy.matrix.include to add the wasm-swift-flags key with the flags: -Xcc
-D_WASI_EMULATED_SIGNAL -Xcc -D_WASI_EMULATED_MMAN -Xlinker --import-memory so
that ${{ matrix.wasm-swift-flags }} is populated for all Wasm runs (including
wasm, wasm-embedded, macos wasm, and wasmtime fallback entries).
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@action.yml`:
- Around line 1828-1901: Multiple test binaries are overwriting
SWIFT_TESTING_OUTPUT and XCTEST_OUTPUT because the loop writes the same file for
each TEST_BINARY; update the wasmkit invocation in the swift-testing, xctest and
both branches (where SWIFT_TESTING_OUTPUT and XCTEST_OUTPUT are used) to either
append instead of overwrite (use tee -a) or create per-binary output files
(e.g., derive a unique name from TEST_BINARY) so each TEST_BINARY's output is
preserved; adjust the success/failure capture logic (SWIFT_TESTING_EXIT,
XCTEST_EXIT) to reference the per-run outputs if you generate unique names.
- Around line 1694-1767: The loop over TEST_BINARY currently uses tee to write
to SWIFT_TESTING_OUTPUT and XCTEST_OUTPUT which overwrites files for each
binary; change all tee invocations in the while IFS= read -r TEST_BINARY loop to
append mode (use tee -a) so outputs from multiple binaries are preserved, and
ensure SWIFT_TESTING_OUTPUT/XCTEST_OUTPUT are either initialized/removed before
the loop if you want fresh files for the run; update every branch
(swift-testing, xctest, both) where tee "$SWIFT_TESTING_OUTPUT" or tee
"$XCTEST_OUTPUT" is used and leave non-tee invocations unchanged.
♻️ Duplicate comments (1)
.github/workflows/swift-test.yml (1)
1223-1278: Missingwasm-swift-flagsin matrix entries - builds will fail.The static analysis correctly identifies that
wasm-swift-flagsis not defined in the matrix object type. Since MixedFrameworkPackage imports Foundation (per past review analysis), all Wasm matrix entries require WASI emulation flags. Currently${{ matrix.wasm-swift-flags }}at line 1264 resolves to empty.🔧 Add wasm-swift-flags to each matrix entry
include: # WasmKit (default) - Ubuntu with Wasm - runs-on: ubuntu-latest container: swift:6.2-jammy type: wasm build-only: false + wasm-swift-flags: >- + -Xcc -D_WASI_EMULATED_SIGNAL + -Xcc -D_WASI_EMULATED_MMAN + -Xlinker -lwasi-emulated-signal + -Xlinker -lwasi-emulated-mman + -Xlinker -lwasi-emulated-getpid # WasmKit (default) - Ubuntu with Wasm-embedded - runs-on: ubuntu-latest container: swift:6.2-jammy type: wasm-embedded build-only: false + wasm-swift-flags: >- + -Xcc -D_WASI_EMULATED_SIGNAL + -Xcc -D_WASI_EMULATED_MMAN + -Xlinker -lwasi-emulated-signal + -Xlinker -lwasi-emulated-mman + -Xlinker -lwasi-emulated-getpid # WasmKit (default) - macOS 26 with Wasm - runs-on: macos-26 xcode: "/Applications/Xcode_26.2.app" type: wasm build-only: false + wasm-swift-flags: >- + -Xcc -D_WASI_EMULATED_SIGNAL + -Xcc -D_WASI_EMULATED_MMAN + -Xlinker -lwasi-emulated-signal + -Xlinker -lwasi-emulated-mman + -Xlinker -lwasi-emulated-getpid # Wasmtime fallback - Ubuntu with WASM - runs-on: ubuntu-latest container: swift:6.2-jammy type: wasm wasmtime-version: '27.0.0' build-only: false + wasm-swift-flags: >- + -Xcc -D_WASI_EMULATED_SIGNAL + -Xcc -D_WASI_EMULATED_MMAN + -Xlinker -lwasi-emulated-signal + -Xlinker -lwasi-emulated-mman + -Xlinker -lwasi-emulated-getpidBased on learnings, Wasm projects using Foundation/CoreFoundation require explicit WASI emulation flags.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/swift-test.ymlaction.yml
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: For WebAssembly builds, use WasmKit runtime by default (bundled with Swift 6.2.3+) for instant test execution without external downloads; optionally configure wasmtime-version parameter for Wasmtime fallback with specific version numbers in X.Y.Z format (no 'latest' support)
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: Wasm projects require explicit wasm-swift-flags input parameter configuration including WASI emulation flags and memory configuration flags; WASI emulation flags (-Xcc -D_WASI_EMULATED_SIGNAL, -Xcc -D_WASI_EMULATED_MMAN, -Xlinker flags) are required for projects using Foundation/CoreFoundation; memory flags (--initial-memory, --max-memory) are required for test suites processing large datasets
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: For Android testing, delegate to skiptools/swift-android-actionv2 which supports Swift 6.2+, API levels 28+; use android-run-tests: false for ARM macOS runners where emulator support is unavailable
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: Use contains-code-coverage output from the build action to conditionally run coverage collection steps; returns 'true' for SwiftPM and Xcode builds with tests enabled, 'false' for Wasm builds, Android builds, and build-only mode
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: For Xcode testing on macOS, use xcodebuild test with appropriate -scheme, -sdk, -destination, and -enableCodeCoverage YES parameters; adjust SDK and destination for iOS Simulator, watchOS, tvOS, and visionOS platforms
📚 Learning: 2026-01-14T14:18:28.247Z
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: Applies to **/{Package.swift,*.swift} : For Swift packages tested locally, use swift build with --build-tests, --cache-path .cache, and --force-resolved-versions flags; then run swift test with --enable-code-coverage, --cache-path .cache, and --force-resolved-versions flags
Applied to files:
.github/workflows/swift-test.ymlaction.yml
📚 Learning: 2026-01-14T14:18:28.247Z
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: For Android testing, delegate to skiptools/swift-android-actionv2 which supports Swift 6.2+, API levels 28+; use android-run-tests: false for ARM macOS runners where emulator support is unavailable
Applied to files:
.github/workflows/swift-test.ymlaction.yml
📚 Learning: 2026-01-14T14:18:28.247Z
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: For WebAssembly builds, use WasmKit runtime by default (bundled with Swift 6.2.3+) for instant test execution without external downloads; optionally configure wasmtime-version parameter for Wasmtime fallback with specific version numbers in X.Y.Z format (no 'latest' support)
Applied to files:
.github/workflows/swift-test.ymlaction.yml
📚 Learning: 2026-01-14T14:18:28.247Z
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: Wasm projects require explicit wasm-swift-flags input parameter configuration including WASI emulation flags and memory configuration flags; WASI emulation flags (-Xcc -D_WASI_EMULATED_SIGNAL, -Xcc -D_WASI_EMULATED_MMAN, -Xlinker flags) are required for projects using Foundation/CoreFoundation; memory flags (--initial-memory, --max-memory) are required for test suites processing large datasets
Applied to files:
.github/workflows/swift-test.ymlaction.yml
📚 Learning: 2026-01-14T14:18:28.247Z
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: GitHub Action wasm-swift-flags input parameter is interpolated directly into shell commands without sanitization; ensure values are properly validated before passing if used in reusable workflows accepting external inputs; never pass untrusted user input directly to this parameter
Applied to files:
.github/workflows/swift-test.ymlaction.yml
📚 Learning: 2026-01-14T14:18:28.247Z
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: For Xcode testing on macOS, use xcodebuild test with appropriate -scheme, -sdk, -destination, and -enableCodeCoverage YES parameters; adjust SDK and destination for iOS Simulator, watchOS, tvOS, and visionOS platforms
Applied to files:
.github/workflows/swift-test.ymlaction.yml
📚 Learning: 2026-01-14T14:18:28.247Z
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: Use contains-code-coverage output from the build action to conditionally run coverage collection steps; returns 'true' for SwiftPM and Xcode builds with tests enabled, 'false' for Wasm builds, Android builds, and build-only mode
Applied to files:
.github/workflows/swift-test.ymlaction.yml
📚 Learning: 2026-01-14T14:18:28.247Z
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: WebAssembly builds do NOT support code coverage; use the contains-code-coverage output to conditionally skip coverage collection for Wasm builds
Applied to files:
action.yml
🪛 actionlint (1.7.10)
.github/workflows/swift-test.yml
318-318: property "download-platform" is not defined in object type {devicename: string; osversion: number; runs-on: string; type: string; use-xcbeautify: bool; xcode: string}
(expression)
325-325: property "build-only" is not defined in object type {devicename: string; osversion: number; runs-on: string; type: string; use-xcbeautify: bool; xcode: string}
(expression)
1264-1264: property "wasm-swift-flags" is not defined in object type {build-only: bool; container: string; runs-on: string; type: string; wasmtime-version: string; xcode: string}
(expression)
🔇 Additional comments (12)
.github/workflows/swift-test.yml (5)
290-330: LGTM - macOS mixed-framework test job well structured.The job correctly sets up testing for both SwiftPM and Xcode (iOS) configurations with appropriate matrix entries. The conditional upload logic at line 325 correctly filters for representative outputs.
Note: The static analysis warning about
download-platformandbuild-onlynot being defined in the matrix is a false positive - these properties are intentionally not defined because they should default to falsy values, and the workflow correctly uses|| 'false'fallbacks at lines 318 and 325.
428-465: LGTM - Ubuntu mixed-framework test job.The job mirrors the existing multi-target Ubuntu job structure correctly with appropriate Swift version and distro matrix. The upload condition at line 460 properly limits artifact uploads to a single representative configuration.
519-543: LGTM - Windows mixed-framework test job.The job is correctly configured with Swift 6.2.3 on windows-2025. Good that it uses the release branch as previously requested.
780-813: LGTM - Android mixed-framework test job.The job correctly mirrors the existing Android test job structure with appropriate parameters for Swift 6.2 and API level 28.
1280-1340: LGTM - Validation job structure is sound.The
validate-mixed-framework-test-countsjob correctly:
- Depends on all mixed-framework test jobs
- Uses
if: always()to run even on partial failures- Downloads artifacts with pattern matching
- Validates combined and Wasm-specific outputs separately
- Uses environment variables for expected test counts
One observation: The expected counts (8 XCTest, 9 Swift Testing) are hardcoded as environment variables, which is appropriate for this validation use case.
action.yml (7)
208-228: LGTM - New input parameters well documented.The
wasm-testing-libraryandwasm-swift-test-flagsinputs are clearly documented with valid values and appropriate defaults. The security note aboutwasm-swift-test-flagscorrectly warns against untrusted input.
300-309: LGTM - Save test output input well documented.The
save-test-outputparameter clearly documents the output file locations for different platforms (SPM/Xcode/Android vs Wasm), enabling downstream validation workflows.
1400-1417: LGTM - SPM test output capture implementation.The output capture logic correctly:
- Sets up
OUTPUT_FILEconditionally- Uses
set -o pipefailto preserve exit codes through the pipe- Only captures output when
save-test-outputis enabled
1966-2006: LGTM - Xcode test output capture implementation.The output capture for Xcode builds correctly handles both xcbeautify and non-xcbeautify paths, and properly excludes build-only mode from output capture.
1730-1754: Exit code handling in 'both' mode is correct.The implementation properly captures exit codes from both framework runs using
|| SWIFT_TESTING_EXIT=$?and|| XCTEST_EXIT=$?, then fails if either is non-zero. This ensures both test suites must pass.
883-887: LGTM - Documentation update for Windows Swift version format.The comment correctly reflects the new patch version naming convention (e.g.,
6.2.3-RELEASEvs6.1-RELEASE).
1499-1556: I need the review comment and repository context to proceed. Please provide:
- The original review comment to rewrite
- The code file or repository context
- Any relevant file paths or code snippets mentioned in the comment
Once you provide these, I'll verify the concerns and generate the rewritten comment in the required format.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Code Review: Swift Testing Auto-Detection for WasmCompleted a thorough review. This is a well-designed feature with intelligent testing framework detection. Strengths1. Excellent Architecture
2. Comprehensive Test Coverage
3. Security Awareness
4. Backward Compatibility
HIGH PRIORITY Issues1. Shell Injection Risk (action.yml:1707-1767) 2. Output File Collision (action.yml:1695-1698, 1829-1832) 3. Missing Validation for wasm-swift-test-flags (action.yml:1702, 1836) MEDIUM PRIORITY Issues4. Error Handling in both Mode (action.yml:1730-1755) 5. Test Validation Parsing Fragility (scripts/validate-test-output.sh:61-77) 6. Missing Documentation for none Mode Overall AssessmentGrade: A- Excellent work with a few important fixes needed. Before Merging - Address HIGH PRIORITY issues:
After Merging - Consider follow-ups:
Great work! This will significantly improve the Wasm testing experience. |
This commit addresses 6 priority issues identified in the Claude code review for Swift Testing auto-detection and mixed framework support (PR #78). HIGH PRIORITY FIXES: - Shell Injection Risk: Add input validation for wasm-testing-library parameter to fail fast on invalid values before case statement execution - Output File Collision: Use tee -a (append mode) to preserve output from all test binaries in multi-target packages, with file clearing before loop - Unquoted Variables: Quote $EXTRA_FLAGS in all 18 test command locations to prevent word splitting when flags contain spaces MEDIUM PRIORITY FIXES: - Pipefail State: Add set +o pipefail after each test command (8 locations) to reset shell state and prevent interference with subsequent commands - Parsing Fragility: Replace threshold-based xcodebuild detection (>2 count) with explicit "Selected tests" format marker for robust format detection - Documentation: Expand "none" mode documentation with use cases, examples, and guidance for when to use custom test harnesses All changes improve defensive coding practices and are localized to Wasm-specific code paths, minimizing risk to existing functionality. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
… refactoring) This commit addresses three issues identified in the PR #78 review: PHASE 1: iOS Validation Fix (Blocking Issue) - Fixed validate-test-output.sh to correctly detect iOS xcodebuild format - iOS output lacks "Selected tests" marker, causing validation to fail - Added hybrid detection: explicit markers OR threshold-based (≥2 "All tests") - iOS tests now correctly report 8 XCTests instead of 2 PHASE 2: Wasm Refactoring 1. Extract Test Execution Function - Created run_wasm_tests() shell function to eliminate ~200 lines of duplication - Function accepts runtime command parameter ("$WASMTIME_BIN" or "wasmkit run") - Reduced net lines by ~50 (108 added, 158 removed) - Single source of truth for test execution logic 2. Document Pipefail Pattern - Added comprehensive inline comments explaining set -o pipefail usage - Documents why it's needed (catch test failures with tee) - Documents why it's toggled (avoid affecting other pipelines) - Improves maintainability for future developers Technical Details: - Wasmtime path: replaced lines with function call + setup (103 lines → 33 lines) - WasmKit path: replaced lines with function call + setup (106 lines → 33 lines) - Shared function: ~100 lines (includes documentation and all 4 test modes) - Net impact: -50 lines total, significantly improved maintainability Files Modified: - action.yml: Wasm test execution refactoring - scripts/validate-test-output.sh: iOS validation fix Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
PR Review: Swift Testing Framework Auto-Detection for WasmThis PR adds comprehensive support for mixed XCTest/Swift Testing frameworks with automatic detection. Overall quality is HIGH with EXCELLENT completeness. Key Strengths
Critical Issues to Address1. Input Sanitization (Medium Priority - Security)Location: action.yml:1784-1809, 1821, 1615-1631 wasm-swift-flags and wasm-swift-test-flags are interpolated into shell commands without validation. While currently sourced from YAML, reusable workflows could introduce injection risks. Recommendation: Add input validation regex and consistent variable quoting 2. Bash Script Robustness (High Priority)Location: scripts/validate-test-output.sh:55-85 The xcodebuild detection uses hybrid approach (threshold-based for iOS) that could match false positives with corrupted output. Recommendation: Add output length validation and document iOS format differences 3. Error Handling (Medium Priority)Location: action.yml:1602-1633 run_wasm_tests() does not validate test binaries exist before execution. Recommendation: Add file existence check 4. Framework Detection Edge Cases (Low Priority)Location: action.yml:1534-1547 Simple grep does not handle commented imports, string literals, or conditional compilation. Impact: Low but should be documented Additional RecommendationsHigh Priority:
Medium Priority:
Low Priority:
Overall AssessmentQuality: High | Completeness: Excellent | Risk: Medium Well-architected feature solving a real problem elegantly. Main concerns are input validation and bash robustness. With recommended changes, this is production-ready. Recommendation: Approve with requested changes Great work on the comprehensive testing and validation framework! |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
action.yml (1)
1696-1732: Wasmtime platform selection ignores CPU architecture.
On ARM runners this will fail because it downloads x86_64 artifacts (wasmtime--x86_64-linux.tar.xz, wasmtime--x86_64-macos.tar.xz) when aarch64 variants exist (wasmtime--aarch64-linux.tar.xz, wasmtime--aarch64-macos.tar.xz). Detect CPU architecture withuname -mand map to the correct platform.🔧 Proposed fix (arch-aware platform selection)
+ # Detect CPU architecture to select the correct Wasmtime artifact + ARCH_RAW=$(uname -m) + case "$ARCH_RAW" in + x86_64|amd64) ARCH_TAG="x86_64" ;; + arm64|aarch64) ARCH_TAG="aarch64" ;; + *) echo "ERROR: Unsupported CPU architecture: $ARCH_RAW" >&2; exit 1 ;; + esac + # Method 1: Use $OSTYPE if available (bash built-in, fastest) if [[ -n "$OSTYPE" ]]; then if [[ "$OSTYPE" == "linux-gnu"* ]] || [[ "$OSTYPE" == "linux"* ]]; then - WASMTIME_PLATFORM="x86_64-linux" + WASMTIME_PLATFORM="${ARCH_TAG}-linux" elif [[ "$OSTYPE" == "darwin"* ]]; then - WASMTIME_PLATFORM="x86_64-macos" + WASMTIME_PLATFORM="${ARCH_TAG}-macos" fi fi- WASMTIME_PLATFORM="x86_64-linux" + WASMTIME_PLATFORM="${ARCH_TAG}-linux"- WASMTIME_PLATFORM="x86_64-macos" + WASMTIME_PLATFORM="${ARCH_TAG}-macos"Alternatively, consider using WasmKit (bundled with Swift 6.2.3+) for broader platform support without external downloads.
🤖 Fix all issues with AI agents
In `@action.yml`:
- Around line 1533-1544: The current grep checks that set HAS_SWIFT_TESTING and
HAS_XCTEST can match substrings like "TestingSupport" or "XCTestUtils"; update
the two grep invocations that detect "import Testing" and "import XCTest" to
anchor the module name so it only matches the exact import token (e.g., use
extended regex with a word-boundary after the module name such as grep -r -E
'import[[:space:]]+Testing\>' and grep -r -E 'import[[:space:]]+XCTest\>'
respectively), leaving the rest of the logic around HAS_SWIFT_TESTING and
HAS_XCTEST unchanged.
In `@scripts/validate-test-output.sh`:
- Around line 68-74: The parsing currently greps for "Test Suite 'All tests'
passed" which skips bundle lines when tests fail; update the grep in the count
calculation to match "Test Suite 'All tests'" (no "passed") so both passing and
failing totals are captured, e.g. change the pattern used in the pipeline that
computes count (the grep -A1 "Test Suite 'All tests' passed") to grep -A1 "Test
Suite 'All tests'"; keep the rest of the pipeline (grep -o 'Executed [0-9]\+
tests' | grep -o '[0-9]\+' | awk '{sum+=$1} END {print sum+0}') and the
variables count, all_tests_count, and input unchanged.
🧹 Nitpick comments (2)
scripts/validate-test-output.sh (1)
36-46: Remove unused YELLOW constant (shellcheck SC2034).
It isn’t referenced anywhere.♻️ Proposed cleanup
- readonly YELLOW='\033[1;33m' readonly NC='\033[0m' # No Color- readonly YELLOW='' readonly NC=''CLAUDE.md (1)
175-194: Consider making staleness risk more explicit or consolidating version details.While the current version information is accurate, version tables become outdated quickly. The file already links to xcodereleases.com and notes to "refer to the live data," but you could strengthen this by:
- Adding a "Last verified" date to the version block, or
- Reducing specific version details in the main content and relying more heavily on the reference link as the source of truth.
This ensures readers understand the snapshot nature of the table without needing to assume staleness.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CLAUDE.mdaction.ymlscripts/validate-test-output.sh
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: For Android testing, delegate to skiptools/swift-android-actionv2 which supports Swift 6.2+, API levels 28+; use android-run-tests: false for ARM macOS runners where emulator support is unavailable
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: Use contains-code-coverage output from the build action to conditionally run coverage collection steps; returns 'true' for SwiftPM and Xcode builds with tests enabled, 'false' for Wasm builds, Android builds, and build-only mode
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: For Xcode testing on macOS, use xcodebuild test with appropriate -scheme, -sdk, -destination, and -enableCodeCoverage YES parameters; adjust SDK and destination for iOS Simulator, watchOS, tvOS, and visionOS platforms
📚 Learning: 2026-01-14T14:18:28.247Z
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: Wasm projects require explicit wasm-swift-flags input parameter configuration including WASI emulation flags and memory configuration flags; WASI emulation flags (-Xcc -D_WASI_EMULATED_SIGNAL, -Xcc -D_WASI_EMULATED_MMAN, -Xlinker flags) are required for projects using Foundation/CoreFoundation; memory flags (--initial-memory, --max-memory) are required for test suites processing large datasets
Applied to files:
CLAUDE.mdaction.yml
📚 Learning: 2026-01-14T14:18:28.247Z
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: For WebAssembly builds, use WasmKit runtime by default (bundled with Swift 6.2.3+) for instant test execution without external downloads; optionally configure wasmtime-version parameter for Wasmtime fallback with specific version numbers in X.Y.Z format (no 'latest' support)
Applied to files:
CLAUDE.mdaction.yml
📚 Learning: 2026-01-14T14:18:28.247Z
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: Applies to **/{Package.swift,*.swift} : For Swift packages tested locally, use swift build with --build-tests, --cache-path .cache, and --force-resolved-versions flags; then run swift test with --enable-code-coverage, --cache-path .cache, and --force-resolved-versions flags
Applied to files:
CLAUDE.mdaction.yml
📚 Learning: 2026-01-14T14:18:28.247Z
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: WebAssembly builds do NOT support code coverage; use the contains-code-coverage output to conditionally skip coverage collection for Wasm builds
Applied to files:
CLAUDE.mdaction.yml
📚 Learning: 2026-01-14T14:18:28.247Z
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: Use contains-code-coverage output from the build action to conditionally run coverage collection steps; returns 'true' for SwiftPM and Xcode builds with tests enabled, 'false' for Wasm builds, Android builds, and build-only mode
Applied to files:
CLAUDE.mdaction.yml
📚 Learning: 2026-01-14T14:18:28.247Z
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: For Android testing, delegate to skiptools/swift-android-actionv2 which supports Swift 6.2+, API levels 28+; use android-run-tests: false for ARM macOS runners where emulator support is unavailable
Applied to files:
CLAUDE.mdaction.yml
📚 Learning: 2026-01-14T14:18:28.247Z
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: GitHub Action wasm-swift-flags input parameter is interpolated directly into shell commands without sanitization; ensure values are properly validated before passing if used in reusable workflows accepting external inputs; never pass untrusted user input directly to this parameter
Applied to files:
CLAUDE.mdaction.yml
📚 Learning: 2026-01-14T14:18:28.247Z
Learnt from: CR
Repo: brightdigit/swift-build PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-14T14:18:28.247Z
Learning: For Xcode testing on macOS, use xcodebuild test with appropriate -scheme, -sdk, -destination, and -enableCodeCoverage YES parameters; adjust SDK and destination for iOS Simulator, watchOS, tvOS, and visionOS platforms
Applied to files:
CLAUDE.mdaction.yml
🪛 Shellcheck (0.11.0)
scripts/validate-test-output.sh
[warning] 45-45: YELLOW appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (5)
action.yml (3)
208-228: New Wasm testing inputs are clearly defined.
Descriptions, defaults, and mode semantics are easy to follow.Also applies to: 300-309
1402-1417: Consistent save-test-output wiring across SPM/Xcode/Wasm.
Nice to see pipefail + tee handling applied uniformly.Also applies to: 1798-1809, 1866-1877, 1941-1981
1588-1678: Helper function improves maintainability.
run_wasm_testsconsolidates mode handling and keeps Wasmtime/WasmKit paths aligned.scripts/validate-test-output.sh (1)
103-315: Validation flow is clear and reusable.
The function split and mode handling read cleanly.CLAUDE.md (1)
80-152: Wasm testing framework guidance reads well.
Auto-detect, overrides, and security notes are easy to follow.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| # Check for Swift Testing imports | ||
| if grep -r "import Testing" Tests/ --include="*.swift" >/dev/null 2>&1; then | ||
| HAS_SWIFT_TESTING=true | ||
| echo " ✓ Found Swift Testing imports" | ||
| else | ||
| HAS_SWIFT_TESTING=false | ||
| fi | ||
| # Check for XCTest imports | ||
| if grep -r "import XCTest" Tests/ --include="*.swift" >/dev/null 2>&1; then | ||
| HAS_XCTEST=true | ||
| echo " ✓ Found XCTest imports" |
There was a problem hiding this comment.
Tighten import detection to avoid false positives.
The current pattern would also match module names like TestingSupport or XCTestUtils, which could flip the detected mode incorrectly. Anchoring on the module name avoids that.
🔧 Proposed fix
- if grep -r "import Testing" Tests/ --include="*.swift" >/dev/null 2>&1; then
+ if grep -r -E '^[[:space:]]*import[[:space:]]+Testing([[:space:]]|$)' Tests/ --include="*.swift" >/dev/null 2>&1; then
HAS_SWIFT_TESTING=true
echo " ✓ Found Swift Testing imports"
else
HAS_SWIFT_TESTING=false
fi
# Check for XCTest imports
- if grep -r "import XCTest" Tests/ --include="*.swift" >/dev/null 2>&1; then
+ if grep -r -E '^[[:space:]]*import[[:space:]]+XCTest([[:space:]]|$)' Tests/ --include="*.swift" >/dev/null 2>&1; then
HAS_XCTEST=true
echo " ✓ Found XCTest imports"
else
HAS_XCTEST=false
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Check for Swift Testing imports | |
| if grep -r "import Testing" Tests/ --include="*.swift" >/dev/null 2>&1; then | |
| HAS_SWIFT_TESTING=true | |
| echo " ✓ Found Swift Testing imports" | |
| else | |
| HAS_SWIFT_TESTING=false | |
| fi | |
| # Check for XCTest imports | |
| if grep -r "import XCTest" Tests/ --include="*.swift" >/dev/null 2>&1; then | |
| HAS_XCTEST=true | |
| echo " ✓ Found XCTest imports" | |
| # Check for Swift Testing imports | |
| if grep -r -E '^[[:space:]]*import[[:space:]]+Testing([[:space:]]|$)' Tests/ --include="*.swift" >/dev/null 2>&1; then | |
| HAS_SWIFT_TESTING=true | |
| echo " ✓ Found Swift Testing imports" | |
| else | |
| HAS_SWIFT_TESTING=false | |
| fi | |
| # Check for XCTest imports | |
| if grep -r -E '^[[:space:]]*import[[:space:]]+XCTest([[:space:]]|$)' Tests/ --include="*.swift" >/dev/null 2>&1; then | |
| HAS_XCTEST=true | |
| echo " ✓ Found XCTest imports" |
🤖 Prompt for AI Agents
In `@action.yml` around lines 1533 - 1544, The current grep checks that set
HAS_SWIFT_TESTING and HAS_XCTEST can match substrings like "TestingSupport" or
"XCTestUtils"; update the two grep invocations that detect "import Testing" and
"import XCTest" to anchor the module name so it only matches the exact import
token (e.g., use extended regex with a word-boundary after the module name such
as grep -r -E 'import[[:space:]]+Testing\>' and grep -r -E
'import[[:space:]]+XCTest\>' respectively), leaving the rest of the logic around
HAS_SWIFT_TESTING and HAS_XCTEST unchanged.
| # Xcodebuild output detected - sum all "All tests" bundle totals | ||
| echo "DEBUG: Detected xcodebuild format (all_tests_count=$all_tests_count)" >&2 | ||
| count=$(echo "$input" | grep -A1 "Test Suite 'All tests' passed" 2>/dev/null | \ | ||
| grep -o 'Executed [0-9]\+ tests' | \ | ||
| grep -o '[0-9]\+' | \ | ||
| awk '{sum+=$1} END {print sum+0}') | ||
| else |
There was a problem hiding this comment.
Count parsing misses failed “All tests” lines.
Filtering only for “passed” returns 0 on failures, which obscures real counts.
🔧 Proposed fix
- count=$(echo "$input" | grep -A1 "Test Suite 'All tests' passed" 2>/dev/null | \
+ count=$(echo "$input" | grep -A1 "Test Suite 'All tests'" 2>/dev/null | \
grep -o 'Executed [0-9]\+ tests' | \
grep -o '[0-9]\+' | \
awk '{sum+=$1} END {print sum+0}')🤖 Prompt for AI Agents
In `@scripts/validate-test-output.sh` around lines 68 - 74, The parsing currently
greps for "Test Suite 'All tests' passed" which skips bundle lines when tests
fail; update the grep in the count calculation to match "Test Suite 'All tests'"
(no "passed") so both passing and failing totals are captured, e.g. change the
pattern used in the pipeline that computes count (the grep -A1 "Test Suite 'All
tests' passed") to grep -A1 "Test Suite 'All tests'"; keep the rest of the
pipeline (grep -o 'Executed [0-9]\+ tests' | grep -o '[0-9]\+' | awk '{sum+=$1}
END {print sum+0}') and the variables count, all_tests_count, and input
unchanged.
Summary by CodeRabbit
New Features
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.