Skip to content

test: add SPM + Xcode-app integration harnesses for the a11y-scan plugin#31

Open
Crash0v3rrid3 wants to merge 1 commit into
mainfrom
chore/tests-spm-xcode-integration
Open

test: add SPM + Xcode-app integration harnesses for the a11y-scan plugin#31
Crash0v3rrid3 wants to merge 1 commit into
mainfrom
chore/tests-spm-xcode-integration

Conversation

@Crash0v3rrid3

Copy link
Copy Markdown
Collaborator

What

Adds a tests/ folder with two consumer harnesses that integrate this repo's a11y-scan command plugin (via a path dependency on the repo root, ../..) and run accessibility scans over sample sources containing intentional issues.

tests/
├── README.md
├── spm/          # SwiftPM package consumer
└── xcode-app/    # Xcode iOS app (XcodeGen)

tests/spm/ — SwiftPM consumer

  • Declares the AccessibilityDevTools dependency and invokes the command plugin via swift package plugin … scan (scripts/run-a11y-scan.sh).
  • Sample SwiftUI views with intentional a11y issues; an XCTest unit test plus a credential-gated end-to-end scan test (RUN_A11Y_SCAN=1).
  • The plugin is intentionally not attached to a target's plugins: array — a11y-scan is a command plugin (not build-tool), and attaching it makes swift build fail with "Plugin is declared with the buildTool capability…".

tests/xcode-app/ — Xcode app

  • An iOS app described by an XcodeGen project.yml, so the generated .xcodeproj is gitignored.
  • Runs the scan from a pre-compile build phase (the official Xcode integration), backed by a minimal Package.swift scan driver. Unit-test target included. ENABLE_USER_SCRIPT_SANDBOXING disabled so the scan can write ~/.cache.

Both harnesses skip/no-op without BROWSERSTACK_USERNAME / BROWSERSTACK_ACCESS_KEY, so builds and tests stay green.

Verification

  • tests/spm: swift build ✅, swift test ✅ (unit test passes, scan test skips by default); swift package plugin --list shows 'scan' (plugin 'a11y-scan' in package 'AccessibilityDevTools').
  • tests/xcode-app: project.yml validates as YAML; scan driver resolves and exposes the plugin; both shell scripts pass bash -n.
  • ⚠️ Not run in this environment: xcodegen generate + xcodebuild test (xcodegen/Xcode simulator unavailable) and a live scan (needs BrowserStack creds). Generate and run locally to validate on your toolchain.

🤖 Generated with Claude Code

Adds a tests/ folder with two consumer harnesses that integrate this repo's a11y-scan command plugin (path dependency on the repo root) and run accessibility scans over sample sources with intentional issues:

- tests/spm/: a SwiftPM package that declares the AccessibilityDevTools dependency and invokes the command plugin via 'swift package plugin … scan' (scripts/run-a11y-scan.sh). Includes an XCTest unit test plus a credential-gated end-to-end scan test. Verified: swift build + swift test pass and the plugin is discoverable. The plugin is intentionally NOT placed in a target's plugins: array — it is a command (not build-tool) plugin, and attaching it breaks swift build.

- tests/xcode-app/: an iOS app described by an XcodeGen project.yml that runs the scan from a pre-compile build phase (the official Xcode integration), backed by a minimal Package.swift scan driver. Includes a unit-test target. project.yml validates as YAML and the scan driver resolves + exposes the plugin; the generated .xcodeproj was not built in this headless environment.

Both harnesses no-op/skip without BrowserStack credentials so builds and tests stay green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Crash0v3rrid3 Crash0v3rrid3 requested a review from a team as a code owner June 18, 2026 13:50

@Crash0v3rrid3 Crash0v3rrid3 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review (automated) — 4 inline finding(s). Full report in the PR comment below. Verdict: Passed.

swift package plugin \
--allow-writing-to-directory "$HOME/.cache" \
--allow-writing-to-package-directory \
--allow-network-connections 'all(ports: [])' \

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Medium] Network-permission scope is broader than the plugin declares

all(ports: []) grants all ports, but the plugin declares .all(ports: [80, 443]). It works (and matches the repo's canonical scripts/.../spm.sh), but as a reference harness it over-grants and would break confusingly if the declared scope is tightened.

Suggestion: Use 'all(ports: [80,443])' to match the plugin's declared scope.

Reviewer: stack:code-reviewer

try process.run()
process.waitUntilExit()

// --non-strict makes the scan exit 0 even when issues are found, so a

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Medium] Docstring overstates what this test verifies

The assertion only checks terminationStatus == 0 under --non-strict, so it confirms the plugin downloaded/authenticated/ran — not that the seeded issues in SampleViews.swift were detected. The docstring's "reports the intentional issues" overstates this.

Suggestion: Reword to make clear issue detection is not asserted (exit 0 = plugin ran). Optionally set process.environment explicitly for CI robustness.

Reviewer: stack:code-reviewer

/// it. The accessibility scan itself runs as the app target's pre-compile
/// build phase (see project.yml), so a successful `xcodebuild test` means the
/// scan ran during the build.
func testContentViewExists() {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Low] @MainActor isolation under Swift 6 strict concurrency

testContentViewExists() instantiates a @MainActor-isolated SwiftUI View from a nonisolated test context — a warning on Xcode 15+, an error in Swift 6 mode.

Suggestion: Mark the test @MainActor (or wrap in MainActor.run).

Reviewer: stack:code-reviewer

settings:
base:
SWIFT_VERSION: "5.9"
# Required so the scan build phase can write the CLI cache to ~/.cache.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Low] Sandboxing disabled globally, not just for the app target

ENABLE_USER_SCRIPT_SANDBOXING: NO in settings.base applies to the test target too, not only the app target that runs the scan build phase.

Suggestion: Move it under targets.A11yScanDemoApp.settings.base to scope it to the app target.

Reviewer: stack:code-reviewer

@Crash0v3rrid3

Copy link
Copy Markdown
Collaborator Author

Claude Code PR Review

PR: #31Head: 9311d3fReviewers: stack:code-reviewer (+ orchestrator verification)

Summary

Adds a tests/ folder with two integration harnesses that consume this repo's a11y-scan SwiftPM command plugin via a path dependency on the repo root: tests/spm/ (a SwiftPM package that invokes the plugin with swift package plugin … scan) and tests/xcode-app/ (an XcodeGen-described iOS app that runs the scan from a pre-compile build phase). No production code changes — test/scaffolding only.

Review Table

Priority Category Check Status Notes
High Security No hardcoded secrets or credentials Pass Creds come from BROWSERSTACK_USERNAME/_ACCESS_KEY env vars; none embedded.
High Security Authentication/authorization checks present N/A No auth surface; harness only forwards env creds to the plugin.
High Security Input validation and sanitization N/A No external input handled.
High Security No IDOR — resource ownership validated N/A
High Security No SQL injection (parameterized queries) N/A
High Correctness Logic is correct, handles edge cases Pass swift build/swift test verified green; plugin discoverable. Minor nits below.
High Correctness Error handling is explicit, no swallowed exceptions Pass SPM script hard-fails on missing creds; xcode script soft-skips by design.
High Correctness No race conditions or concurrency issues Pass One @MainActor test-isolation nit under Swift 6 strict concurrency (F7, Low).
Medium Testing New code has corresponding tests Pass Unit test + credential-gated e2e scan test (spm); unit test (xcode-app).
Medium Testing Error paths and edge cases tested Partial E2e test is a smoke test (asserts exit 0); does not assert issue detection (F2).
Medium Testing Existing tests still pass (no regressions) Pass Additive only; no existing tests touched.
Medium Performance No N+1 queries or unbounded data fetching N/A
Medium Performance Long-running tasks use background jobs N/A
Medium Quality Follows existing codebase patterns Pass Plugin invocation mirrors repo's scripts/{bash,zsh,fish}/spm.sh.
Medium Quality Changes are focused (single concern) Pass Scoped to the new tests/ harness.
Low Quality Meaningful names, no dead code Pass Clear names; sample views document each intentional issue.
Low Quality Comments explain why, not what Pass Notably documents the command-vs-build-tool-plugin gotcha.
Low Quality No unnecessary dependencies added Pass Only a path dependency on the repo root.

Findings

  • File: tests/spm/scripts/run-a11y-scan.sh:19 (and tests/xcode-app/scripts/run-a11y-scan.sh:22)

  • Severity: Medium

  • Reviewer: stack:code-reviewer

  • Issue: --allow-network-connections 'all(ports: [])' grants all ports, whereas the plugin declares .all(ports: [80, 443]). It works (matches the repo's canonical scripts/.../spm.sh), but as a reference harness it grants a broader scope than the plugin needs and would break confusingly if the declared scope is ever tightened.

  • Suggestion: Use 'all(ports: [80,443])' in both harness scripts to match the plugin's declared scope. (Optional — aligns with, but is stricter than, the canonical script.)

  • File: tests/spm/Tests/A11yDemoLibTests/A11yDemoLibTests.swift:11-13,40

  • Severity: Medium

  • Reviewer: stack:code-reviewer

  • Issue: The docstring says the test "reports the intentional issues," but the assertion only checks terminationStatus == 0 under --non-strict — i.e. it verifies the plugin downloaded/authenticated/ran, not that the seeded issues were detected. The claim overstates what is verified. (Env propagation: the test inherits ProcessInfo env by default, so creds do forward — explicit process.environment is belt-and-suspenders, not required.)

  • Suggestion: Reword the docstring to "exit 0 means the plugin downloaded, authenticated, and scanned; issue detection is not asserted." Optionally set process.environment explicitly for CI robustness.

  • File: tests/xcode-app/Tests/A11yScanDemoAppTests.swift:10

  • Severity: Low

  • Reviewer: stack:code-reviewer

  • Issue: testContentViewExists() instantiates a @MainActor-isolated SwiftUI View from a nonisolated test context — a warning under Xcode 15+ strict concurrency, an error in Swift 6 mode.

  • Suggestion: Mark the test @MainActor (or wrap in MainActor.run).

  • File: tests/xcode-app/Package.swift:19

  • Severity: Low

  • Reviewer: stack:code-reviewer

  • Issue: targets: [] emits a "has no supported targets" warning on Swift 5.10+/Xcode 16. Intentional and commented, but consumers may mistake the warning for a bug.

  • Suggestion: Add one line to tests/xcode-app/README.md noting the expected warning.

  • File: tests/xcode-app/project.yml:20

  • Severity: Low

  • Reviewer: stack:code-reviewer

  • Issue: ENABLE_USER_SCRIPT_SANDBOXING: NO sits in settings.base, so it disables sandboxing for the test target too, not just the app target that runs the scan.

  • Suggestion: Move it under targets.A11yScanDemoApp.settings.base to scope it to the app target.

Orchestrator override — reviewer F3 ("cd "$(dirname "$0")/.." breaks on paths with spaces") is a FALSE POSITIVE and was dropped. Inside $(…), the "$0" is an independent quoting context; the outer quotes wrap the whole substitution, so the idiom is space-safe. No change needed.

Informational (not blocking): the #if canImport(SwiftUI) guard in SampleViews.swift, the SRCROOT coupling in project.yml, and the credential hard-fail-vs-soft-skip asymmetry between the two scripts (intentional, documented) — all fine as-is.

Verification performed

  • tests/spm: swift build ✅, swift test ✅ (unit passes, e2e scan test skips without RUN_A11Y_SCAN); swift package plugin --list'scan' (plugin 'a11y-scan' in package 'AccessibilityDevTools').
  • tests/xcode-app: project.yml valid YAML; scan-driver Package.swift resolves and lists the plugin; both scripts pass bash -n. Not run: xcodegen generate + xcodebuild test (xcodegen/simulator unavailable) and a live scan (needs creds).

Verdict: PASS — solid, well-documented test harness; all findings are Medium/Low polish on scaffolding, none blocking. Recommend addressing the docstring overstatement (F2), the @MainActor test nit (F7), and scoping the sandboxing flag (F5) in a follow-up.

@sunny-se sunny-se left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — reviewed via harness stack:pr-review. Good integration test harnesses.

Minor nit (non-blocking): Both tests/spm/Package.swift and tests/xcode-app/Package.swift use the deprecated name: parameter for path dependencies (.package(name: "AccessibilityDevTools", path: "../..")). SwiftPM 5.9+ infers the name — should be .package(path: "../.."). Works today but may emit warnings in future toolchains.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants