🚀 Feature: Failures in before() and beforeAll() should cause all impacted tests to be reported as failed

[jmiller-airfox]

Description

every other testing suite that i've come across in my career (including, since it's a peer-ish library to mocha, jest) will consistently report results for all tests on each run - more specifically, it will show a test failure (or error, which is distinct from a "failure", depending on the framework) for each test in the suite(s) you're trying to execute, regardless of whether that failure occurred in a setup phase or in the test itself. mocha, however, appears to roll test setup failures that occur in before/beforeEach hooks into a single "failure", effectively swallowing counts of all the tests that were sidelined by the setup failure. this makes it rather frustrating to compare outputs between runs, understand the number of tests impacted by that setup failure at a glance, and can make reporting metrics from test runs somewhat meaningless due to the variance in the number of tests actually being reported on.

additionally, it seems that a failure in beforeEach will just circuit-break the execution of that setup function entirely, instead of retrying for subsequent tests, which honestly contravenes the name and intent of the function in the context of software testing conventions.

Steps to Reproduce

• on any test suite with a before or beforeEach hook, introduce an intentional failure in the hook.
  • for your reference, i've thrown together a simple demonstration
    • with failing before
    • with failing beforeEach
• run npm test
• --reporter options don't appear to have any affect on this

Expected behavior:

• a suite containing N tests (regardless of how they're nested in subsuites) should report on the outcome of N tests, even if there is a failure in a before or beforeEach hook.
• examples (modified from output of failing runs) of what i'd expect to see:
  • before
  • beforeEach

Actual behavior: [What actually happens]

• a suite with a failure in a before or beforeEach hook will only report a single failure.
  • before
  • beforeEach

Reproduces how often: 100% - it seems to be by design..? so perhaps this should be a feature request instead?

Versions

• mocha --version: 8.0.1
• node node_modules/.bin/mocha --version: 8.0.1
• node --version: v12.18.1
• Operating system
  • name and version: osX Catalina 10.15.5
  • architecture (32 or 64-bit): x64
• Shell (e.g., bash, zsh, PowerShell, cmd): zsh 5.7.1 (x86_64-apple-darwin19.0)
• browser and version: N/A
• Any third-party Mocha-related modules (and their versions): N/A
• Any code transpiler (e.g., TypeScript, CoffeeScript, Babel) being used (and its version): none

[boneskull]

If before or beforeEach fails, would you expect the tests to be run or would you just expect them to be reported?

[boneskull]

Some notes:

• If you have a failing hook at or near the root of your test suites, the reporter output will repeat the same error, potentially many, many times; we should consider showing a unique error once in the reporter's epilogue.
• This would likely be a high-impact breaking change for tools built upon Mocha
• This may require changes to built-in reporters AND third-party reporters
• There is arguably a conceptual difference between a hook failure and a test failure (seems like a philosophical opinion, to me). Understanding whether a test is failing because its assertions are failing, or if it's failing because an associated "hook" is failing may become murky. Mocha is very explicit about this, as you've discovered. :smile:
• Knowing how many tests are supposed to be run vs. how many were actually run is something Mocha does not do, but probably should. This isn't the same as enumerating all tests up-front, because we don't necessarily have that information (parallel mode will lazily load test files; the best we can do is consider each file individually). The root suite for any given test file will "know" about how many suites and tests are contained therein before running them, however, and this could be aggregated.

Assuming you just want tests reported if their hooks fail, this could be implemented in a non-breaking, opt-in manner: Mocha's runner could emit new events. Third-party reporters could then output this additional information.

[jmiller-airfox]

• i agree; repeating the same error ad nauseum isn't terribly helpful.
• definitely agree, at least if it is defined as the new default and isn't switchable.
• i would expect as much, since the contract between them would potentially be changing.
• i also agree that clearly and explicitly differentiating between hook and test failures is a good idea - again, the use case i'm trying to express here isn't necessarily that they should be called a "failed test", but simply that each test affected (i.e. that was expected to run but couldn't) is reported on.
• segueing from the above point: what you are talking about in the last point is basically what i was trying to express here: the consistent reporting of tests that are expected to run vs actually run. showing the before/beforeEach stacktrack/error once is totally fine and reasonable, but reporting the actual tests downstream of that error as "failed" (or "blocked", or whatever other term to indicate they couldn't even start execution, though it should be distinct from "skipped" imo) would be a pleasant improvement in specificity.

also, i definitely understand that changing this sort of functionality (and specifically, making it the new default) could totally hose a lot of people's projects / CI/CD, but as you also mention above, making the feature an opt-in switch on the runner would be a really nice and relatively non-disruptive way to do this.

thanks for your detailed response!

[DJ-Glock]

Hello I would like to say that reporting of tests that were not actually run because of failed hook is very important for us too. So it will be great if the following can be implemented, for example with some additional flag like --hook-fails-test-report:

Assuming you just want tests reported if their hooks fail, this could be implemented in a non-breaking, opt-in manner: Mocha's runner could emit new events. Third-party reporters could then output this additional information.

[jmiller-airfox]

@DJ-Glock the quick-and-dirty approach we ended up using to end-run this problem was to just wrap the contents of any before/beforeEach hook in a try/catch, save any error that occurs, and then check if any error was saved at the beginning of each test case. Yes, it's a gross hack, but at the end of the day it yields the behavior we need.

[DJ-Glock]

@jmiller-airfox OMG! Thanks for your workaround. And we just replaced before/after hooks with test where it was possible. But it won't work for beforeEach/afterEach.

We also did it because we use global hook afterEach for taking screenshot for every failed test. So sometimes before hooks may fail, but hook afterEach is not called for hooks so it could be hard to understand what happened without screenshot. But it's an off topic. :)

[alexmi256]

I think the original post explains why the current behavior is bad and the only things I can add are:

• I can understand that if before fails then the whole suite should not be run, however the whole short circuiting early means that we never know about the tests which were not run. Maybe a good compromise would be to mark all such subsequent tests as skipped.
• beforeEach should be retried. Grouping errors would be nice to be done somehow but I'd take duplicate errors any day over missing tests.

The try...catch hack is ugly and I might have to implement it because I can't think of a better way to do it.

[machty]

@DJ-Glock I like your idea of introducing this change/fix behind a flag/configuration.

When I'm doing large refactors, it's very useful/important to be able to report progress based on the remaining number of failing tests, and the current fast-fail beforeEach behavior greatly obscures the number of failing/passing/total specs, so I'd definitely love a way to opt into the new behavior under discussion.