Skip to content

fix(ci): pin Node from .node-version in the cargo-test jobs - #10383

Closed
proggeramlug wants to merge 2 commits into
mainfrom
fix/pin-node-in-cargo-test-jobs
Closed

proggeramlug wants to merge 2 commits into
mainfrom
fix/pin-node-in-cargo-test-jobs

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

cargo-test is red on main, and it is not the code under test:

AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
+ '22.23.2'
- '26.5.1'
    at scripts/test-bun-embedded-compression.mjs:10:8

crates/perry/tests/bun_embedded_compression.rs shells out to that script, whose first line of work is asserting process.versions.node equals the .node-version pin. That assertion is the point of the test — the loader and byte behaviour it gates on is version-specific — so it is doing its job. What is wrong is the job it runs in.

Neither cargo-test nor the cargo-test-perry shards ran setup-node, so both inherited whatever Node the ubuntu-latest image ships. Today that is 22.23.2 against a 26.5.1 pin. Every other job that runs node-backed tests — lint, e2e-scoped, windows-build — already had the step; these two were the gap.

Both are pinned here, not just the one observed failing. Which shard a node-backed test lands in is decided by ci_cargo_test_shard.py, so pinning only cargo-test would move the failure with the sharding rather than remove it.

Pinned from the file, never a literal: check_node_version_consistency.py is a lint step and requires every literal node-version: in a workflow to be a registered exemption, and this is not one.

Validation

  • python3 scripts/check_node_version_consistency.py → OK.
  • python3 scripts/check_gate_freshness.py → OK.
  • The workflow still parses as YAML, and the new step sits immediately before Run cargo test in both jobs.
  • The assertion itself passes under the pinned Node: with .node-version = 26.5.1 and node 26.5.1, assert.equal(process.versions.node, pin) succeeds. That is the whole of what this change affects; the rest of the test is untouched.

This is one of several independently red gates on main — it is not the only thing keeping pr-gate red, and it does not attempt to be.

Summary by CodeRabbit

  • Bug Fixes

    • Fixed Node.js version mismatches affecting Node-backed integration tests in CI.
    • Cargo test jobs now use the repository’s pinned Node.js version, preventing related test failures.
  • Documentation

    • Added a changelog entry documenting the Node.js version pinning for cargo test jobs.

Ralph Küpper added 2 commits September 16, 2026 17:47
crates/perry/tests/bun_embedded_compression.rs shells out to
scripts/test-bun-embedded-compression.mjs, whose first assertion is that
process.versions.node equals the .node-version pin — the loader and byte
behaviour it gates on is version-specific, so the assertion is the point of
the test, not incidental.

Neither the cargo-test job nor the cargo-test-perry shards ran setup-node, so
both inherited whatever node the ubuntu-latest image ships. That is 22.23.2
today against a 26.5.1 pin, which is why standalone_compressed_asset_regression
fails on main with

  AssertionError: + '22.23.2' - '26.5.1'
    at scripts/test-bun-embedded-compression.mjs:10:8

and takes the whole job red. Every other job that runs node-backed tests
(lint, e2e-scoped, windows-build) already had the step; these two were the gap.

Pinned from the FILE rather than a literal: check_node_version_consistency.py
is a lint step and requires every literal node-version: to be a registered
exemption. Both shards are pinned, not just the one observed failing — which
shard a node-backed test lands in is decided by ci_cargo_test_shard.py, so
pinning only one moves the failure with the sharding instead of removing it.
@coderabbitai

coderabbitai Bot commented Sep 16, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: c6684ddb-7d7a-4dcf-9a0a-92287a7672d1

📥 Commits

Reviewing files that changed from the base of the PR and between 33690c5 and b784262.

📒 Files selected for processing (2)
  • .github/workflows/test.yml
  • changelog.d/10383-pin-node-cargo-test.md

Included review availability: Your plan provides up to 8 included reviews per hour; 4 remain after this review.


📝 Walkthrough

Walkthrough

The cargo-test and cargo-test-perry jobs now install the Node.js version specified by .node-version before running integration tests. A changelog entry documents the CI change and its version-consistency rationale.

Changes

Node.js test environment

Layer / File(s) Summary
Configure Node.js for integration tests
.github/workflows/test.yml, changelog.d/10383-pin-node-cargo-test.md
Both cargo test jobs use actions/setup-node with .node-version before test execution. The changelog records the previous image-default behavior and the reason for pinning both jobs.

Priority: ➖ Normal

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: jdalton

Merge Risk: ⚪ Minimal · up to b7842

Both CI test jobs now use the pinned Node.js version, preventing the reported version-mismatch failures. No actionable merge risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main CI change: pinning Node from .node-version in both cargo-test jobs.
Description check ✅ Passed The description clearly explains the failure, root cause, scope, implementation, and validation. It covers the summary, changes, and test plan, but it does not use the template headings or provide an …
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/pin-node-in-cargo-test-jobs

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Landed via merge train #10397 (v0.5.1584). All source commits preserve authorship; merged main matches the validated train exactly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant