Skip to content

Conversation

@aduh95
Copy link
Contributor

@aduh95aduh95 commented Oct 5, 2025

Test assertions that are unreachable are pretty much useless, and it’s not always obvious whether a callback is actually executed. By adding a lint rule that forbids writing assertions in callbacks that are not wrapped with common.mustCall, we should end up with more robust tests.

This is only targeting a few test subdirs, the end goal would be to target more in follow up PRs and eventually the entire test/ dir.

ESlint failures in test/parallel
{glob: 'test/parallel/test-n*', result: '✖ 126 problems (126 errors, 0 warnings)', exitCode: 1 }{glob: 'test/parallel/test-o*', result: '✖ 8 problems (8 errors, 0 warnings)', exitCode: 1 }{glob: 'test/parallel/test-p*', result: '✖ 84 problems (84 errors, 0 warnings)', exitCode: 1 }{glob: 'test/parallel/test-q*', result: '✖ 8 problems (8 errors, 0 warnings)', exitCode: 1 }{glob: 'test/parallel/test-r*', result: '✖ 102 problems (102 errors, 0 warnings)', exitCode: 1 } 

@nodejs-github-botnodejs-github-bot added loaders Issues and PRs related to ES module loaders module Issues and PRs related to the module subsystem. needs-ci PRs that need a full CI run. node-api Issues and PRs related to the Node-API. test Issues and PRs related to the tests. tty Issues and PRs related to the tty subsystem. labels Oct 5, 2025
@codecov
Copy link

codecovbot commented Oct 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.52%. Comparing base (e978a63) to head (16ea131).
⚠️ Report is 17 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #60125 +/- ## ========================================== - Coverage 88.53% 88.52% -0.02%  ========================================== Files 703 703 Lines 207833 207825 -8 Branches 40011 40001 -10 ========================================== - Hits 184009 183974 -35 - Misses 15824 15858 +34 + Partials 8000 7993 -7 

see 28 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.

@aduh95aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Oct 6, 2025
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Oct 6, 2025
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@marco-ippolitomarco-ippolito added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. labels Oct 6, 2025
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Oct 7, 2025
@nodejs-github-botnodejs-github-bot merged commit ec26b1c into nodejs:mainOct 7, 2025
72 of 76 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in ec26b1c

@RafaelGSSRafaelGSS mentioned this pull request Oct 13, 2025
9 tasks
aduh95 added a commit that referenced this pull request Nov 11, 2025
PR-URL: #60125 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Marco Ippolito <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.commit-queue-squashAdd this label to instruct the Commit Queue to squash all the PR commits into the first one.dont-land-on-v20.xPRs that should not land on the v20.x-staging branch and should not be released in v20.x.loadersIssues and PRs related to ES module loadersmoduleIssues and PRs related to the module subsystem.needs-ciPRs that need a full CI run.node-apiIssues and PRs related to the Node-API.testIssues and PRs related to the tests.ttyIssues and PRs related to the tty subsystem.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants

@aduh95@nodejs-github-bot@cjihrig@marco-ippolito