test: ensure assertions are reachable in test/addons

[aduh95]

Follow up of #60125

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.56%. Comparing base (da9cd74) to head (72c550e).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #60142      +/-   ##
==========================================
- Coverage   88.56%   88.56%   -0.01%     
==========================================
  Files         704      704              
  Lines      208123   208123              
  Branches    40014    40013       -1     
==========================================
- Hits       184330   184318      -12     
- Misses      15815    15824       +9     
- Partials     7978     7981       +3     

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

[nodejs-github-bot]

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

[lpinca]

There are cases where common.mustCall() (& co.) is omitted on purpose because invariants, dedicated tests, and logic make it redundant. For example, it doesn't make much sense to use it every time an assertion is found in a setTimeout() callback if dedicated tests already exist for setTimeout(). For this reason I'm not a fan of these refactors that blindly add it everywhere. Sometimes it is only unnecessary overhead.

[aduh95]

For example, it doesn't make much sense to use it every time an assertion is found in a setTimeout() callback if dedicated tests already exist for setTimeout()

I've thought about making setTimeout part of the exceptions, but decided against it because there are many things that could cause a timer to never run its callback.

Sometimes it is only unnecessary overhead.

Sure but the other side of the coin is that sometimes it is actually useful and will catch a regression / bug as I did in #60125 (comment). I certainly agree that it adds redundancy, but arguably that's a good thing for tests.

For this reason I'm not a fan of these refactors that blindly add it everywhere.

I'm not sure "blindly" is fair, I've spent quite some time crafting this PR and gauging which of mustCall, mustCallAtLeast or eslint-disable-next-line was appropriate.

[nodejs-github-bot]

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

[nodejs-github-bot]

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