fix(test): close the Node-version guard's remaining coverage gap#7629
Merged
Conversation
#7619 wired scripts/check-node-version.mjs as a pretest* hook on 5 npm script names (test, test:ci, test:coverage, test:workers, ui:test). A repo-wide audit found this misses 8 of 12 vitest-invoking script names (test:unit, test:integration, test:upstream-contract, test:engine-parity, test:live-gate-parity, test:driver-parity, test:changed, test:watch) -- npm's pre<name> hook only fires for the exact script name it's wired to. Worse: this class of mechanism can never cover a direct `npx vitest run test/unit/<file>.test.ts` invocation -- which reference.md and SKILL.md both explicitly recommend for fast iteration -- since that command doesn't go through any npm script at all. Add test/helpers/vitest-global-setup-node-version.ts, wired as `globalSetup` in every vitest.config.ts in the repo (root, workers, and every workspace with its own config). globalSetup runs once before any test file regardless of invocation path, closing the gap completely instead of requiring anyone to remember a matching pretest* entry for every current and future script name. Verified directly on Node 26: a direct `npx vitest run <file>` now fails immediately via globalSetup, both at the root and inside apps/loopover-ui -- no confusing downstream test failures, one clear message. The existing pretest* hooks stay as a genuinely-faster fail for the 5 high-traffic commands (they run before npm even spawns vitest); check-node-version.mjs's header comment now describes globalSetup as the real guarantee and the hooks as a nicety on top. Closes #7627
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
loopover-ui | 9b46d3e | Commit Preview URL Branch Preview URL |
Jul 21 2026, 05:06 AM |
Bundle ReportBundle size has no change ✅ |
loopover-orb
Bot
added
the
gittensor:bug
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
1 participant
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.
Closes #7627
Summary
pretest*hooks (fix(test): enforce the Node version pin on every test run, not just install #7619) only cover 5 npm script names and can never cover a directnpx vitest run <file>call, which the contributing docs explicitly recommend for fast iteration. Adds a vitestglobalSetupwired into all 6 vitest configs in the repo, closing the gap completely regardless ofinvocation path -- current or future script name, or a direct
npx vitestcall.Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
src/**-only include glob, sothis owes no patch-coverage number. Verified via direct reproduction instead of new tests: a direct
npx vitest run <file>on Node 26 now fails immediately via globalSetup with a clear message, checkedboth at the repo root and inside
apps/loopover-ui(the workspace where the underlying Node-26 bugactually lived).
Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.UI Evidence
Not applicable — no visible UI/frontend change (test infrastructure only).
Notes
it shipped.