fix(test runner): generate unique outputDir for beforeAll/afterAll (#8633)

src/test/index.ts

@@ -17,7 +17,7 @@
 import * as fs from 'fs';
 import * as path from 'path';
 import type { LaunchOptions, BrowserContextOptions, Page, BrowserContext, BrowserType } from '../../types/types';
-import type { TestType, PlaywrightTestArgs, PlaywrightTestOptions, PlaywrightWorkerArgs, PlaywrightWorkerOptions } from '../../types/test';
+import type { TestType, PlaywrightTestArgs, PlaywrightTestOptions, PlaywrightWorkerArgs, PlaywrightWorkerOptions, TestInfo } from '../../types/test';
 import { rootTestType } from './testType';
 import { createGuid, removeFolders } from '../utils/utils';
 import { TestInfoImpl } from './types';
@@ -242,7 +242,7 @@ export const test = _baseTest.extend<TestFixtures, WorkerAndFileFixtures>({
 
     // 3. Determine whether we need the artifacts.
     const testFailed = testInfo.status !== testInfo.expectedStatus;
-    const isHook = testInfo.title === 'beforeAll' || testInfo.title === 'afterAll';
+    const isHook = !!hookType(testInfo);
     const preserveTrace = captureTrace && !isHook && (trace === 'on' || (testFailed && trace === 'retain-on-failure') || (trace === 'on-first-retry' && testInfo.retry === 1));
     const captureScreenshots = !isHook && (screenshot === 'on' || (screenshot === 'only-on-failure' && testFailed));
 
@@ -294,8 +294,9 @@ export const test = _baseTest.extend<TestFixtures, WorkerAndFileFixtures>({
   }, { auto: true }],
 
   context: async ({ browser, video, _artifactsDir }, use, testInfo) => {
-    if (testInfo.title === 'beforeAll' || testInfo.title === 'afterAll')
-      throw new Error(`"context" and "page" fixtures are not supported in ${testInfo.title}. Use browser.newContext() instead.`);
+    const hook = hookType(testInfo);
+    if (hook)
+      throw new Error(`"context" and "page" fixtures are not supported in ${hook}. Use browser.newContext() instead.`);
 
     let videoMode = typeof video === 'string' ? video : video.mode;
     if (videoMode === 'retry-with-video')
@@ -367,6 +368,13 @@ function formatStackFrame(frame: StackFrame) {
   return `${file}:${frame.line || 1}:${frame.column || 1}`;
 }
 
+function hookType(testInfo: TestInfo): 'beforeAll' | 'afterAll' | undefined {
+  if (testInfo.title.startsWith('beforeAll'))
+    return 'beforeAll';
+  if (testInfo.title.startsWith('afterAll'))
+    return 'afterAll';
+}
+
 type StackFrame = {
   file: string,
   line?: number,

src/test/testType.ts

@@ -113,7 +113,9 @@ export class TestTypeImpl {
     if (!suite)
       throw errorWithLocation(location, `${name} hook can only be called in a test file`);
     if (name === 'beforeAll' || name === 'afterAll') {
-      const hook = new TestCase(name, name, fn, 0, this, location);
+      const sameTypeCount = suite._allHooks.filter(hook => hook._type === name).length;
+      const suffix = sameTypeCount ? String(sameTypeCount) : '';
+      const hook = new TestCase(name, name + suffix, fn, 0, this, location);
       hook._requireFile = suite._requireFile;
       suite._addAllHook(hook);
     } else {

src/test/workerRunner.ts

@@ -206,7 +206,7 @@ export class WorkerRunner extends EventEmitter {
     const baseOutputDir = (() => {
       const relativeTestFilePath = path.relative(this._project.config.testDir, test._requireFile.replace(/\.(spec|test)\.(js|ts|mjs)$/, ''));
       const sanitizedRelativePath = relativeTestFilePath.replace(process.platform === 'win32' ? new RegExp('\\\\', 'g') : new RegExp('/', 'g'), '-');
-      const fullTitleWithoutSpec = test.titlePath().slice(1).join(' ');
+      const fullTitleWithoutSpec = test.titlePath().slice(1).join(' ') + (test._type === 'test' ? '' : '-worker' + this._params.workerIndex);
       let testOutputDir = sanitizedRelativePath + '-' + sanitizeForFilePath(fullTitleWithoutSpec);
       if (this._uniqueProjectNamePathSegment)
         testOutputDir += '-' + this._uniqueProjectNamePathSegment;

tests/playwright-test/playwright.spec.ts

@@ -248,6 +248,7 @@ test('should throw when using page in beforeAll', async ({ runInlineTest }, test
   const result = await runInlineTest({
     'a.test.ts': `
       const { test } = pwt;
+      test.beforeAll(() => {});
       test.beforeAll(async ({ page }) => {
       });
       test('ok', async ({ page }) => {

tests/playwright-test/test-output-dir.spec.ts

@@ -83,6 +83,56 @@ test('should include repeat token', async ({runInlineTest}) => {
   expect(result.passed).toBe(3);
 });
 
+test('should be unique for beforeAll and afterAll hooks', async ({runInlineTest}, testInfo) => {
+  const result = await runInlineTest({
+    'a.spec.js': `
+      const { test } = pwt;
+      test.beforeAll(({}, testInfo) => {
+        console.log('\\n%%' + testInfo.outputDir);
+      });
+      test.beforeAll(({}, testInfo) => {
+        console.log('\\n%%' + testInfo.outputDir);
+      });
+      test.afterAll(({}, testInfo) => {
+        console.log('\\n%%' + testInfo.outputDir);
+      });
+      test.afterAll(({}, testInfo) => {
+        console.log('\\n%%' + testInfo.outputDir);
+      });
+      test.describe('suite', () => {
+        test.beforeAll(({}, testInfo) => {
+          console.log('\\n%%' + testInfo.outputDir);
+        });
+        test.afterAll(({}, testInfo) => {
+          console.log('\\n%%' + testInfo.outputDir);
+        });
+        test('fails', ({}, testInfo) => {
+          expect(1).toBe(2);
+        });
+        test('passes', ({}, testInfo) => {
+        });
+      });
+    `
+  });
+  expect(result.exitCode).toBe(1);
+  expect(result.passed).toBe(1);
+  expect(result.failed).toBe(1);
+  expect(result.output.split('\n').filter(x => x.startsWith('%%'))).toEqual([
+    `%%${testInfo.outputPath('test-results', 'a-beforeAll-worker0')}`,
+    `%%${testInfo.outputPath('test-results', 'a-beforeAll1-worker0')}`,
+    `%%${testInfo.outputPath('test-results', 'a-suite-beforeAll-worker0')}`,
+    `%%${testInfo.outputPath('test-results', 'a-suite-afterAll-worker0')}`,
+    `%%${testInfo.outputPath('test-results', 'a-afterAll-worker0')}`,
+    `%%${testInfo.outputPath('test-results', 'a-afterAll1-worker0')}`,
+    `%%${testInfo.outputPath('test-results', 'a-beforeAll-worker1')}`,
+    `%%${testInfo.outputPath('test-results', 'a-beforeAll1-worker1')}`,
+    `%%${testInfo.outputPath('test-results', 'a-suite-beforeAll-worker1')}`,
+    `%%${testInfo.outputPath('test-results', 'a-suite-afterAll-worker1')}`,
+    `%%${testInfo.outputPath('test-results', 'a-afterAll-worker1')}`,
+    `%%${testInfo.outputPath('test-results', 'a-afterAll1-worker1')}`,
+  ]);
+});
+
 test('should include the project name', async ({ runInlineTest }) => {
   const result = await runInlineTest({
     'helper.ts': `