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

beforeAll block runs even if it is inside a describe.skip block #10451

Closed
capaj opened this issue Aug 26, 2020 · 12 comments · Fixed by #10806
Closed

beforeAll block runs even if it is inside a describe.skip block #10451

capaj opened this issue Aug 26, 2020 · 12 comments · Fixed by #10806

Comments

@capaj
Copy link
Contributor

capaj commented Aug 26, 2020

🐛 Bug Report

image

To Reproduce

describe.skip('CommentsSummaryEmail', () => {
  beforeAll(async () => {
    console.log('WTF are you doing jest')
  })
  it('should whatever', async () => {
    expect(0).toBe(0)
  })
  it.todo(
    `it whateves`
  )
})

Expected behavior

none of the block inside describe.skip run

Link to repl or repo (highly encouraged)

envinfo

  System:
    OS: Linux 5.4 Ubuntu 20.04.1 LTS (Focal Fossa)
    CPU: (16) x64 AMD Ryzen 7 3700X 8-Core Processor
  Binaries:
    Node: 12.14.1 - ~/.nvm/versions/node/v12.14.1/bin/node
    Yarn: 1.22.4 - ~/.yarn/bin/yarn
    npm: 6.13.4 - ~/.nvm/versions/node/v12.14.1/bin/npm
  npmPackages:
    jest: ^26.4.2 => 26.4.2 

@capaj capaj changed the title beforeAll block runs eve if it is inside a describe.skip block beforeAll block runs even if it is inside a describe.skip block Aug 26, 2020
@cdoublev
Copy link

cdoublev commented Nov 3, 2020

Side note: beforeEach also runs (when nested in a describe.skip).

EDIT: beforeEach is skipped as expected, sorry.

@cdoublev
Copy link

cdoublev commented Nov 4, 2020

I believe the title should be beforeAll: not skipped if describe contains a test.todo.

If you comment/remove it.todo('test 2') from the code below, all tests pass, otherwise test 3 fails.

let actual = 0

describe.skip('not skipped if it contains a test todo', () => {
    beforeAll(() => actual++)
    it('test 1', () => expect(actual).toBe(1))
    it.todo('test 2')
})

describe('not skipped', () => {
    it('test 3', () => expect(actual).toBe(0))
})

Side note: obviously a test should not rely on the result of another test but the purpose of the above code is to demonstrate a real-world scenario issue in a simple way.

I guess that node.markedTodo is missing here.

@SimenB
Copy link
Member

SimenB commented Nov 4, 2020

Good catch! Seems reasonable. jest-circus has the same issue, I'm guessing the logic is somewhere around here: https://github.com/facebook/jest/blob/3a8eeb91bca14717bb8e6f9f0ffa9279055c4594/packages/jest-circus/src/run.ts#L92-L105

@sarathps93
Copy link
Contributor

@SimenB @cdoublev Are you guys currently working on this issue? If not, shall I give it a shot?

@cdoublev
Copy link

cdoublev commented Nov 9, 2020

I'm not. I'm sorry. I was unable to install Jest in my Vagrant env since upgrading npm to v7. Can't speak for SimenB but I think you have all you need to fix it. My last comment is a guess after reading PR #9931, which you might find helpful as well.

@sarathps93
Copy link
Contributor

Thanks for the inputs @cdoublev , I will go through it and will start working on it

@sarathps93
Copy link
Contributor

sarathps93 commented Nov 9, 2020

Hi @SimenB @cdoublev I have fixed the issue related to the beforeAll and afterAll hook getting called when the test is marked with todo even if the describe block is skipped.

Additionally I have noticed that the same behaviour is happening if the test is having only specified. Trying to get a fix for this one as well

@SimenB
Copy link
Member

SimenB commented Nov 9, 2020

tests with only should run the lifecycle hooks

@sarathps93
Copy link
Contributor

@SimenB Even if it is inside a describe.skip block, should it still run?

describe.skip('Parent`, () => {
    beforeAll(() => console.log('Should not get displayed'));

    it.only('child test', () => {});
});

In the above case the beforeAll hook should not get executed right?

@SimenB
Copy link
Member

SimenB commented Nov 9, 2020

ah! yes, that's correct. skip of describe should override only. It's a separate issue though (happy to take a PR fixing both together of course 👍)

@sarathps93
Copy link
Contributor

Thanks @SimenB . I have done the fixes and will raise a PR after adding few unit tests.
I have addressed the below issues in both jest-circus & jest-jasmine2

  1. beforeAll block runs even if it is inside a describe.skip block when the it block contains tests marked as todo or only
  2. it tests run even if it is inside a describe.skip when the it tests are marked as todo or only

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants