From 2d678869c9112fc660fea9d84bb8c280c5660e3f Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Fri, 9 Aug 2024 11:46:06 +0200 Subject: [PATCH 1/8] chore: add test for --only-changed with project dependencies --- tests/playwright-test/only-changed.spec.ts | 42 ++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/playwright-test/only-changed.spec.ts b/tests/playwright-test/only-changed.spec.ts index 45b750cbe4cdd..bfd359e309a5a 100644 --- a/tests/playwright-test/only-changed.spec.ts +++ b/tests/playwright-test/only-changed.spec.ts @@ -364,4 +364,46 @@ test('UI mode is not supported', async ({ runInlineTest }) => { const result = await runInlineTest({}, { 'only-changed': true, 'ui': true }); expect(result.exitCode).toBe(1); expect(result.output).toContain('--only-changed is not supported in UI mode'); +}); + +test('should run project dependencies of changed tests', async ({ runInlineTest, git, writeFiles }) => { + await writeFiles({ + 'playwright.config.ts': ` + module.exports = { + projects: [ + { name: 'setup', testMatch: 'setup.ts', }, + { name: 'main', dependencies: ['setup'] }, + ], + }; + `, + 'setup.ts': ` + console.log("setup is run") + `, + 'a.spec.ts': ` + import { test, expect } from '@playwright/test'; + test('fails', () => { expect(1).toBe(2); }); + `, + 'b.spec.ts': ` + import { test, expect } from '@playwright/test'; + test('fails', () => { expect(1).toBe(2); }); + `, + }); + + git(`add .`); + git(`commit -m init`); + + const result = await runInlineTest({ + 'c.spec.ts': ` + import { test, expect } from '@playwright/test'; + test('fails', () => { expect(1).toBe(2); }); + ` + }, { 'only-changed': true }); + + expect(result.exitCode).toBe(1); + expect(result.failed).toBe(1); + expect(result.passed).toBe(0); + + console.log(result.output); + expect(result.output).toContain('setup is run'); + expect(result.output).toContain('c.spec.ts'); }); \ No newline at end of file From 7d1bd3c817f85f56c03d138819eca90c683da9ac Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Mon, 12 Aug 2024 14:13:50 +0200 Subject: [PATCH 2/8] repro test --- tests/playwright-test/only-changed.spec.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/tests/playwright-test/only-changed.spec.ts b/tests/playwright-test/only-changed.spec.ts index bfd359e309a5a..cdd36089d83d9 100644 --- a/tests/playwright-test/only-changed.spec.ts +++ b/tests/playwright-test/only-changed.spec.ts @@ -371,13 +371,17 @@ test('should run project dependencies of changed tests', async ({ runInlineTest, 'playwright.config.ts': ` module.exports = { projects: [ - { name: 'setup', testMatch: 'setup.ts', }, + { name: 'setup', testMatch: 'setup.spec.ts', }, { name: 'main', dependencies: ['setup'] }, ], }; `, - 'setup.ts': ` - console.log("setup is run") + 'setup.spec.ts': ` + import { test, expect } from '@playwright/test'; + + test('setup test', async ({ page }) => { + console.log('setup test is executed') + }); `, 'a.spec.ts': ` import { test, expect } from '@playwright/test'; @@ -401,9 +405,7 @@ test('should run project dependencies of changed tests', async ({ runInlineTest, expect(result.exitCode).toBe(1); expect(result.failed).toBe(1); - expect(result.passed).toBe(0); + expect(result.passed).toBe(1); - console.log(result.output); - expect(result.output).toContain('setup is run'); - expect(result.output).toContain('c.spec.ts'); + expect(result.output).toContain('setup test is executed'); }); \ No newline at end of file From 853d6e46434252a1b8865a468a1723a9f08f534c Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Mon, 12 Aug 2024 14:14:59 +0200 Subject: [PATCH 3/8] naive fix --- packages/playwright/src/runner/loadUtils.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/playwright/src/runner/loadUtils.ts b/packages/playwright/src/runner/loadUtils.ts index 4cac21cd08106..c19c0af3ba764 100644 --- a/packages/playwright/src/runner/loadUtils.ts +++ b/packages/playwright/src/runner/loadUtils.ts @@ -123,6 +123,7 @@ export async function createRootSuite(testRun: TestRun, errors: TestError[], sho const config = testRun.config; // Create root suite, where each child will be a project suite with cloned file suites inside it. const rootSuite = new Suite('', 'root'); + const unfilteredProjectSuites = new Map(); const projectSuites = new Map(); const filteredProjectSuites = new Map(); @@ -136,6 +137,7 @@ export async function createRootSuite(testRun: TestRun, errors: TestError[], sho // Filter file suites for all projects. for (const [project, fileSuites] of testRun.projectSuites) { + unfilteredProjectSuites.set(project, createProjectSuite(project, fileSuites)); const filteredFileSuites = additionalFileMatcher ? fileSuites.filter(fileSuite => additionalFileMatcher(fileSuite.location!.file)) : fileSuites; const projectSuite = createProjectSuite(project, filteredFileSuites); projectSuites.set(project, projectSuite); @@ -202,7 +204,7 @@ export async function createRootSuite(testRun: TestRun, errors: TestError[], sho // Clone file suites for dependency projects. for (const project of projectClosure.keys()) { if (projectClosure.get(project) === 'dependency') - rootSuite._prependSuite(buildProjectSuite(project, projectSuites.get(project)!)); + rootSuite._prependSuite(buildProjectSuite(project, unfilteredProjectSuites.get(project)!)); } } From 46432f35e82d9010f331d8d5ff437e4c77d69045 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Mon, 12 Aug 2024 14:15:50 +0200 Subject: [PATCH 4/8] refactor --- packages/playwright/src/runner/loadUtils.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/playwright/src/runner/loadUtils.ts b/packages/playwright/src/runner/loadUtils.ts index c19c0af3ba764..5192fa88247b7 100644 --- a/packages/playwright/src/runner/loadUtils.ts +++ b/packages/playwright/src/runner/loadUtils.ts @@ -202,8 +202,8 @@ export async function createRootSuite(testRun: TestRun, errors: TestError[], sho const projectClosure = new Map(buildProjectsClosure(rootSuite.suites.map(suite => suite._fullProject!))); // Clone file suites for dependency projects. - for (const project of projectClosure.keys()) { - if (projectClosure.get(project) === 'dependency') + for (const [project, level] of projectClosure.entries()) { + if (level === 'dependency') rootSuite._prependSuite(buildProjectSuite(project, unfilteredProjectSuites.get(project)!)); } } From 5638e9565002cde50a7d613ad34a4f38a6521d91 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Mon, 12 Aug 2024 14:20:20 +0200 Subject: [PATCH 5/8] refactor --- packages/playwright/src/runner/loadUtils.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/playwright/src/runner/loadUtils.ts b/packages/playwright/src/runner/loadUtils.ts index 5192fa88247b7..3096879906f42 100644 --- a/packages/playwright/src/runner/loadUtils.ts +++ b/packages/playwright/src/runner/loadUtils.ts @@ -123,7 +123,6 @@ export async function createRootSuite(testRun: TestRun, errors: TestError[], sho const config = testRun.config; // Create root suite, where each child will be a project suite with cloned file suites inside it. const rootSuite = new Suite('', 'root'); - const unfilteredProjectSuites = new Map(); const projectSuites = new Map(); const filteredProjectSuites = new Map(); @@ -137,11 +136,10 @@ export async function createRootSuite(testRun: TestRun, errors: TestError[], sho // Filter file suites for all projects. for (const [project, fileSuites] of testRun.projectSuites) { - unfilteredProjectSuites.set(project, createProjectSuite(project, fileSuites)); + projectSuites.set(project, createProjectSuite(project, fileSuites)); + const filteredFileSuites = additionalFileMatcher ? fileSuites.filter(fileSuite => additionalFileMatcher(fileSuite.location!.file)) : fileSuites; - const projectSuite = createProjectSuite(project, filteredFileSuites); - projectSuites.set(project, projectSuite); - const filteredProjectSuite = filterProjectSuite(projectSuite, { cliFileFilters, cliTitleMatcher, testIdMatcher: config.testIdMatcher }); + const filteredProjectSuite = filterProjectSuite(createProjectSuite(project, filteredFileSuites), { cliFileFilters, cliTitleMatcher, testIdMatcher: config.testIdMatcher }); filteredProjectSuites.set(project, filteredProjectSuite); } } @@ -204,7 +202,7 @@ export async function createRootSuite(testRun: TestRun, errors: TestError[], sho // Clone file suites for dependency projects. for (const [project, level] of projectClosure.entries()) { if (level === 'dependency') - rootSuite._prependSuite(buildProjectSuite(project, unfilteredProjectSuites.get(project)!)); + rootSuite._prependSuite(buildProjectSuite(project, projectSuites.get(project)!)); } } From 97c4096d6153e31bd15b1dfd815ebd4b9b9e61ac Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Mon, 12 Aug 2024 14:25:27 +0200 Subject: [PATCH 6/8] add issue annotation --- tests/playwright-test/only-changed.spec.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/playwright-test/only-changed.spec.ts b/tests/playwright-test/only-changed.spec.ts index cdd36089d83d9..355bcf977d205 100644 --- a/tests/playwright-test/only-changed.spec.ts +++ b/tests/playwright-test/only-changed.spec.ts @@ -366,7 +366,12 @@ test('UI mode is not supported', async ({ runInlineTest }) => { expect(result.output).toContain('--only-changed is not supported in UI mode'); }); -test('should run project dependencies of changed tests', async ({ runInlineTest, git, writeFiles }) => { +test('should run project dependencies of changed tests', { + annotation: { + type: 'issue', + description: 'https://github.com/microsoft/playwright/issues/32070', + }, +}, async ({ runInlineTest, git, writeFiles }) => { await writeFiles({ 'playwright.config.ts': ` module.exports = { From 1899d2745a6d856da518634aa1e8fe0a08cc5344 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Mon, 12 Aug 2024 15:04:34 +0200 Subject: [PATCH 7/8] refactor: dont create suite twice --- packages/playwright/src/runner/loadUtils.ts | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/playwright/src/runner/loadUtils.ts b/packages/playwright/src/runner/loadUtils.ts index 3096879906f42..10d08d1d2099e 100644 --- a/packages/playwright/src/runner/loadUtils.ts +++ b/packages/playwright/src/runner/loadUtils.ts @@ -26,7 +26,7 @@ import type { Matcher, TestFileFilter } from '../util'; import { buildProjectsClosure, collectFilesForProject, filterProjects } from './projectUtils'; import type { TestRun } from './tasks'; import { requireOrImport } from '../transform/transform'; -import { applyRepeatEachIndex, bindFileSuiteToProject, filterByFocusedLine, filterByTestIds, filterOnly, filterTestsRemoveEmptySuites } from '../common/suiteUtils'; +import { applyRepeatEachIndex, bindFileSuiteToProject, filterByFile, filterByFocusedLine, filterByTestIds, filterOnly, filterTestsRemoveEmptySuites } from '../common/suiteUtils'; import { createTestGroups, filterForShard, type TestGroup } from './testGroups'; import { dependenciesForTestFile } from '../transform/compilationCache'; import { sourceMapSupport } from '../utilsBundle'; @@ -136,10 +136,10 @@ export async function createRootSuite(testRun: TestRun, errors: TestError[], sho // Filter file suites for all projects. for (const [project, fileSuites] of testRun.projectSuites) { - projectSuites.set(project, createProjectSuite(project, fileSuites)); + const projectSuite = createProjectSuite(project, fileSuites); + projectSuites.set(project, projectSuite); - const filteredFileSuites = additionalFileMatcher ? fileSuites.filter(fileSuite => additionalFileMatcher(fileSuite.location!.file)) : fileSuites; - const filteredProjectSuite = filterProjectSuite(createProjectSuite(project, filteredFileSuites), { cliFileFilters, cliTitleMatcher, testIdMatcher: config.testIdMatcher }); + const filteredProjectSuite = filterProjectSuite(projectSuite, { cliFileFilters, cliTitleMatcher, testIdMatcher: config.testIdMatcher, additionalFileMatcher }); filteredProjectSuites.set(project, filteredProjectSuite); } } @@ -225,9 +225,9 @@ function createProjectSuite(project: FullProjectInternal, fileSuites: Suite[]): return projectSuite; } -function filterProjectSuite(projectSuite: Suite, options: { cliFileFilters: TestFileFilter[], cliTitleMatcher?: Matcher, testIdMatcher?: Matcher }): Suite { +function filterProjectSuite(projectSuite: Suite, options: { cliFileFilters: TestFileFilter[], cliTitleMatcher?: Matcher, testIdMatcher?: Matcher, additionalFileMatcher?: Matcher }): Suite { // Fast path. - if (!options.cliFileFilters.length && !options.cliTitleMatcher && !options.testIdMatcher) + if (!options.cliFileFilters.length && !options.cliTitleMatcher && !options.testIdMatcher && !options.additionalFileMatcher) return projectSuite; const result = projectSuite._deepClone(); @@ -238,6 +238,8 @@ function filterProjectSuite(projectSuite: Suite, options: { cliFileFilters: Test filterTestsRemoveEmptySuites(result, (test: TestCase) => { if (options.cliTitleMatcher && !options.cliTitleMatcher(test._grepTitle())) return false; + if (options.additionalFileMatcher && !options.additionalFileMatcher(test.location.file)) + return false; return true; }); return result; From 32305e97eedad60777fb7171ca7cb1ee94f941a1 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Mon, 12 Aug 2024 16:36:22 +0200 Subject: [PATCH 8/8] remove unused import --- packages/playwright/src/runner/loadUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/playwright/src/runner/loadUtils.ts b/packages/playwright/src/runner/loadUtils.ts index 10d08d1d2099e..cd735ceca39c5 100644 --- a/packages/playwright/src/runner/loadUtils.ts +++ b/packages/playwright/src/runner/loadUtils.ts @@ -26,7 +26,7 @@ import type { Matcher, TestFileFilter } from '../util'; import { buildProjectsClosure, collectFilesForProject, filterProjects } from './projectUtils'; import type { TestRun } from './tasks'; import { requireOrImport } from '../transform/transform'; -import { applyRepeatEachIndex, bindFileSuiteToProject, filterByFile, filterByFocusedLine, filterByTestIds, filterOnly, filterTestsRemoveEmptySuites } from '../common/suiteUtils'; +import { applyRepeatEachIndex, bindFileSuiteToProject, filterByFocusedLine, filterByTestIds, filterOnly, filterTestsRemoveEmptySuites } from '../common/suiteUtils'; import { createTestGroups, filterForShard, type TestGroup } from './testGroups'; import { dependenciesForTestFile } from '../transform/compilationCache'; import { sourceMapSupport } from '../utilsBundle';