fix(fixtures): fail loud on non-ENOENT readdir errors in the conformance runner#80
Conversation
…nce runner The bare catch around readdirSync treated every error as "directory not found", silently skipped the whole category, and continued to exit 0. A filesystem hiccup, permission error, or transient I/O fault on valid/, invalid/, or edge-cases/ would report partial success as full success. Narrow the catch: on err.code === "ENOENT" keep the quiet skip and return; on any other code (EACCES, ENOTDIR, EIO, EMFILE, ...) write a loud message to stderr and process.exit(2). The exit code is distinct from the existing exit 1 for test failures and exit 0 for all-pass, so CI can tell "runner could not run" apart from "tests failed". No automated exit-2 test is committed: the runner is a self-executing script with hardcoded categories that calls process.exit at the top level, so asserting the exit code would require refactoring it into an importable module plus a subprocess harness in a separate file, beyond this single-file fix. The non-ENOENT path was verified manually against the real runner code (pointing a category at a file yields "Failed to read .../: ENOTDIR ..." and exit 2), and the normal run still exits 0 across all three categories. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 42 minutes and 13 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
1 participant
Summary
spec/fixtures/run-fixtures.mjswrappedreaddirSyncin a barecatch {}that treated every error as "directory not found", silently skipped the whole category (valid/,invalid/, oredge-cases/), and still exited 0. A filesystem hiccup, permission glitch, or transient I/O error on any fixture directory would report partial success as full success.Fix
Narrow the catch in
runCategory:err.code === "ENOENT": keep the quiet skip and return (a genuinely absent directory).EACCES,ENOTDIR,EIO,EMFILE, ...): write✗ Failed to read <category>/: <err.message>to stderr andprocess.exit(2).Exit 2 is distinct from the existing exit 1 (test failures) and exit 0 (all pass), so CI can tell "the runner could not run" apart from "tests failed".
Verification
The non-ENOENT path was exercised against the real runner code by temporarily pointing a category at a file (a non-directory) inside
spec/fixtures/:It stopped before the results summary, as intended. The ENOENT path still skips quietly and continues. The temporary probe file was removed; only the one-file change is committed.
Why no committed exit-2 test
The runner is a self-executing script: it hardcodes
runCategory("valid"|"invalid"|"edge-cases")and callsprocess.exitat the top level. Asserting an exit code in-process is not possible (the call exits the process), and a faithful automated test would require refactoring the runner into an importable module and adding a subprocess harness in a separate test file. That exceeds the single-file scope of this fix and the issue's intent, so the non-ENOENT path was verified manually instead (shown above).Gates (all pass, node v22.22.2)
npm run build:coreand fullnpm run build: successnpm test(root): 473 passed, 0 failednpm run typecheck: cleannpm run lint: changed file clean (repo baseline of 5 errors / 22 warnings is in untouched files)node spec/fixtures/run-fixtures.mjs: exit 0, all three categories reported (valid/,invalid/,edge-cases/), 29 passed / 0 failedCloses #68
Summary by cubic
Stops the fixtures conformance runner from silently skipping categories on non-ENOENT errors. On read failures, it now prints a clear error and exits with code 2 so CI can distinguish runner errors from test failures.
Written for commit 8270b64. Summary will update on new commits. Review in cubic