Repository navigation
fix(ci): deduplicate turbo.json test tasks and guard against repeats - #183
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 57 minutes. View limit details
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 72 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
|
The following comment was made by an LLM, it may be inaccurate: |
Problem
turbo.jsondefined 11#testtasks twice (#145). Turbo's JSONC parser silently keeps the last definition, so the first (transit) block was dead config: 8 of the tasks actually ran withdependsOn: ["^build"]and 3 (http-recorder,theme,util) with nodependsOnat all — losing even thetransithash-folding the file's own comment describes.Fixes #145
Solution — deliberate choice: restore
transitThe
transitblock is the newer, intended definition: commit1fc489d878added it together with the comment "Workspace packages export their sources, so tests never wait for a dependency's build … No package has a transit script: it runs nothing and only folds every workspace dependency into the test hash" — and left the older block behind it. Every other#testtask in the file already usestransit(exceptions are single, deliberate ones:protocol^build,schema/desktopnone).So this PR deletes the trailing duplicate block (41 lines) and keeps the
transitdefinitions. This is an intentional behaviour change, not a neutral dedup:turbo run test --dry=jsonbefore → after (76 → 65 tasks in graph):@opencode/client#test:^builddeps (3 builds) →@opencode/client#transit@opencode/gui-extensions#test: 6#builddeps → owntransit@opencode/plugin#test: 6#builddeps → owntransit@opencode/plugin-browser#test,@opencode/plugin-github#test,@opencode/merman#test:#builddeps → owntransit@opencode/sdk#test: 9#builddeps →@opencode/sdk#transit(which itself folds all 9 workspacetransitdeps)@opencode/simulation#test: 5#builddeps → owntransit@opencode/http-recorder#test,@opencode/theme#test,@opencode/util#test:[]→ owntransit(cache hash now folds workspace dependencies again)#buildtasks leave the test graph entirely — tests no longer wait for dependency builds, per the file's stated intent. No other task's graph changes.Guard
script/turbo-json.test.ts: a duplicate-aware JSONC scan (tracks every object's keys in the raw text; comments and string contents never count) assertingturbo.jsondefines every key exactly once, plus two self-tests for the scanner.test.yml's unit job next to the existing mcpx-cleanup script step (samescript/pattern as test(script): mcpx-cleanup.test.ts is run by nothing — no CI job, turbo task, or package script executes it #138), so a repeat fails CI on both OS legs.Verification
#testkeys, 32 unique, 0 duplicates (was 43/32/11).bun test turbo-json.test.ts(fromscript/): 3/3 pass.turbo.jsonrestored, exactly the real-file test fails (listing the duplicates); with the fix, 3/3 pass.turbo run test --dry=jsonsucceeds before and after; graph diff above.test.ymlYAML parses.Single-repo: no
turbo.jsonexists in developing-today/code, developing-today/openchamber, or dezren39/nix (checked), so no cross-repo set is owed.Need help on this PR? Tag
@codesmith-botwith what you need.