You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
test: make the generator and dev server tests stable - #4885
Flaky CI runs in the generator and dev server tests, fixed without skipping anything. This PR changes only tests.
create-webpack-app tests: real npm install failures
The init, loader and plugin tests install real packages. A failed install made the CLI exit with code 2 and print the reason on stderr. The tests only check stdout, so CI showed a misleading "Expected substring: Project has been initialised with webpack!", with stdout ending right after the generated files. Examples:
Forcing an install failure locally gives the same output.
A failed install now fails the test with npm's error, e.g. Installing packages failed: npm error ..., instead of a missing line in stdout.
The audit, fund and update-notifier requests are turned off, since the tests don't need them, so fewer requests can fail.
prefer-offline is not used. An earlier commit of this PR tried it, and CI then failed with ETARGET: No matching version found for @babel/generator@^8.0.10: npm's cached metadata lacked a version that a freshly fetched package already depended on.
npm's default retries are kept. With more retries, an install that can't succeed ran past the 480 s test timeout, which hid npm's error again.
create-webpack-app tests: shared Yarn cache
The init, loader and plugin tests also install with Yarn, in parallel Jest workers. Yarn 1 can't fill one cache from two installs at the same time: on macOS CI an install failed with SyntaxError: .../Caches/Yarn/v6/npm-path-exists-3.0.0-.../package.json: Unexpected end of JSON input. Each Jest worker now uses its own Yarn cache through YARN_CACHE_FOLDER.
Dev server tests: port already in use
get-port() with no options takes a port from the OS ephemeral range. Outgoing connections, such as the parallel installs above, use the same range and can take the port before the dev server listens. Parallel Jest workers can also get the same port.
A shared getFreePort() helper in test/utils/test-utils.js gives each Jest worker its own 100-port range. There are 120 such ranges between 20000 and 31999, below the ephemeral ranges of Linux, macOS and Windows. The serve-basic, serve-variable and with-custom-port tests and multi-dev-server.config.js use it.
Testing
test/serve/basic, test/serve/serve-variable and test/serve/with-custom-port: 41 passed, 1 skipped.
test/create-webpack-app: 6 suites, 77 tests passed, with the three Yarn tests in different workers each using their own cache.
An install against an unusable registry now fails with npm's error, well within the test timeout.
The `create-webpack-app` tests install real packages, and a registry
hiccup made npm fail, which the tests only reported as a missing line in
stdout. npm now prefers its cache and retries requests more, without the
audit, fund and update-notifier requests, and a failed install fails the
test with npm's error.
The dev server tests took ports from the ephemeral range, where outgoing
connections or another Jest worker could take them before the server
listened. Each worker now takes ports from its own range below the
ephemeral ones.
Claude-Session: https://claude.ai/code/session_013FhutRFjuQemBzekJMYcqL
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.
No actionable comments were generated in the recent review.
Recent review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 2e742b76-f317-4023-90dd-b1ac1176eb78
Commits
Reviewing files that changed from the base of the PR and between f3c80dd and ed4db6e.
Files selected for processing: 1
test/create-webpack-app/test.utils.js
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.
Walkthrough
The package-manager test wrappers now set npm defaults and a Yarn 1 cache directory per Jest worker. Caller environment values override these defaults. The run and runPromptWithAnswers helpers check for package-install errors; runWatch does not. Serve tests now use the shared getFreePort() utility, which selects a free port from a worker-specific range.
Priority: Normal
Merge Risk: Moderate · up to ed4db
Concurrent Jest runs can still select the same port and fail with EADDRINUSE; coordinate allocations across runs or retry failed binds before merging.
Pre-merge checks: 5
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title "test: make the generator and dev server tests stable" matches the main change. The PR adds a shared getFreePort() helper so each Jest worker gets its own port range, and it updates the gene…
Docstring Coverage
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Title check
Explanation
The title "test: make the generator and dev server tests stable" matches the main change. The PR adds a shared getFreePort() helper so each Jest worker gets its own port range, and it updates the generator test utilities so install errors surface through stderr. Both parts of the title match the changeset, and the title is short and specific.
Full details: Docstring Coverage
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 6 files.
Finishing Touches
📝 Generate docstrings
Commit to this branch
Create a new PR
🧪 Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Some tools did not complete. Review the errors below.
ESLint
If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.
test/create-webpack-app/test.utils.js
Oops! Something went wrong! :(
ESLint: 10.13.0
Error: Error while loading rule 'jest/no-deprecated-functions': Unable to detect Jest version - please ensure jest package is installed, or otherwise set version explicitly
Occurred while linting /test/create-webpack-app/test.utils.js
at detectJestVersion (/.eslint-tmp/node_modules/eslint-plugin-jest/lib/rules/utils/detectJestVersion.js:22:9)
at create (/.eslint-tmp/node_modules/eslint-plugin-jest/lib/rules/no-deprecated-functions.js:31:107)
at Object.create (/.eslint-tmp/node_modules/@typescript-eslint/utils/dist/eslint-utils/RuleCreator.js:39:20)
at createRuleListeners (/.eslint-tmp/node_modules/eslint/lib/linter/linter.js:502:15)
at /.eslint-tmp/node_modules/eslint/lib/linter/linter.js:628:7
at Array.forEach ()
at runRules (/.eslint-tmp/node_modules/eslint/lib/linter/linter.js:562:31)
at #flatVerifyWithoutProcessors (/.eslint-tmp/node_modules/eslint/lib/linter/linter.js:1297:4)
at Linter._verifyWithFlatConfigArrayAndWithoutProcessors (/.eslint-tmp/node_modules/eslint/lib/linter/linter.js:1382:43)
at Linter._verifyWithFlatConfigArray (/.eslint-tmp/node_modules/eslint/lib/linter/linter.js:1449:15)
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.
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
Cap maxWorkers at 24 so worker IDs 1 and 25 cannot share a port range.
getFreePort() maps JEST_WORKER_ID with % 24, so worker 25 uses the same range as worker 1 (20000–20499). The parent mapping (workerId - 1) * 500 gave each worker a distinct range through worker 25.
Two workers in one run can call getFreePort() at overlapping times. The helper returns a port number and does not reserve it. If worker 1 has not started its dev server yet, worker 25 can receive the same port. The second dev server then fails with EADDRINUSE.
This requires at least 25 concurrent workers. jest.config.js does not set maxWorkers, so the default worker count applies. Runs with fewer than 25 workers are not affected. An explicit -w or --maxWorkers value overrides the config setting.
Setting maxWorkers to 24 keeps worker IDs in the range 1–24, so the modulo never wraps in a default run.
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 0b8b75ad-7338-450c-92d0-659108955100
Commits
Reviewing files that changed from the base of the PR and between 3c1d9a6 and 0236d0a.
Files selected for processing: 1
test/utils/test-utils.js
Files skipped from review as they are similar to previous changes: 1
test/utils/test-utils.js
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.
@coderabbitai the finding outside the diff (worker 25 sharing worker 1's port range) is fixed in 9bbf4a6. I shrank each range instead of capping maxWorkers, so the Jest config stays unchanged. Each worker now gets 100 ports, which is plenty because a worker uses only a few at a time. That gives 120 separate ranges between 20000 and 31999, below the Linux ephemeral range that starts at 32768.
When ENABLE_PIPE=1, createProcess uses inherited stdio and reject: false. The create-webpack-app install can exit with code 2, but checkInstall only checks captured stderr, so it returns the failed result without reporting the install error.
Check the nonzero exit status, while preserving the existing stderr check.
Suggested fix
- * @template {{ stderr: string }} T+ * @template {{ stderr?: string, exitCode?: number }} T
...
const checkInstall = (result) => {
- if (result.stderr && result.stderr.includes("Error occurred while installing packages")) {- throw new Error(`Installing packages failed:\n${result.stderr}`);+ const failedExit =+ typeof result.exitCode === "number" && result.exitCode !== 0;+ const failedInstall =+ result.stderr?.includes("Error occurred while installing packages") ?? false;++ if (failedExit || failedInstall) {+ const details = result.stderr ?? `Process exited with code ${result.exitCode}`;+ throw new Error(`Installing packages failed:\n${details}`);
}
Review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: aea60134-6702-4b04-bac1-bdf7a50f4f66
Commits
Reviewing files that changed from the base of the PR and between 0236d0a and 9bbf4a6.
Files selected for processing: 1
test/utils/test-utils.js
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.
@coderabbitai the ENABLE_PIPE=1 finding stays as is. ENABLE_PIPE is a local debugging switch that sends the CLI's output to the terminal. In that mode stdout isn't captured either, so a failed install still fails the test: the expect(stdout).toContain("Project has been initialised with webpack!") assertion fails. npm's error is already on screen too, because stderr is inherited. Checking every nonzero exit code would also break tests that expect the CLI to fail on purpose. For example, "should throw if the current path is not writable" expects init to exit with code 2.
The other remaining port-collision risk is the cross-run one. I answered that on its thread: CI runs one Jest process per job.
For best results, initiate chat on the files or code changes.
@alexander-akait You're right. In ENABLE_PIPE=1 mode, the success-output assertion still fails when installation fails, and inherited stderr shows npm’s error. Rejecting every nonzero exit code would also break tests that intentionally expect failure. I withdraw the ENABLE_PIPE=1 finding.
The cross-run port-collision concern does not apply to the CI setup you described, with one Jest process per job. No change is needed for either concern.
For best results, initiate chat on the files or code changes.
@alexander-akait Thanks for fixing this in 9bbf4a6. The 100-port ranges remove the overlap between workers 1 and 25 and support 120 distinct worker ranges without changing maxWorkers. This addresses the reported collision.
If you found this review helpful, would you consider giving us a shout-out on X?
With `prefer-offline`, npm used cached metadata that lacked a version a
freshly fetched package depended on, and the install failed with
ETARGET (`@babel/generator@^8.0.10`) on Windows CI.
Also add a changeset for not detecting browsers in `webpack info` on
Windows.
Claude-Session: https://claude.ai/code/session_013FhutRFjuQemBzekJMYcqL
Agreed. The changeset wasn't for the test changes. It was for the webpack info fix ported from #4847: on Windows, info no longer detects browsers, because envinfo starts them and can hang. That fix changes packages/webpack-cli. I moved the changeset to #4847 (466391f), which carries the same fix, and removed it here in 77665c5. That also avoids a duplicate changelog entry if both PRs land.
The init, loader and plugin tests install with Yarn in parallel Jest
workers. Yarn 1 can't fill one cache from two installs at the same time,
and an install failed on macOS CI reading a half-written package.json
from the shared cache.
Claude-Session: https://claude.ai/code/session_013FhutRFjuQemBzekJMYcqL
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
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Summary
Flaky CI runs in the generator and dev server tests, fixed without skipping anything. This PR changes only tests.
create-webpack-apptests: realnpm installfailuresThe init, loader and plugin tests install real packages. A failed install made the CLI exit with code 2 and print the reason on stderr. The tests only check stdout, so CI showed a misleading "Expected substring: Project has been initialised with webpack!", with stdout ending right after the generated files. Examples:
main, run 40124087314 (vue template)Forcing an install failure locally gives the same output.
Installing packages failed: npm error ..., instead of a missing line in stdout.prefer-offlineis not used. An earlier commit of this PR tried it, and CI then failed withETARGET: No matching version found for @babel/generator@^8.0.10: npm's cached metadata lacked a version that a freshly fetched package already depended on.create-webpack-apptests: shared Yarn cacheThe init, loader and plugin tests also install with Yarn, in parallel Jest workers. Yarn 1 can't fill one cache from two installs at the same time: on macOS CI an install failed with
SyntaxError: .../Caches/Yarn/v6/npm-path-exists-3.0.0-.../package.json: Unexpected end of JSON input. Each Jest worker now uses its own Yarn cache throughYARN_CACHE_FOLDER.Dev server tests: port already in use
get-port()with no options takes a port from the OS ephemeral range. Outgoing connections, such as the parallel installs above, use the same range and can take the port before the dev server listens. Parallel Jest workers can also get the same port.A shared
getFreePort()helper intest/utils/test-utils.jsgives each Jest worker its own 100-port range. There are 120 such ranges between 20000 and 31999, below the ephemeral ranges of Linux, macOS and Windows. Theserve-basic,serve-variableandwith-custom-porttests andmulti-dev-server.config.jsuse it.Testing
test/serve/basic,test/serve/serve-variableandtest/serve/with-custom-port: 41 passed, 1 skipped.test/create-webpack-app: 6 suites, 77 tests passed, with the three Yarn tests in different workers each using their own cache.🤖 Generated with Claude Code
https://claude.ai/code/session_013FhutRFjuQemBzekJMYcqL
Summary by CodeRabbit