fix(last-run): remove globalOutputDir (#30571)

Author: pavelfeldman

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<FullConfig['webServer']>[];
   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),

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<LastRunInfo> {
-  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: [] };
 }

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',