Skip to content

test_runner: correctly filter --experimental-config-file#58833

Merged
nodejs-github-bot merged 1 commit intonodejs:mainfrom
pmarchini:fix/58828
Jun 29, 2025
Merged

test_runner: correctly filter --experimental-config-file#58833
nodejs-github-bot merged 1 commit intonodejs:mainfrom
pmarchini:fix/58828

Conversation

@pmarchini
Copy link
Member

@pmarchini pmarchini commented Jun 25, 2025 •
edited by JakobJingleheimer
Loading

Fixes #58828

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 25, 2025

Review requested:

  • @nodejs/test_runner

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels Jun 25, 2025
@JakobJingleheimer

This comment was marked as off-topic.

Sign in to view
@pmarchini pmarchini marked this pull request as ready for review June 27, 2025 11:58
@codecov
Copy link

codecov bot commented Jun 27, 2025 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.08%. Comparing base (100c6da) to head (ecb029f).
⚠️ Report is 560 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #58833      +/-   ##
==========================================
- Coverage   90.10%   90.08%   -0.02%     
==========================================
  Files         640      640              
  Lines      188384   188453      +69     
  Branches    36932    36970      +38     
==========================================
+ Hits       169735   169775      +40     
- Misses      11356    11395      +39     
+ Partials     7293     7283      -10
Files with missing lines Coverage Δ
lib/internal/test_runner/runner.js 92.70% <100.00%> (+0.05%) ⬆️

... and 51 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

JakobJingleheimer
JakobJingleheimer approved these changes Jun 27, 2025
View reviewed changes
Copy link
Member

@JakobJingleheimer JakobJingleheimer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙌 LGTM

@JakobJingleheimer JakobJingleheimer added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 27, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 27, 2025
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@pmarchini
Copy link
Member Author

pmarchini commented Jun 27, 2025

There's a related failing CI, I'll investigate ASAP

@JakobJingleheimer
Copy link
Member

JakobJingleheimer commented Jun 27, 2025

Ah, yes. Looks like test/parallel/test-runner-cli.js:476 AssertionError [ERR_ASSERTION]: Expected values to be strictly equal: undefined !== 1

@pmarchini
Copy link
Member Author

pmarchini commented Jun 28, 2025

https://github.com/nodejs/node/blob/main/test%2Fparallel%2Ftest-coverage-with-inspector-disabled.js <-- I keep forgetting this every time: coverage-related tests must be skipped under specific cases 😁 I'll update the test ASAP

@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@JakobJingleheimer
Copy link
Member

JakobJingleheimer commented Jun 28, 2025

I keep forgetting this every time: coverage-related tests must be skipped under specific cases 😁 I'll update the test ASAP

TIL 💡

@pmarchini pmarchini marked this pull request as draft June 28, 2025 11:32
@pmarchini pmarchini marked this pull request as ready for review June 28, 2025 11:44
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 28, 2025 •
edited by jasnell
Loading

CI: https://ci.nodejs.org/job/node-test-pull-request/67709/ 💛

@JakobJingleheimer JakobJingleheimer added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Jun 29, 2025
@pmarchini pmarchini requested a review from jakecastelli June 29, 2025 18:59
@JakobJingleheimer JakobJingleheimer added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 29, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 29, 2025
@jasnell jasnell added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 29, 2025
@nodejs-github-bot nodejs-github-bot merged commit 61a0b12 into nodejs:main Jun 29, 2025
90 of 92 checks passed
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jun 29, 2025

Landed in 61a0b12

targos pushed a commit that referenced this pull request Jul 3, 2025
@pmarchini @targos
test_runner: correctly filter --experimental-config-file
b5c7e64 
PR-URL: #58833
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
@github-actions github-actions bot mentioned this pull request Jul 8, 2025
Merged
@richardlau richardlau added the backport-requested-v22.x PRs awaiting manual backport to the v22.x-staging branch. label Sep 20, 2025
@richardlau
Copy link
Member

richardlau commented Sep 20, 2025

When cherry-picked onto v22.x-staging the test fails:

$ out/Release/node --test-reporter=./test/common/test-error-reporter.js --test-reporter-destination=stdout /home/local/rlau/sandbox/github/trees/v22.x/test/parallel/test-runner-cli.js
node:assert:150
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

undefined !== 1

    at Object.<anonymous> (/home/local/rlau/sandbox/github/trees/v22.x/test/parallel/test-runner-cli.js:477:12)
    at Module._compile (node:internal/modules/cjs/loader:1706:14)
    at Object..js (node:internal/modules/cjs/loader:1839:10)
    at Module.load (node:internal/modules/cjs/loader:1441:32)
    at Function._load (node:internal/modules/cjs/loader:1263:12)
    at TracingChannel.traceSync (node:diagnostics_channel:322:14)
    at wrapModuleLoad (node:internal/modules/cjs/loader:237:24)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:171:5)
    at node:internal/main/run_main_module:36:49 {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: undefined,
  expected: 1,
  operator: 'strictEqual',
  diff: 'simple'
}

Node.js v22.19.1-pre

I suspect it is dependent on other test runner commits that are not on v22.x-staging.

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

Reviewers

@jasnell jasnell jasnell approved these changes

@JakobJingleheimer JakobJingleheimer JakobJingleheimer approved these changes

@atlowChemi atlowChemi atlowChemi approved these changes

@cjihrig cjihrig Awaiting requested review from cjihrig

@MoLow MoLow Awaiting requested review from MoLow

@jakecastelli jakecastelli Awaiting requested review from jakecastelli

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. backport-requested-v22.x PRs awaiting manual backport to the v22.x-staging branch. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Setting "experimental-test-coverage": true within node.config.json causes multiple reports

6 participants

@pmarchini @nodejs-github-bot @JakobJingleheimer @richardlau @jasnell @atlowChemi