feat: add --fail-hook-affected-tests option to report skipped tests as failed

[akshah123]

When before() or beforeEach() hooks fail, Mocha currently only reports the hook failure and silently skips affected tests. This makes it difficult to track test coverage and understand the full impact of hook failures.

This change introduces a new CLI option --fail-hook-affected-tests that, when enabled, reports all tests affected by hook failures as failed instead of silently skipping them.

Changes:

• Added --fail-hook-affected-tests boolean CLI option
• Implemented failAffectedTests() method in Runner to fail all affected tests
• Updated hook() method to call failAffectedTests when hooks fail
• Added failHookAffectedTests option to Mocha class
• Added integration tests for both before() and beforeEach() hook failures
• All affected tests now receive descriptive error messages indicating which hook caused them to be skipped

Fixes #4392

PR Checklist

• Addresses an existing open issue: fixes 🚀 Feature: Failures in before() and beforeAll() should cause all impacted tests to be reported as failed #4392
• That issue was marked as status: accepting prs
• Steps in CONTRIBUTING.md were taken

Overview

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: mark-wiemer / name: Mark Wiemer (20f38d8, b9df13b, cd44988, 72381b7, b7b7041)

[mark-wiemer]

Hmm, I think we can ignore the CLA issue as the non-signer is AI :D

[mark-wiemer]

I'll review this in more detail sometime in the next ~7 days :)

[codecov]

Codecov Report

❌ Patch coverage is 92.59259% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.40%. Comparing base (1972dd7) to head (72381b7).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/mocha.js	33.33%	2 Missing ⚠️
lib/runner.js	96.07%	2 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5519   +/-   ##
=======================================
  Coverage   88.40%   88.40%           
=======================================
  Files          66       66           
  Lines        4734     4787   +53     
  Branches      964      977   +13     
=======================================
+ Hits         4185     4232   +47     
- Misses        549      555    +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 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.

[mark-wiemer]

Overall looks OK, just requesting some renames for clarity. I'll check the coverage and make sure I can repro this one locally as it's a big change. Fortunately we should be able to include this in 12.0.0-beta-1 :)

[mark-wiemer]

Took me a minute to remember I needed to actively add the --fail-hook-affected-tests flag to see the new behavior :D was able to repro locally and the code does look good. Going to wait for @JoshuaKGoldberg to review, hopefully early next week.

[JoshuaKGoldberg]

Generally looking good - the approach is solid and respects existing patterns. Great! 🙌

Requesting changes on the [Bug] around throwing non-Error values. Otherwise we're looking pretty good to merge soon.

@mark-wiemer shall we back-port this to v11?

[akshah123]

@JoshuaKGoldberg Thanks for the thorough review! I've addressed both of your comments in the latest commit (c171cf1):

Re: [Docs] JSDoc placement - Fixed! The JSDoc now correctly documents failHookAffectedTests and has been separated from failZero's documentation. Each function now has its own properly positioned JSDoc comment.

Re: [Bug] Non-Error handling - Great catch! The createHookSkipError function now properly handles:

• Falsy values (null/undefined) using createInvalidExceptionError
• Non-Error objects (strings, numbers, etc.) using thrown2Error

This follows the existing patterns from runner.js:1112 and runner.js:537. I've also added comprehensive test coverage for these edge cases with new fixture files testing null, undefined, string, and number throws in both before and beforeEach hooks.

All tests are passing, including the 2 new integration tests specifically for non-Error handling. ✅

[JoshuaKGoldberg]

LGTM. One little touchup but we can always apply this before merging. Thanks! ⭐

[mark-wiemer]

@akshah123 looks like your latest commit broke some stuff, please review the failed checks.

@JoshuaKGoldberg happy to port this back to v11, created #5580

[akshah123]

Sorry about that @mark-wiemer . I didn't verify syntax issues with suggested changes. It should be fixed now.

[mark-wiemer]

Still LGTM to me, great work :) running tests now to make sure nothing weird changed, then I plan to apply my suggestions and run tests once more. @JoshuaKGoldberg , OK to merge this into 12.0 or should we wait until 12.1? Obviously the flag is still disabled by default so I don't foresee issues and would love to merge this into 12.0 :)

[JoshuaKGoldberg]

If we merge it into 12, I think we should probably also back-port it to 11. 👍 from me!

[mark-wiemer]

Yep, we already have #5580 :)

[JoshuaKGoldberg]

heck yes