Skip to content
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.

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

Report skipped tests upon beforeAll hook failure #4221

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

Conversation

juergba
Copy link
Contributor

@juergba juergba commented Apr 1, 2020

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

Coverage Status

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

@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
@boneskull
Copy link
Contributor

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

7 participants