[Bug]: beforeEach and afterEach hooks not running in sync for tests that are skipped at runtime

[jthielAB]

Version

1.43.1

Steps to reproduce

We were able to reproduce with the following:

test.only("skip in automatic fixture should not run beforeEach/afterEach", async ({
  runInlineTest,
}) => {
  const result = await runInlineTest({
    "a.test.js": `
      import { test as base, expect } from '@playwright/test';
      const test = base.extend({
        foo: [async ({}, use, testInfo) => {
          test.skip();
          await use('foo');
        },{ auto: true }],
      });
      test.beforeAll(() => {
        console.log('\\n%%beforeAll');
      });
      test.beforeEach(() => {
        console.log('\\n%%beforeEach');
      });
      test('skipped1', () => {
        console.log('\\n%%test');
      });
      test('skipped2', () => {
        console.log('\\n%%test');
      });
      test.afterEach(() => {
        console.log('\\n%%afterEach');
      });
      test.afterAll(() => {
        console.log('\\n%%afterAll');
      });
    `,
  });
  expect(result.exitCode).toBe(0);
  expect(result.outputLines).toEqual(["beforeAll", "afterAll"]);
  expect(result.report.suites[0].specs[0].tests[0].annotations).toEqual([
    { type: "skip" },
  ]);
  expect(result.report.suites[0].specs[1].tests[0].annotations).toEqual([
    { type: "skip" },
  ]);
});

This test passes on v1.42.1 but not v1.43.1.

Expected behavior

If a test is skipped at runtime (e.g., by calling test.skip()), I expect the beforeEach and the afterEach hooks to either both be skipped or both run.

Actual behavior

If a test is skipped at runtime (e.g., by calling test.skip()), I am seeing that the beforeEach hooks are skipped and the afterEach hooks are run, which is causing unexpected test failures for the skipped tests.

Additional context

No response

Environment

System:
    OS: macOS 14.4
    CPU: (10) arm64 Apple M1 Pro
    Memory: 46.59 MB / 32.00 GB
  Binaries:
    Node: 16.20.2 - ~/.nvm/versions/node/v16.20.2/bin/node
    Yarn: 4.0.0-rc.44 - ~/.volta/bin/yarn
    npm: 8.19.4 - ~/.nvm/versions/node/v16.20.2/bin/npm
    pnpm: 9.0.2 - ~/.volta/bin/pnpm
  IDEs:
    VSCode: 1.88.1 - /opt/homebrew/bin/code
  Languages:
    Bash: 5.2.26 - /opt/homebrew/bin/bash
  npmPackages:
    @playwright/test: 1.43.1 => 1.43.1 
    playwright-json-summary-reporter: 1.0.1 => 1.0.1 
    playwright-merge-html-reports: 0.2.8 => 0.2.8 
    playwright-merge-summary-json-reports: 1.0.4 => 1.0.4

[jessewjoseph]

Similarly, if a beforeEach function calls test.skip() it will still try to execute subsequent beforeEach functions. It didn't do this in v1.42.1

test.only("skip in automatic fixture should not run beforeEach/afterEach", async ({
  runInlineTest,
}) => {
  const result = await runInlineTest({
    "a.test.js": `
      import { test, expect } from '@playwright/test';
      test.beforeEach(() => {
        test.skip();
        console.log('\\n%%beforeEach');
      });
      test.beforeEach(() => {
        console.log('\\n%%beforeEach2');
      });
      test('skipped1', () => {
        console.log('\\n%%test');
      });
    `,
  });
  expect(result.exitCode).toBe(0);
  expect(result.outputLines).toEqual([]);
  expect(result.report.suites[0].specs[0].tests[0].annotations).toEqual([
    { type: "skip" },
  ]);
});

[dgozman]

@jessewjoseph I've tried your snippet in v1.42.1 and v1.43.1 - both versions run all beforeEach hooks and skip the test.

This is by design, because the test runner cannot differentiate between hooks and does not have an idea which hooks should be skipped at which point. So far, the rule is "once we started running beforeEach hooks, we must run all of them and also run all afterEach hooks at the end". Otherwise, the rules would get complicated and/or we'll get into other edge cases that do not work for someone.

[jessewjoseph]

Dang, yeah that's right. I spent too long poking at this and got my wires crossed, sorry about that.

It's just the original issue John posted then where a skip in an auto fixture used to prevent beforeEach and afterEach hooks from executing, but now executes afterEach.