From fb90797d7308e9016151c99794c45c0fd47802f1 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Mon, 29 Apr 2024 09:02:54 -0700 Subject: [PATCH] fix(last-run): remove globalOutputDir (#30571) --- packages/playwright/src/common/config.ts | 13 ++++--------- packages/playwright/src/runner/runner.ts | 16 ++++++++++++---- .../playwright-test/playwright.artifacts.spec.ts | 8 ++++++++ 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/packages/playwright/src/common/config.ts b/packages/playwright/src/common/config.ts index 31819bdeccb31..7bc56cf4f6dba 100644 --- a/packages/playwright/src/common/config.ts +++ b/packages/playwright/src/common/config.ts @@ -41,10 +41,8 @@ export const defaultTimeout = 30000; export class FullConfigInternal { readonly config: FullConfig; - readonly globalOutputDir: string; readonly configDir: string; readonly configCLIOverrides: ConfigCLIOverrides; - readonly preserveOutputDir: boolean; readonly webServers: NonNullable[]; readonly plugins: TestRunnerPluginRegistration[]; readonly projects: FullProjectInternal[] = []; @@ -63,13 +61,10 @@ export class FullConfigInternal { const { resolvedConfigFile, configDir } = location; const packageJsonPath = getPackageJsonPath(configDir); - const packageJsonDir = packageJsonPath ? path.dirname(packageJsonPath) : undefined; - const throwawayArtifactsPath = packageJsonDir || process.cwd(); + const packageJsonDir = packageJsonPath ? path.dirname(packageJsonPath) : process.cwd(); this.configDir = configDir; this.configCLIOverrides = configCLIOverrides; - this.globalOutputDir = takeFirst(configCLIOverrides.outputDir, pathResolve(configDir, userConfig.outputDir), throwawayArtifactsPath, path.resolve(process.cwd())); - this.preserveOutputDir = configCLIOverrides.preserveOutputDir || false; const privateConfiguration = (userConfig as any)['@playwright/test']; this.plugins = (privateConfiguration?.plugins || []).map((p: any) => ({ factory: p })); @@ -128,7 +123,7 @@ export class FullConfigInternal { } const projectConfigs = configCLIOverrides.projects || userConfig.projects || [userConfig]; - this.projects = projectConfigs.map(p => new FullProjectInternal(configDir, userConfig, this, p, this.configCLIOverrides, throwawayArtifactsPath)); + this.projects = projectConfigs.map(p => new FullProjectInternal(configDir, userConfig, this, p, this.configCLIOverrides, packageJsonDir)); resolveProjectDependencies(this.projects); this._assignUniqueProjectIds(this.projects); setTransformConfig({ @@ -167,7 +162,7 @@ export class FullProjectInternal { deps: FullProjectInternal[] = []; teardown: FullProjectInternal | undefined; - constructor(configDir: string, config: Config, fullConfig: FullConfigInternal, projectConfig: Project, configCLIOverrides: ConfigCLIOverrides, throwawayArtifactsPath: string) { + constructor(configDir: string, config: Config, fullConfig: FullConfigInternal, projectConfig: Project, configCLIOverrides: ConfigCLIOverrides, packageJsonDir: string) { this.fullConfig = fullConfig; const testDir = takeFirst(pathResolve(configDir, projectConfig.testDir), pathResolve(configDir, config.testDir), fullConfig.configDir); const defaultSnapshotPathTemplate = '{snapshotDir}/{testFileDir}/{testFileName}-snapshots/{arg}{-projectName}{-snapshotSuffix}{ext}'; @@ -176,7 +171,7 @@ export class FullProjectInternal { this.project = { grep: takeFirst(projectConfig.grep, config.grep, defaultGrep), grepInvert: takeFirst(projectConfig.grepInvert, config.grepInvert, null), - outputDir: takeFirst(configCLIOverrides.outputDir, pathResolve(configDir, projectConfig.outputDir), pathResolve(configDir, config.outputDir), path.join(throwawayArtifactsPath, 'test-results')), + outputDir: takeFirst(configCLIOverrides.outputDir, pathResolve(configDir, projectConfig.outputDir), pathResolve(configDir, config.outputDir), path.join(packageJsonDir, 'test-results')), // Note: we either apply the cli override for repeatEach or not, depending on whether the // project is top-level vs dependency. See collectProjectsAndTestFiles in loadUtils. repeatEach: takeFirst(projectConfig.repeatEach, config.repeatEach, 1), diff --git a/packages/playwright/src/runner/runner.ts b/packages/playwright/src/runner/runner.ts index 05d3640ac5e14..467911dd411be 100644 --- a/packages/playwright/src/runner/runner.ts +++ b/packages/playwright/src/runner/runner.ts @@ -154,18 +154,26 @@ export type LastRunInfo = { }; async function writeLastRunInfo(testRun: TestRun, status: FullResult['status']) { - await fs.promises.mkdir(testRun.config.globalOutputDir, { recursive: true }); - const lastRunReportFile = path.join(testRun.config.globalOutputDir, 'last-run.json'); + const [project] = filterProjects(testRun.config.projects, testRun.config.cliProjectFilter); + if (!project) + return; + const outputDir = project.project.outputDir; + await fs.promises.mkdir(outputDir, { recursive: true }); + const lastRunReportFile = path.join(outputDir, '.last-run.json'); const failedTests = testRun.rootSuite?.allTests().filter(t => !t.ok()).map(t => t.id); const lastRunReport = JSON.stringify({ status, failedTests }, undefined, 2); await fs.promises.writeFile(lastRunReportFile, lastRunReport); } export async function readLastRunInfo(config: FullConfigInternal): Promise { - const lastRunReportFile = path.join(config.globalOutputDir, 'last-run.json'); + const [project] = filterProjects(config.projects, config.cliProjectFilter); + if (!project) + return { status: 'passed', failedTests: [] }; + const outputDir = project.project.outputDir; try { + const lastRunReportFile = path.join(outputDir, '.last-run.json'); return JSON.parse(await fs.promises.readFile(lastRunReportFile, 'utf8')) as LastRunInfo; - } catch (e) { + } catch { } return { status: 'passed', failedTests: [] }; } diff --git a/tests/playwright-test/playwright.artifacts.spec.ts b/tests/playwright-test/playwright.artifacts.spec.ts index 8a6db7fd2e62c..b666aa4b70e6d 100644 --- a/tests/playwright-test/playwright.artifacts.spec.ts +++ b/tests/playwright-test/playwright.artifacts.spec.ts @@ -136,6 +136,7 @@ test('should work with screenshot: on', async ({ runInlineTest }, testInfo) => { expect(result.passed).toBe(5); expect(result.failed).toBe(5); expect(listFiles(testInfo.outputPath('test-results'))).toEqual([ + '.last-run.json', 'artifacts-failing', ' test-failed-1.png', 'artifacts-own-context-failing', @@ -175,6 +176,7 @@ test('should work with screenshot: only-on-failure', async ({ runInlineTest }, t expect(result.passed).toBe(5); expect(result.failed).toBe(5); expect(listFiles(testInfo.outputPath('test-results'))).toEqual([ + '.last-run.json', 'artifacts-failing', ' test-failed-1.png', 'artifacts-own-context-failing', @@ -209,6 +211,7 @@ test('should work with screenshot: only-on-failure & fullPage', async ({ runInli expect(result.passed).toBe(0); expect(result.failed).toBe(1); expect(listFiles(testInfo.outputPath('test-results'))).toEqual([ + '.last-run.json', 'artifacts-should-fail-and-take-fullPage-screenshots', ' test-failed-1.png', ]); @@ -230,6 +233,7 @@ test('should work with trace: on', async ({ runInlineTest }, testInfo) => { expect(result.passed).toBe(5); expect(result.failed).toBe(5); expect(listFiles(testInfo.outputPath('test-results'))).toEqual([ + '.last-run.json', 'artifacts-failing', ' trace.zip', 'artifacts-own-context-failing', @@ -265,6 +269,7 @@ test('should work with trace: retain-on-failure', async ({ runInlineTest }, test expect(result.passed).toBe(5); expect(result.failed).toBe(5); expect(listFiles(testInfo.outputPath('test-results'))).toEqual([ + '.last-run.json', 'artifacts-failing', ' trace.zip', 'artifacts-own-context-failing', @@ -290,6 +295,7 @@ test('should work with trace: on-first-retry', async ({ runInlineTest }, testInf expect(result.passed).toBe(5); expect(result.failed).toBe(5); expect(listFiles(testInfo.outputPath('test-results'))).toEqual([ + '.last-run.json', 'artifacts-failing-retry1', ' trace.zip', 'artifacts-own-context-failing-retry1', @@ -315,6 +321,7 @@ test('should work with trace: on-all-retries', async ({ runInlineTest }, testInf expect(result.passed).toBe(5); expect(result.failed).toBe(5); expect(listFiles(testInfo.outputPath('test-results'))).toEqual([ + '.last-run.json', 'artifacts-failing-retry1', ' trace.zip', 'artifacts-failing-retry2', @@ -350,6 +357,7 @@ test('should work with trace: retain-on-first-failure', async ({ runInlineTest } expect(result.passed).toBe(5); expect(result.failed).toBe(5); expect(listFiles(testInfo.outputPath('test-results'))).toEqual([ + '.last-run.json', 'artifacts-failing', ' trace.zip', 'artifacts-own-context-failing',