fix(ui): respect --output param (#32351)

Closes #32331 We're already passing the `outputDir` param to the UI, but the UI isn't passing it back to the TestServer. This PR fixes that. I've added it to `listTests`, which is requires to that `TestServerDispatcher#_ignoredProjectOutputs` is populated with the correct output dir. And i've added it to `runGlobalSetup`, which is what the bug report was about.
microsoft · Aug 30, 2024 · ed5c21b · ed5c21b
1 parent 90e7b9e
commit ed5c21b
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 13 deletions.
diff --git a/packages/playwright/src/isomorphic/testServerInterface.ts b/packages/playwright/src/isomorphic/testServerInterface.ts
@@ -44,7 +44,7 @@ export interface TestServerInterface {
 
   installBrowsers(params: {}): Promise<void>;
 
-  runGlobalSetup(params: {}): Promise<{
+  runGlobalSetup(params: { outputDir?: string }): Promise<{
     report: ReportEntry[],
     status: reporterTypes.FullResult['status']
   }>;
@@ -81,6 +81,7 @@ export interface TestServerInterface {
     locations?: string[];
     grep?: string;
     grepInvert?: string;
+    outputDir?: string;
   }): Promise<{
     report: ReportEntry[],
     status: reporterTypes.FullResult['status']

diff --git a/packages/playwright/src/runner/testServer.ts b/packages/playwright/src/runner/testServer.ts
@@ -148,7 +148,10 @@ class TestServerDispatcher implements TestServerInterface {
   async runGlobalSetup(params: Parameters<TestServerInterface['runGlobalSetup']>[0]): ReturnType<TestServerInterface['runGlobalSetup']> {
     await this.runGlobalTeardown();
 
-    const { config, error } = await this._loadConfig();
+    const overrides: ConfigCLIOverrides = {
+      outputDir: params.outputDir,
+    };
+    const { config, error } = await this._loadConfig(overrides);
     if (!config) {
       const { reporter, report } = await this._collectingInternalReporter();
       // Produce dummy config when it has an error.
@@ -256,6 +259,7 @@ class TestServerDispatcher implements TestServerInterface {
     const overrides: ConfigCLIOverrides = {
       repeatEach: 1,
       retries: 0,
+      outputDir: params.outputDir,
     };
     const { config, error } = await this._loadConfig(overrides);
     if (!config) {

diff --git a/packages/trace-viewer/src/ui/uiModeView.tsx b/packages/trace-viewer/src/ui/uiModeView.tsx
@@ -205,12 +205,14 @@ export const UIModeView: React.FC<{}> = ({
           interceptStdio: true,
           watchTestDirs: true
         });
-        const { status, report } = await testServerConnection.runGlobalSetup({});
+        const { status, report } = await testServerConnection.runGlobalSetup({
+          outputDir: queryParams.outputDir,
+        });
         teleSuiteUpdater.processGlobalReport(report);
         if (status !== 'passed')
           return;
 
-        const result = await testServerConnection.listTests({ projects: queryParams.projects, locations: queryParams.args, grep: queryParams.grep, grepInvert: queryParams.grepInvert });
+        const result = await testServerConnection.listTests({ projects: queryParams.projects, locations: queryParams.args, grep: queryParams.grep, grepInvert: queryParams.grepInvert, outputDir: queryParams.outputDir });
         teleSuiteUpdater.processListReport(result.report);
 
         testServerConnection.onReport(params => {
@@ -333,7 +335,7 @@ export const UIModeView: React.FC<{}> = ({
       commandQueue.current = commandQueue.current.then(async () => {
         setIsLoading(true);
         try {
-          const result = await testServerConnection.listTests({ projects: queryParams.projects, locations: queryParams.args, grep: queryParams.grep, grepInvert: queryParams.grepInvert });
+          const result = await testServerConnection.listTests({ projects: queryParams.projects, locations: queryParams.args, grep: queryParams.grep, grepInvert: queryParams.grepInvert, outputDir: queryParams.outputDir });
           teleSuiteUpdater.processListReport(result.report);
         } catch (e) {
           // eslint-disable-next-line no-console

diff --git a/tests/playwright-test/ui-mode-test-setup.spec.ts b/tests/playwright-test/ui-mode-test-setup.spec.ts
@@ -19,7 +19,7 @@ import path from 'path';
 
 test.describe.configure({ mode: 'parallel', retries });
 
-test('should run global setup and teardown', async ({ runUITest }) => {
+test('should run global setup and teardown', async ({ runUITest }, testInfo) => {
   const { page, testProcess } = await runUITest({
     'playwright.config.ts': `
       import { defineConfig } from '@playwright/test';
@@ -29,26 +29,36 @@ test('should run global setup and teardown', async ({ runUITest }) => {
       });
     `,
     'globalSetup.ts': `
-      export default () => console.log('\\n%%from-global-setup');
+      import { basename } from "node:path";
+      export default (config) => {
+        console.log('\\n%%from-global-setup');
+        console.log("setupOutputDir: " + basename(config.projects[0].outputDir));
+      };
     `,
     'globalTeardown.ts': `
-      export default () => console.log('\\n%%from-global-teardown');
+      export default (config) => {
+        console.log('\\n%%from-global-teardown');
+        console.log('%%' + JSON.stringify(config));
+      };
     `,
     'a.test.js': `
       import { test, expect } from '@playwright/test';
       test('should work', async ({}) => {});
     `
-  });
+  }, undefined, { additionalArgs: ['--output=foo'] });
   await page.getByTitle('Run all').click();
   await expect(page.getByTestId('status-line')).toHaveText('1/1 passed (100%)');
 
   await page.getByTitle('Toggle output').click();
-  await expect(page.getByTestId('output')).toContainText('from-global-setup');
+  const output = page.getByTestId('output');
+  await expect(output).toContainText('from-global-setup');
+  await expect(output).toContainText('setupOutputDir: foo');
   await page.close();
 
-  await expect.poll(() => testProcess.outputLines()).toEqual([
-    'from-global-teardown',
-  ]);
+  await expect.poll(() => testProcess.outputLines()).toContain('from-global-teardown');
+
+  const teardownConfig = JSON.parse(testProcess.outputLines()[1]);
+  expect(teardownConfig.projects[0].outputDir).toEqual(testInfo.outputPath('foo'));
 });
 
 test('should teardown on sigint', async ({ runUITest, nodeVersion }) => {