From bd280d73f4c7b633cc94307a15f8f3321417c11e Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Wed, 17 Apr 2024 16:27:32 -0700 Subject: [PATCH] fix: rebuild project tree from scratch when listing tests Instead of filtering tests assuming there are no two projects with same name we always rebuild test tree from scratch and restore previos test results in the list mode. Fixes https://github.com/microsoft/playwright/issues/30396 --- .../playwright/src/isomorphic/teleReceiver.ts | 36 +++---------- .../trace-viewer/src/ui/teleSuiteUpdater.ts | 45 ++++++++++++++-- .../playwright-test/ui-mode-test-tree.spec.ts | 54 +++++++++++++++++++ 3 files changed, 101 insertions(+), 34 deletions(-) diff --git a/packages/playwright/src/isomorphic/teleReceiver.ts b/packages/playwright/src/isomorphic/teleReceiver.ts index 4ea983ea02..2eeabcce22 100644 --- a/packages/playwright/src/isomorphic/teleReceiver.ts +++ b/packages/playwright/src/isomorphic/teleReceiver.ts @@ -146,6 +146,11 @@ export class TeleReporterReceiver { this._reporter = reporter; } + reset() { + this._rootSuite._entries = []; + this._tests.clear(); + } + dispatch(message: JsonEvent): Promise | void { const { method, params } = message; if (method === 'onConfigure') { @@ -206,35 +211,6 @@ export class TeleReporterReceiver { projectSuite._project = this._parseProject(project); for (const suite of project.suites) this._mergeSuiteInto(suite, projectSuite); - - // Remove deleted tests when listing. Empty suites will be auto-filtered - // in the UI layer. - if (this.isListing) { - const testIds = new Set(); - const collectIds = (suite: JsonSuite) => { - suite.entries.forEach(entry => { - if ('testId' in entry) - testIds.add(entry.testId); - else - collectIds(entry); - }); - }; - project.suites.forEach(collectIds); - - const filterTests = (suite: TeleSuite) => { - suite._entries = suite._entries.filter(entry => { - if (entry.type === 'test') { - if (testIds.has(entry.id)) - return true; - this._tests.delete(entry.id); - return false; - } - filterTests(entry); - return true; - }); - }; - filterTests(projectSuite); - } } private _onBegin() { @@ -585,7 +561,7 @@ class TeleTestStep implements reporterTypes.TestStep { } } -class TeleTestResult implements reporterTypes.TestResult { +export class TeleTestResult implements reporterTypes.TestResult { retry: reporterTypes.TestResult['retry']; parallelIndex: reporterTypes.TestResult['parallelIndex'] = -1; workerIndex: reporterTypes.TestResult['workerIndex'] = -1; diff --git a/packages/trace-viewer/src/ui/teleSuiteUpdater.ts b/packages/trace-viewer/src/ui/teleSuiteUpdater.ts index 600722db73..af1aac76c3 100644 --- a/packages/trace-viewer/src/ui/teleSuiteUpdater.ts +++ b/packages/trace-viewer/src/ui/teleSuiteUpdater.ts @@ -15,6 +15,7 @@ */ import { TeleReporterReceiver, TeleSuite } from '@testIsomorphic/teleReceiver'; +import type { TeleTestResult } from '@testIsomorphic/teleReceiver'; import { statusEx } from '@testIsomorphic/testTree'; import type { ReporterV2 } from 'playwright/src/reporters/reporterV2'; import type * as reporterTypes from 'playwright/types/testReporter'; @@ -27,7 +28,7 @@ export type TeleSuiteUpdaterOptions = { }; export class TeleSuiteUpdater { - rootSuite: reporterTypes.Suite | undefined; + rootSuite: TeleSuite | undefined; config: reporterTypes.FullConfig | undefined; readonly loadErrors: reporterTypes.TestError[] = []; readonly progress: Progress = { @@ -76,7 +77,7 @@ export class TeleSuiteUpdater { onBegin: (suite: reporterTypes.Suite) => { if (!this.rootSuite) - this.rootSuite = suite; + this.rootSuite = suite as TeleSuite; this.progress.total = this._lastRunTestCount; this.progress.passed = 0; this.progress.failed = 0; @@ -123,10 +124,13 @@ export class TeleSuiteUpdater { } processListReport(report: any[]) { - this._receiver.isListing = true; + // Save test results and reset all projects. + const results = this.rootSuite && TestResultsSnapshot.saveResults(this.rootSuite); + this._receiver.reset(); for (const message of report) this._receiver.dispatch(message); - this._receiver.isListing = false; + // After recreating all projects restore previous results. + results?.restore(this.rootSuite!) } processTestReportEvent(message: any) { @@ -145,3 +149,36 @@ export class TeleSuiteUpdater { }; } } + +class TestResultsSnapshot { + private _testIdToResults = new Map(); + + static saveResults(rootSuite: TeleSuite) { + const snapshot = new TestResultsSnapshot(); + snapshot._saveForSuite(rootSuite); + return snapshot; + } + + private _saveForSuite(suite: TeleSuite) { + for (const entry of suite.entries()) { + if (entry.type === 'test') { + if (entry.results.length) + this._testIdToResults.set(entry.id, entry.results); + } else { + this._saveForSuite(entry); + } + } + } + + restore(suite: TeleSuite) { + for (const entry of suite.entries()) { + if (entry.type === 'test') { + const results = this._testIdToResults.get(entry.id); + if (results) + entry.results = results; + } else { + this.restore(entry); + } + } + } +} diff --git a/tests/playwright-test/ui-mode-test-tree.spec.ts b/tests/playwright-test/ui-mode-test-tree.spec.ts index 0e0bc01f28..7346c204e4 100644 --- a/tests/playwright-test/ui-mode-test-tree.spec.ts +++ b/tests/playwright-test/ui-mode-test-tree.spec.ts @@ -48,6 +48,60 @@ test('should list tests', async ({ runUITest }) => { `); }); +test('should list all tests from projects with clashing names', async ({ runUITest }) => { + test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/30396' }); + const { page } = await runUITest({ + 'playwright.config.ts': ` + import { defineConfig } from '@playwright/test'; + + export default defineConfig({ + projects: [ + { + name: 'proj-uno', + testDir: './foo', + }, + { + name: 'proj-dos', + testDir: './foo', + }, + { + name: 'proj-uno', + testDir: './bar', + }, + { + name: 'proj-dos', + testDir: './bar', + }, + ] + }); + `, + 'foo/a.test.ts': ` + import { test, expect } from '@playwright/test'; + test('one', () => {}); + test('two', () => {}); + `, + 'bar/b.test.ts': ` + import { test, expect } from '@playwright/test'; + test('three', () => {}); + test('four', () => {}); + `, + }); + await page.getByTestId('test-tree').getByText('b.test.ts').click(); + await page.keyboard.press('ArrowRight'); + await page.getByTestId('test-tree').getByText('a.test.ts').click(); + await page.keyboard.press('ArrowRight'); + await expect.poll(dumpTestTree(page)).toBe(` + ▼ ◯ bar + ▼ ◯ b.test.ts + ◯ three + ◯ four + ▼ ◯ foo + ▼ ◯ a.test.ts <= + ◯ one + ◯ two + `); +}); + test('should traverse up/down', async ({ runUITest }) => { const { page } = await runUITest(basicTestTree); await page.getByText('a.test.ts').click();