fix(test runner): make sure to run afterAll after skipped tests (#19878)

Fixes #19745.

packages/playwright-test/src/workerRunner.ts

@@ -289,6 +289,7 @@ export class WorkerRunner extends EventEmitter {
 
     const suites = getSuites(test);
     const reversedSuites = suites.slice().reverse();
+    const nextSuites = new Set(getSuites(nextTest));
 
     // Inherit test.setTimeout() from parent suites, deepest has the priority.
     for (const suite of reversedSuites) {
@@ -312,8 +313,11 @@ export class WorkerRunner extends EventEmitter {
     this.emit('testBegin', buildTestBeginPayload(testInfo));
 
     const isSkipped = testInfo.expectedStatus === 'skipped';
-    if (isSkipped && nextTest) {
+    const hasAfterAllToRunBeforeNextTest = reversedSuites.some(suite => {
-      // Fast path - this test and skipped, and there are more tests that will handle cleanup.
+      return this._activeSuites.has(suite) && !nextSuites.has(suite) && suite._hooks.some(hook => hook.type === 'afterAll');
+    });
+    if (isSkipped && nextTest && !hasAfterAllToRunBeforeNextTest) {
+      // Fast path - this test is skipped, and there are more tests that will handle cleanup.
       testInfo.status = 'skipped';
       this.emit('testEnd', buildTestEndPayload(testInfo));
       return;
@@ -445,7 +449,6 @@ export class WorkerRunner extends EventEmitter {
       }
 
       // Run "afterAll" hooks for suites that are not shared with the next test.
-      const nextSuites = new Set(getSuites(nextTest));
       // In case of failure the worker will be stopped and we have to make sure that afterAll
       // hooks run before test fixtures teardown.
       for (const suite of reversedSuites) {

tests/playwright-test/hooks.spec.ts

@@ -844,3 +844,31 @@ test('afterEach exception after skipped test should be reported', async ({ runIn
   expect(result.failed).toBe(1);
   expect(result.output).toContain('Error: oh my!');
 });
+
+test('afterAll should be run for test.skip', async ({ runInlineTest }) => {
+  const result = await runInlineTest({
+    'a.test.js': `
+      const { test } = pwt;
+      test.describe('suite1', () => {
+        test.beforeAll(() => console.log('\\n%%beforeAll1'));
+        test.afterAll(() => console.log('\\n%%afterAll1'));
+        test('test1', () => console.log('\\n%%test1'));
+        test.skip('test2', () => {});
+        test.skip('test2.5', () => {});
+      });
+      test.describe('suite2', () => {
+        test.beforeAll(() => console.log('\\n%%beforeAll2'));
+        test.afterAll(() => console.log('\\n%%afterAll2'));
+        test('test3', () => console.log('\\n%%test3'));
+      });
+    `,
+  });
+  expect(result.output.split('\n').filter(line => line.startsWith('%%'))).toEqual([
+    '%%beforeAll1',
+    '%%test1',
+    '%%afterAll1',
+    '%%beforeAll2',
+    '%%test3',
+    '%%afterAll2',
+  ]);
+});