Jest: beforeAll block runs even if it is inside a describe.skip block

[capaj]

🐛 Bug Report

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 

[cdoublev]

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

EDIT: beforeEach is skipped as expected, sorry.

[cdoublev]

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]

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]

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

[cdoublev]

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]

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

[sarathps93]

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]

tests with only should run the lifecycle hooks

[sarathps93]

@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]

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]

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

[sarathps93]

@SimenB The build seems to have intermittent issues. Sometimes it is failing for Node15 version, sometimes for Node10 etc. I have already retriggered it 3-4 times and everytime it is failing in a different place. Just want to check how should I proceed, shall I keep on re-triggering until it passes or is there any other efficient way?