From 4f4cf448c23760e0136faa9c08040d3772f0ad26 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Wed, 1 Sep 2021 13:41:35 -0700 Subject: [PATCH] fix(test runner): generate unique outputDir for beforeAll/afterAll (#8633) --- src/test/index.ts | 16 ++++-- src/test/testType.ts | 4 +- src/test/workerRunner.ts | 2 +- tests/playwright-test/playwright.spec.ts | 1 + tests/playwright-test/test-output-dir.spec.ts | 50 +++++++++++++++++++ 5 files changed, 67 insertions(+), 6 deletions(-) diff --git a/src/test/index.ts b/src/test/index.ts index 015efe416e..8bedfae348 100644 --- a/src/test/index.ts +++ b/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({ // 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({ }, { 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, diff --git a/src/test/testType.ts b/src/test/testType.ts index 875aa71949..351338e5bb 100644 --- a/src/test/testType.ts +++ b/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 { diff --git a/src/test/workerRunner.ts b/src/test/workerRunner.ts index 0be035f817..32baa162d0 100644 --- a/src/test/workerRunner.ts +++ b/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; diff --git a/tests/playwright-test/playwright.spec.ts b/tests/playwright-test/playwright.spec.ts index 2b5c68f79e..1cff43167d 100644 --- a/tests/playwright-test/playwright.spec.ts +++ b/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 }) => { diff --git a/tests/playwright-test/test-output-dir.spec.ts b/tests/playwright-test/test-output-dir.spec.ts index 5550ed00c3..debbf4c754 100644 --- a/tests/playwright-test/test-output-dir.spec.ts +++ b/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': `