test(utils): report modern command failures instead of swallowing them - #8817
Open
keepview wants to merge 5 commits into
Open
test(utils): report modern command failures instead of swallowing them#8817keepview wants to merge 5 commits into
keepview wants to merge 5 commits into
Conversation
Both spawn helpers hide why a command died: - runModernCommand resolves on any exit code unless stdout happened to contain 'Compile error', and callers do not check the resolved code, so a crashed `modern build` is indistinguishable from a successful one. The test then fails on a missing artifact, which points at the wrong place. - runContinuousTask never reads stderr at all, and resolves when the app closes before printing its ready marker. A dev server that fails to boot leaves no trace; the next assertion reports a connection refused. Report the exit code and captured output in both cases. Neither change alters control flow: the promises settle exactly as before, so tests that expect a failing build are unaffected.
✅ Deploy Preview for modernjs-byted ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Contributor
Rsdoctor Bundle Diff Analysis📊 Quick Summary
Generated by Rsdoctor GitHub Action |
A dev/build failure surfaces as `SyntaxError: Unexpected token ')'` with no file and no line: the CLI catches the loader error and re-logs only the message, dropping the frame Node would have printed. Generated code is the only input that differs per platform, so parse what was written under `.modern-js` after a failure and let `node --check` report the offender. The copy to a temp `.mjs` is deliberate: `node --check` treats a `.js` file as CommonJS and silently accepts specifiers that only break under module semantics, which is precisely the failure being chased. Co-Authored-By: Riff
Only `.modern-js` was swept, which misses transpiled configs written to `node_modules/.cache` — a likely home for the module that fails to parse. Walk `node_modules` instead: installed packages are pnpm symlinks and `isDirectory()` is false for those, so the walk covers what this run wrote without descending into the store. Co-Authored-By: Riff
node cannot parse JSX at all, so every generated .jsx file was reported as broken — the sweep produced a false positive instead of a lead. Co-Authored-By: Riff
Every fixture that fails is "type": "module", and every one that passes is CommonJS — including tailwind v2/v3 passing in the same suite where v4 fails. A module project resolves the framework to dist/esm-node, a CommonJS one to dist/cjs, and all the failures are the same SyntaxError raised while node compiles an ES module. That points at the ESM build itself, which the existing sweep never looked at. Parse every published .mjs with vm.SourceTextModule (parse only, nothing is linked or evaluated): 566 files in under 0.1s, so it can run on failure. It names the file the loader refuses to. Co-Authored-By: Riff
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.
Both spawn helpers hide why a command died:
modern buildis indistinguishable from a successful one. The test then fails on a missing artifact, which points at the wrong place.Report the exit code and captured output in both cases. Neither change alters control flow: the promises settle exactly as before, so tests that expect a failing build are unaffected.
Summary
Related Links
Checklist
pnpm run change.