From ed5c21b827d19ca03e1bf38b2b15cb392bece625 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Fri, 30 Aug 2024 08:29:49 +0200 Subject: [PATCH] fix(ui): respect --output param (#32351) Closes https://github.com/microsoft/playwright/issues/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. --- .../src/isomorphic/testServerInterface.ts | 3 ++- packages/playwright/src/runner/testServer.ts | 6 ++++- packages/trace-viewer/src/ui/uiModeView.tsx | 8 +++--- .../ui-mode-test-setup.spec.ts | 26 +++++++++++++------ 4 files changed, 30 insertions(+), 13 deletions(-) diff --git a/packages/playwright/src/isomorphic/testServerInterface.ts b/packages/playwright/src/isomorphic/testServerInterface.ts index 0460ae56d9566..28f82688dc004 100644 --- a/packages/playwright/src/isomorphic/testServerInterface.ts +++ b/packages/playwright/src/isomorphic/testServerInterface.ts @@ -44,7 +44,7 @@ export interface TestServerInterface { installBrowsers(params: {}): Promise; - 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 index 7ed1d181917b6..474ed6ae5d1b0 100644 --- 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[0]): ReturnType { 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 index 0f799b2035a99..b88aebe6fc271 100644 --- 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 index e8809ddad947f..cd5503427d1f4 100644 --- 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 }) => {