From e1189a96b6e314365b453af6a89122cd8b5b2609 Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Mon, 21 Nov 2022 09:23:28 -0800 Subject: [PATCH] feat(runner): run all setup files if none matched the filter (#18922) The behavior regarding filters (both in config, command line and .only) is the following: - if some of tests match and none of setup match then we'll run all setup files and all matching tests - otherwise the filters apply to setup files the same way as to regular tests --- packages/playwright-test/src/runner.ts | 57 +++++--- tests/playwright-test/project-setup.spec.ts | 136 +++++++++++++++++++- 2 files changed, 169 insertions(+), 24 deletions(-) diff --git a/packages/playwright-test/src/runner.ts b/packages/playwright-test/src/runner.ts index b4ae1b7544..2f71cca462 100644 --- a/packages/playwright-test/src/runner.ts +++ b/packages/playwright-test/src/runner.ts @@ -29,7 +29,7 @@ import type { TestRunnerPlugin } from './plugins'; import { setRunnerToAddPluginsTo } from './plugins'; import { dockerPlugin } from './plugins/dockerPlugin'; import { webServerPluginsForConfig } from './plugins/webServerPlugin'; -import { formatError, relativeFilePath } from './reporters/base'; +import { formatError } from './reporters/base'; import DotReporter from './reporters/dot'; import EmptyReporter from './reporters/empty'; import GitHubReporter from './reporters/github'; @@ -272,12 +272,12 @@ export class Runner { filesByProject.set(project, testFiles); } - // If the filter didn't match any tests, apply it to the setup files. - const applyFilterToSetup = setupFiles.size === fileToProjectName.size; + // If none of the setup files matched the filter, we inlude all of them, otherwise + // only those that match the filter. + const applyFilterToSetup = !!commandLineFileFilters.length && [...setupFiles].some(commandLineFileMatcher); if (applyFilterToSetup) { - // We now have only setup files in filesByProject, because all test files were filtered out. - for (const [project, setupFiles] of filesByProject) { - const filteredFiles = setupFiles.filter(commandLineFileMatcher); + for (const [project, files] of filesByProject) { + const filteredFiles = files.filter(commandLineFileMatcher); if (filteredFiles.length) filesByProject.set(project, filteredFiles); else @@ -287,15 +287,6 @@ export class Runner { if (!commandLineFileMatcher(file)) setupFiles.delete(file); } - } else if (commandLineFileFilters.length) { - const setupFile = [...setupFiles].find(commandLineFileMatcher); - // If the filter is not empty and it matches both setup and tests then it's an error: we allow - // to run either subset of tests with full setup or partial setup without any tests. - if (setupFile) { - const testFile = Array.from(fileToProjectName.keys()).find(f => !setupFiles.has(f)); - const config = this._loader.fullConfig(); - throw new Error(`Both setup and test files match command line filter.\n Setup file: ${relativeFilePath(config, setupFile)}\n Test file: ${relativeFilePath(config, testFile!)}`); - } } return { filesByProject, setupFiles, applyFilterToSetup }; @@ -329,7 +320,6 @@ export class Runner { filterByFocusedLine(preprocessRoot, options.testFileFilters, applyFilterToSetup ? new Set() : setupFiles); // Complain about only. - // TODO: check in project setup. if (config.forbidOnly) { const onlyTestsAndSuites = preprocessRoot._getOnlyItems(); if (onlyTestsAndSuites.length > 0) @@ -350,6 +340,13 @@ export class Runner { const grepMatcher = createTitleMatcher(project.grep); const grepInvertMatcher = project.grepInvert ? createTitleMatcher(project.grepInvert) : null; + const titleMatcher = (test: TestCase) => { + const grepTitle = test.titlePath().join(' '); + if (grepInvertMatcher?.(grepTitle)) + return false; + return grepMatcher(grepTitle) && options.testTitleMatcher(grepTitle); + }; + const projectSuite = new Suite(project.name, 'project'); projectSuite._projectConfig = project; if (project._fullyParallel) @@ -361,15 +358,35 @@ export class Runner { continue; for (let repeatEachIndex = 0; repeatEachIndex < project.repeatEach; repeatEachIndex++) { const builtSuite = this._loader.buildFileSuiteForProject(project, fileSuite, repeatEachIndex, test => { - const grepTitle = test.titlePath().join(' '); - if (grepInvertMatcher?.(grepTitle)) - return false; - return grepMatcher(grepTitle) && options.testTitleMatcher(grepTitle); + if (setupFiles.has(test._requireFile)) + return true; + return titleMatcher(test); }); if (builtSuite) projectSuite._addSuite(builtSuite); } } + + // At this point projectSuite contains all setup tests (unfiltered) and all regular + // tests matching the filter. + if (projectSuite.allTests().some(test => !setupFiles.has(test._requireFile))) { + // If >0 tests match and + // - none of the setup files match the filter then we run all setup files, + // - if the filter also matches some of the setup tests, we'll run only + // that maching subset of setup tests. + const filterMatchesSetup = projectSuite.allTests().some(test => { + if (!setupFiles.has(test._requireFile)) + return false; + return titleMatcher(test); + }); + if (filterMatchesSetup) { + filterSuiteWithOnlySemantics(projectSuite, () => false, test => { + if (!setupFiles.has(test._requireFile)) + return true; + return titleMatcher(test); + }); + } + } } const allTestGroups = createTestGroups(rootSuite.suites, config.workers); diff --git a/tests/playwright-test/project-setup.spec.ts b/tests/playwright-test/project-setup.spec.ts index 992763d1b6..ec9be6c1ed 100644 --- a/tests/playwright-test/project-setup.spec.ts +++ b/tests/playwright-test/project-setup.spec.ts @@ -461,6 +461,38 @@ test('should allow .only in setup files', async ({ runGroups }, testInfo) => { expect(passed).toBe(2); }); +test('should allow .only in both setup and test files', async ({ runGroups }, testInfo) => { + const files = { + 'playwright.config.ts': ` + module.exports = { + projects: [ + { + name: 'p1', + setup: /.*.setup.ts/, + }, + ] + };`, + 'a.test.ts': ` + const { test } = pwt; + test('test1', async () => { }); + test.only('test2', async () => { }); + test('test3', async () => { }); + test('test4', async () => { }); + `, + 'a.setup.ts': ` + const { test } = pwt; + test.only('setup1', async () => { }); + test('setup2', async () => { }); + test('setup3', async () => { }); + `, + }; + + const { exitCode, output } = await runGroups(files); + expect(exitCode).toBe(0); + expect(output).toContain('[p1] › a.setup.ts:5:12 › setup1'); + expect(output).toContain('[p1] › a.test.ts:7:12 › test2'); +}); + test('should allow filtering setup by file:line', async ({ runGroups }, testInfo) => { const files = { 'playwright.config.ts': ` @@ -517,7 +549,7 @@ test('should allow filtering setup by file:line', async ({ runGroups }, testInfo } }); -test('should prohibit filters matching both setup and test', async ({ runGroups }, testInfo) => { +test('should support filters matching both setup and test', async ({ runGroups }, testInfo) => { const files = { 'playwright.config.ts': ` module.exports = { @@ -539,11 +571,67 @@ test('should prohibit filters matching both setup and test', async ({ runGroups test('setup1', async () => { }); test('setup2', async () => { }); `, + 'b.setup.ts': ` + const { test } = pwt; + test('setup1', async () => { }); + `, + 'b.test.ts': ` + const { test } = pwt; + test('test1', async () => { }); + `, }; - const { exitCode, output } = await runGroups(files, undefined, undefined, { additionalArgs: ['.*ts$'] }); - expect(output).toContain('Error: Both setup and test files match command line filter.'); - expect(exitCode).toBe(1); + const { exitCode, output } = await runGroups(files, undefined, undefined, { additionalArgs: ['.*a.(setup|test).ts$'] }); + expect(exitCode).toBe(0); + expect(output).toContain('Running 5 tests using 1 worker'); + expect(output).toContain('[p1] › a.setup.ts:5:7 › setup1'); + expect(output).toContain('[p1] › a.setup.ts:6:7 › setup2'); + expect(output).toContain('[p1] › a.test.ts:6:7 › test1'); + expect(output).toContain('[p1] › a.test.ts:7:7 › test2'); + expect(output).toContain('[p1] › a.test.ts:8:7 › test3'); +}); + +test('should run setup for a project if tests match only in another project', async ({ runGroups }, testInfo) => { + const files = { + 'playwright.config.ts': ` + module.exports = { + projects: [ + { + name: 'p1', + testMatch: /.*a.test.ts/, + setup: /.*a.setup.ts/, + }, + { + name: 'p2', + testMatch: /.*b.test.ts/, + setup: /.*b.setup.ts/, + }, + ] + };`, + 'a.test.ts': ` + const { test } = pwt; + test('test1', async () => { }); + `, + 'a.setup.ts': ` + const { test } = pwt; + test('setup1', async () => { }); + `, + 'b.setup.ts': ` + const { test } = pwt; + test('setup1', async () => { }); + `, + 'b.test.ts': ` + const { test } = pwt; + test('test1', async () => { }); + `, + }; + + const { exitCode, output } = await runGroups(files, undefined, undefined, { additionalArgs: ['.*a.test.ts$'] }); + expect(exitCode).toBe(0); + expect(output).toContain('Running 3 tests using 2 workers'); + expect(output).toContain('[p1] › a.setup.ts:5:7 › setup1'); + expect(output).toContain('[p1] › a.test.ts:6:7 › test1'); + expect(output).toContain('[p2] › b.setup.ts:5:7 › setup1'); }); test('should run all setup files if only tests match filter', async ({ runGroups }, testInfo) => { @@ -582,3 +670,43 @@ test('should run all setup files if only tests match filter', async ({ runGroups expect(output).toContain('[p1] › b.setup.ts:5:7 › setup1'); expect(output).toContain('[p1] › a.test.ts:7:7 › test2'); }); + +test('should run all setup files if only tests match grep filter', async ({ runGroups }, testInfo) => { + const files = { + 'playwright.config.ts': ` + module.exports = { + projects: [ + { + name: 'p1', + setup: /.*.setup.ts/, + }, + ] + };`, + 'a.test.ts': ` + const { test } = pwt; + test('test1', async () => { }); + test('test2', async () => { }); + test('test3', async () => { }); + `, + 'a.setup.ts': ` + const { test } = pwt; + test('setup1', async () => { }); + test('setup2', async () => { }); + `, + 'b.setup.ts': ` + const { test } = pwt; + test('setup1', async () => { }); + `, + }; + + const { exitCode, output } = await runGroups(files, undefined, undefined, { additionalArgs: ['--grep', '.*test2$'] }); + expect(exitCode).toBe(0); + expect(output).toContain('Running 4 tests using 2 workers'); + expect(output).toContain('[p1] › a.setup.ts:5:7 › setup1'); + expect(output).toContain('[p1] › a.setup.ts:6:7 › setup2'); + expect(output).toContain('[p1] › b.setup.ts:5:7 › setup1'); + expect(output).toContain('[p1] › a.test.ts:7:7 › test2'); +}); + + +// TODO: test that grep applies to both setup and tests \ No newline at end of file