test(detectors): add test coverage for relay-setup's detectors module - #1
Merged
Conversation
…odule Adds test/detectors/*.test.mjs covering every exported function across skills/relay-setup/scripts/detectors/*.mjs (previously zero coverage — the gap that let detectAppId, detectLintCmd/detectFormatCmd/ detectFormatWriteCmd, and detectSourceDirs bugs ship undetected). Widens package.json's test glob to pick up the new test/detectors/ subdirectory. Dev's first pass (Sonnet, real claude-cli calls) produced 190 tests with 57 failing — traced every failure to a real signature/argument-order mismatch (the agent had no source-read access when guessing several detectors' contracts) and corrected them against actual source. That pass also surfaced 3 genuine, previously-undiscovered detector bugs (fixed separately on main: the 'biome' dependency key not matching the real @biomejs/biome package, @sentry/node not recognized, and findFiles never testing its predicate against directories) — see dev-log.md for the full account. 204/204 tests passing.
…commits, not part of this feature
Previous wording ('Fixed the key', 'Added it', 'this fix') read as if
this feature's own change touched detector source — it doesn't. This
feature's own commits have zero diff under
skills/relay-setup/scripts/detectors/ (confirmed via git diff
main...feat/detector-tests). The three bugs were reported here per AC 26
and fixed via separate, dedicated commits directly on main, which this
branch is now based on top of.
- analytics/paywall/stack/error-tracking test files: replaced generic
hasSignal() truthy checks with exact literal-value assertions for
every recognized dependency (confirmed against real source), matching
this repo's stated characterization-testing convention. Removed the
dangling "see dev-log.md (Batch 2)" comments — that section was never
written; there's no remaining caveat to defer since exact values are
now asserted directly.
- analytics.test.mjs: added the previously-untested firebase-analytics
branch (requires firebase dep AND a real src/ dir), removed a
duplicate no-signal test.
- source-layout.test.mjs: clarified the AC15 Expo-router test with an
explicit source citation confirming the signal is the expo-router
dependency, not an app/_layout.tsx file.
- project.test.mjs: added a negative characterization test confirming
detectAppId does NOT read capacitor.config.ts (only .json).
- Verified .ai/config.json's commands.test ("npm test") already covers
the widened glob via package.json delegation — no change needed.
233/233 tests passing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds unit test coverage for
skills/relay-setup/scripts/detectors/*.mjs(previously zero coverage) — the auto-detection logicdetect-stack.mjsuses to bootstrap a new project's.ai/config.json.test/detectors/, one per detector module (fs-helpers,project,project-type,commands,source-layout,stack,analytics,paywall,e2e,error-tracking,locales), mirroring the existingtest/agent-runner.test.ts↔agent-runner.tsconvention.package.json'sscripts.testglob to pick uptest/detectors/**/*.test.mjs.skills/relay-setup/scripts/detectors/ordetect-stack.mjsare touched (confirmed via diff).This was built as a dogfooding exercise, running Relay's own pipeline (PM → Dev Review → Architect → Dev → Review → QA → Retro) against Relay's own codebase, with real LLM calls throughout. Full account in
.ai/artifacts/features/detector-tests/.3 real detector bugs found while writing these tests
All three were fixed separately, directly on
main(already merged, this branch is rebased on top) — not as part of this test-only change:detectLintCmd/detectFormatCmd/detectFormatWriteCmdchecked the dependency key'biome', which isn't a real npm package (the actual package is@biomejs/biome) — the Biome fallback path never fired for any real project using it.detectErrorTrackingdidn't recognize@sentry/node(the plain Node.js/backend Sentry SDK) — only frontend-framework-flavored Sentry packages.fs-helpers.mjs'sfindFilesonly ever tested its predicate against files, never directories — silently breakingdetectLocales's own directory-name fallback branch.Tests assert the current (already-fixed) behavior.
Process notes
mainalong the way, see48fccbf/bdce31b). All PASS_WITH_NOTES action items (exact-value assertions, dangling doc references, a duplicate test, two scenario-fidelity clarifications) were addressed in a follow-up commit.Test plan
npm test— 233/233 passingnpm run typecheck— cleangit diff main...feat/detector-tests -- skills/relay-setup/scripts/detectors/— empty (test-only, confirmed)