ci: make runtime matrix resilient to one-off flaky failures - #4977
Merged
Merged
Conversation
The runtime test suite occasionally fails with failures unrelated to the code under test, most recently a fatal V8 crash in Node.js 24.18.0 (v8::ToLocalChecked Empty MaybeLocal in node::cjs_lexer::Parse) while a runtime worker thread was preparsing CJS modules during startup in worker-scaler-1.test.js. Such a crash aborts the whole test process and, with the default fail-fast behavior, cancels all the other 29 matrix jobs, requiring a manual rerun of the entire matrix. - Disable fail-fast for the runtime matrix so a single flaky job no longer cancels the healthy ones. - Retry the test step once (same nick-fields/retry action already used for pnpm install), automating the current practice of rerunning failed jobs before investigating. Persistent failures still fail the job. - Raise the job timeout to accommodate the second attempt; each attempt is individually capped at 25 minutes. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Matteo Collina <[email protected]>
|
Dependency limit exceeded — report not shown. This pull request scan exceeded the 10,000-dependency limit applied to this scan, so the results are incomplete and may be inaccurate. To avoid reporting false positives, Socket has not posted a report. Upgrade your plan to raise the dependency limit and get complete reports, or view the partial scan in the dashboard. Socket is always free for open source. If this is a non-commercial open source project, contact us to request a free Team account. |
The default shell of nick-fields/retry on Windows is PowerShell 5.1, which does not support the && statement separator. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Matteo Collina <[email protected]>
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
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.
Problem
The runtime test suite occasionally fails for reasons unrelated to the code under test. The latest instance (run 29703993963, job
runtime - ubuntu-24.04 - 24 - mainon PR #4976) was a fatal V8 crash in Node.js 24.18.0:The crash happened ~3 seconds into
worker-scaler-1.test.js, while a runtime worker thread was preparsing CJS modules duringapp.start(). A fatal V8 error in any worker thread aborts the whole test process, so the file fails with no usable test output. This is nodejs/node#63323 — a race in the native CJS lexer introduced in Node 24.14.0 (whencjs-module-lexerWASM was replaced with the native implementation in nodejs/node#61456), triggered probabilistically by multiple worker threads concurrently doing ESM→CJS preparse. It was fixed by nodejs/node#63885 and released in Node 26.4.0, but the fix is not yet in any 24.x LTS release (24.18.0, which CI uses, was cut on 2026-06-23 from before the backport) — which is exactly why the Node 24 job crashed while 22 (no native lexer) and 26 (fix released) passed. Once the 24.x backport ships, CI picks it up automatically vianode-version: 24.It is not a defect in the test: the same file passed on Node 22/26 and on rerun, and the historical ELU-timing flakiness of this test was already addressed by #4806 and #4904 (recent
mainfailures are in other suites).The cost of one such crash is currently amplified by CI structure: the runtime matrix uses default
fail-fast: true, so a single flaky job cancels the other 29 matrix jobs and the whole matrix has to be rerun manually.Changes
fail-fast: falsefor the runtime matrix: a single flaky job no longer cancels the healthy ones, andgh run rerun --failedonly repeats the job that actually failed.nick-fields/retryaction already used forpnpm installin every job. This automates the current practice of rerunning failed runtime jobs before investigating. Persistent failures still fail the job (and now do so twice, making a real failure easier to distinguish from a flake).main) suite currently takes.No test code changes: the worker-scaler tests were already hardened against ELU-timing flakiness, and the V8 crash cannot be mitigated from test code since it kills the entire process.