8000 Report skipped tests upon beforeAll hook failure by juergba · Pull Request #4221 · mochajs/mocha · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Report skipped tests upon beforeAll hook failure #4221

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Report skipped tests upon beforeAll hook failure #4221

wants to merge 6 commits into from

Conversation

juergba
Copy link
Contributor
@juergba juergba commented Apr 1, 2020 •
edited
Loading

Description

related: #4233

describe("parent suite", function() {
  before(function() {
    throw new Error("this is a failure in a before hook");
  });

  it("test 1", function() { });
  it("test 2", function() { });

  describe("inner suite", function() {
    it("test 3", function() { });
  });
})

Before: a failing before hook ignores the three tests completely. They disappear and are not even summarised in the epilogue.

parent suite
  1) "before all" hook for "test 1"

0 passing (15ms)
1 failing

1) parent suite
     "before all" hook for "test 1":
   Error: this is a failure in a before hook
   [...]

After:

parent suite
  1) "before all" hook for "test 1"
  - test 1
  - test 2
  inner suite
    - test 3

0 / 3 passing (15ms)
3 skipped
1 failing

1) parent suite
     "before all" hook for "test 1":
  Error: this is a failure in a before hook
  [...]

Description of the Change

  • add a new event EVENT_TEST_SKIPPED
  • add a new test.state: skipped
  • beforeAll hook: each skipped test is reported and summerised in the epilogue. The output is very similar to pending test cases, but in color red.
  • there are no changes to the hook pattern, skipped tests will not be executed.
  • adapt reporter SPEC and JSON

We now have four test states:

  • passes
  • pending: set by the user
    • test without callback it('some test')
    • static: it.skip() or describe.skip()
    • conditional: this.skip()
  • skipped: test not run because of a failing hook
  • failed: test failed or hook failed

We are aware of pending not being an optimal label. Nevertheless we will stick to it for historical reasons, it has been established for many years.

Applicable issues

ToDo:

  • afterEach hooks
  • reporter HTML

#1955
#1815

@juergba juergba force-pushed the juergba/hook-failure branch 2 times, most recently from acf8687 to 3b59dd8 Compare April 9, 2020 08:11
@coveralls
Copy link
coveralls commented Apr 9, 2020 •
edited
Loading

Coverage Status

Coverage increased (+0.08%) to 93.139% when pulling 06fc1d0 on juergba/hook-failure into 38d579a on master.

@juergba juergba force-pushed the juergba/hook-failure branch from 3b59dd8 to 9bcd7fc Compare April 9, 2020 13:52
@juergba juergba changed the title List skipped tests on beforeAll hook failure Report skipped tests upon beforeAll hook failure Apr 15, 2020
@juergba juergba force-pushed the juergba/hook-failure branch 2 times, most recently from a85e55f to 9febfaf Compare April 16, 2020 20:35
@juergba juergba self-assigned this Apr 17, 2020
@juergba juergba requested a review from a team April 17, 2020 07:33
@juergba juergba added block status: needs review a maintainer should (re-)review this pull request type: cleanup a refactor area: reporters involving a specific reporter semver-major implementation requires increase of "major" version number; "breaking changes" area: usability concerning user experience or interface labels Apr 17, 2020
@juergba juergba marked this pull request as ready for review April 17, 2020 07:35
@juergba juergba force-pushed the juergba/hook-failure branch from 9febfaf to 989fae1 Compare April 18, 2020 06:40
@juergba juergba force-pushed the juergba/hook-failure branch from 989fae1 to 06fc1d0 Compare April 20, 2020 05:36
This was referenced Apr 20, 2020
Closed
Open
@boneskull
Copy link
Member

I'm confused about this vs. #4223

@thomasbrooks4
Copy link

@boneskull I don't know if this is actually much different then that issue described, but I forced failure at a beforeEach statement. I got no other pending or skipped test cases for the tests omitted after the failure while using Mocha 8.1.1

@abhinaba-ghosh
Copy link

Any update on this? would be really helpful for multiple e2e testing workflows.

@juergba juergba removed the status: needs review a maintainer should (re-)review this pull request label Aug 31, 2021
@jerler1
Copy link
jerler1 commented Sep 19, 2023

Any updates on this? Could really use this update.

@JoshuaKGoldberg
Copy link
Member

Per #1815 (comment), marking this and #4223 as status: blocked for now. We can take a deeper look once we've gone through some smaller changes first.

@JoshuaKGoldberg JoshuaKGoldberg added the status: blocked Waiting for something else to be resolved label Mar 1, 2024
@voxpelli voxpelli requested review from Copilot and removed request for a team June 12, 2025 07:46
Copilot
Copilot AI reviewed Jun 12, 2025
View reviewed changes
Copy link
@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR extends Mocha’s reporting functionality to include tests skipped due to failing before hooks. Key changes include:

  • Introducing a new event (EVENT_TEST_SKIPPED) and a new test state ("skipped") to differentiate tests skipped because of hook failures.
  • Updating reporters (spec, JSON, base) and the stats-collector to report and display skipped tests.
  • Adjusting test fixtures and helpers to validate the new behavior.

Reviewed Changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/integration/hook-err.spec.js Updated tests to verify proper reporting of skipped tests for various hook error scenarios.
test/integration/helpers.js Modified helper to include and parse the new "skipped" count in the summary output.
test/integration/glob.spec.js Adjusted assertions to work with updated expected output strings.
lib/suite.js, lib/runnable.js Added new "skipped" property and corresponding isSkipped() methods.
lib/stats-collector.js Updated to count skipped tests in the overall stats.
lib/runner.js Modified hook and test execution flows to emit and handle skipped test events.
lib/reporters/* Updated reporters to display the new skipped tests count and log skipped test titles.
docs/index.md Added documentation on the behavior change when hooks fail, leading to skipped tests.
Comments suppressed due to low confidence (3)

test/integration/helpers.js:310

  • When parsing the output summary, consider using parseInt(match[1], 10) instead of parseInt(match, 10) to ensure the captured group is correctly converted to a number.
summary[type] = match ? parseInt(match, 10) : 0;

lib/reporters/spec.js:92

  • [nitpick] Since EVENT_TEST_SKIPPED uses the red fail color to display skipped tests, consider clarifying in the reporter docs or using a distinct visual cue to differentiate these from genuinely failing tests.
runner.on(EVENT_TEST_SKIPPED, function(test) {

docs/index.md:450

  • [nitpick] Consider adding further clarification that tests marked as 'skipped' due to hook failures are treated differently from user-declared pending tests, ensuring users understand the distinction in test outcomes.
### Failing Hooks

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

Copilot code review Copilot Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
area: reporters involving a specific reporter area: usability concerning user experience or interface semver-major implementation requires increase of "major" version number; "breaking changes" status: blocked Waiting for something else to be resolved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@juergba @coveralls @boneskull @thomasbrooks4 @abhinaba-ghosh @jerler1 @JoshuaKGoldberg
0