diff --git a/packages/playwright-test/src/runner.ts b/packages/playwright-test/src/runner.ts index 60c7d95e84..8ceb9a0cd9 100644 --- a/packages/playwright-test/src/runner.ts +++ b/packages/playwright-test/src/runner.ts @@ -235,13 +235,13 @@ export class Runner { return projects; } - private async _collectFiles(projects: FullProjectInternal[], commandLineFileFilters: TestFileFilter[]): Promise<{filesByProject: Map; setupFiles: Set, applyFilterToSetup: boolean}> { + private async _collectFiles(projects: FullProjectInternal[], commandLineFileFilters: TestFileFilter[]): Promise<{filesByProject: Map; setupFiles: Set}> { const extensions = ['.js', '.ts', '.mjs', '.tsx', '.jsx']; const testFileExtension = (file: string) => extensions.includes(path.extname(file)); const filesByProject = new Map(); const setupFiles = new Set(); const fileToProjectName = new Map(); - const commandLineFileMatcher = fileMatcherFrom(commandLineFileFilters); + const commandLineFileMatcher = commandLineFileFilters.length ? createFileMatcherFromFilters(commandLineFileFilters) : () => true; for (const project of projects) { const allFiles = await collectFiles(project.testDir, project._respectGitIgnore); const setupMatch = createFileMatcher(project._setup); @@ -272,31 +272,44 @@ export class Runner { filesByProject.set(project, testFiles); } - // 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) { - for (const [project, files] of filesByProject) { - const filteredFiles = files.filter(commandLineFileMatcher); - if (filteredFiles.length) - filesByProject.set(project, filteredFiles); - else - filesByProject.delete(project); - } - for (const file of setupFiles) { - if (!commandLineFileMatcher(file)) - setupFiles.delete(file); - } - } - - return { filesByProject, setupFiles, applyFilterToSetup }; + return { filesByProject, setupFiles }; } private async _collectTestGroups(options: RunOptions, fatalErrors: TestError[]): Promise<{ rootSuite: Suite, projectSetupGroups: TestGroup[], testGroups: TestGroup[] }> { const config = this._loader.fullConfig(); const projects = this._collectProjects(options.projectFilter); - const { filesByProject, setupFiles, applyFilterToSetup } = await this._collectFiles(projects, options.testFileFilters); + const { filesByProject, setupFiles } = await this._collectFiles(projects, options.testFileFilters); + let result = await this._createFilteredRootSuite(options, filesByProject, new Set(), !!setupFiles.size); + if (setupFiles.size) { + const allTests = result.rootSuite.allTests(); + const tests = allTests.filter(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. + if (tests.length > 0 && tests.length === allTests.length) + result = await this._createFilteredRootSuite(options, filesByProject, setupFiles, false); + } + + fatalErrors.push(...result.fatalErrors); + const { rootSuite } = result; + + const allTestGroups = createTestGroups(rootSuite.suites, config.workers); + const projectSetupGroups = []; + const testGroups = []; + for (const group of allTestGroups) { + if (setupFiles.has(group.requireFile)) + projectSetupGroups.push(group); + else + testGroups.push(group); + } + return { rootSuite, projectSetupGroups, testGroups }; + } + + private async _createFilteredRootSuite(options: RunOptions, filesByProject: Map, doNotFilterFiles: Set, shouldCloneTests: boolean): Promise<{rootSuite: Suite, fatalErrors: TestError[]}> { + const config = this._loader.fullConfig(); + const fatalErrors: TestError[] = []; const allTestFiles = new Set(); for (const files of filesByProject.values()) files.forEach(file => allTestFiles.add(file)); @@ -307,7 +320,8 @@ export class Runner { const fileSuite = await this._loader.loadTestFile(file, 'runner'); if (fileSuite._loadError) fatalErrors.push(fileSuite._loadError); - preprocessRoot._addSuite(fileSuite); + // We have to clone only if there maybe subsequent calls of this method. + preprocessRoot._addSuite(shouldCloneTests ? fileSuite._deepClone() : fileSuite); } // Complain about duplicate titles. @@ -316,8 +330,7 @@ export class Runner { fatalErrors.push(duplicateTitlesError); // Filter tests to respect line/column filter. - if (options.testFileFilters.length) - filterByFocusedLine(preprocessRoot, options.testFileFilters, applyFilterToSetup ? new Set() : setupFiles); + filterByFocusedLine(preprocessRoot, options.testFileFilters, doNotFilterFiles); // Complain about only. if (config.forbidOnly) { @@ -327,13 +340,8 @@ export class Runner { } // Filter only. - if (!options.listOnly) { - const onlyItems = preprocessRoot._getOnlyItems(); - if (onlyItems.length) { - const hasOnlyInSetup = onlyItems.some(item => setupFiles.has(item._requireFile)); - filterOnly(preprocessRoot, hasOnlyInSetup ? new Set() : setupFiles); - } - } + if (!options.listOnly) + filterOnly(preprocessRoot, doNotFilterFiles); // Generate projects. const fileSuites = new Map(); @@ -346,6 +354,8 @@ export class Runner { const grepInvertMatcher = project.grepInvert ? createTitleMatcher(project.grepInvert) : null; const titleMatcher = (test: TestCase) => { + if (doNotFilterFiles.has(test._requireFile)) + return true; const grepTitle = test.titlePath().join(' '); if (grepInvertMatcher?.(grepTitle)) return false; @@ -362,50 +372,13 @@ export class Runner { if (!fileSuite) continue; for (let repeatEachIndex = 0; repeatEachIndex < project.repeatEach; repeatEachIndex++) { - const builtSuite = this._loader.buildFileSuiteForProject(project, fileSuite, repeatEachIndex, test => { - if (setupFiles.has(test._requireFile)) - return true; - return titleMatcher(test); - }); + const builtSuite = this._loader.buildFileSuiteForProject(project, fileSuite, repeatEachIndex, titleMatcher); 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); - - const projectSetupGroups = []; - const testGroups = []; - for (const group of allTestGroups) { - if (setupFiles.has(group.requireFile)) - projectSetupGroups.push(group); - else - testGroups.push(group); - } - - return { rootSuite, projectSetupGroups, testGroups }; + return { rootSuite, fatalErrors }; } private _filterForCurrentShard(rootSuite: Suite, projectSetupGroups: TestGroup[], testGroups: TestGroup[]) { @@ -677,26 +650,25 @@ export class Runner { } function filterOnly(suite: Suite, doNotFilterFiles: Set) { - const suiteFilter = (suite: Suite) => suite._only; - const testFilter = (test: TestCase) => doNotFilterFiles.has(test._requireFile) || test._only; + if (!suite._getOnlyItems().length) + return; + const suiteFilter = (suite: Suite) => suite._only || doNotFilterFiles.has(suite._requireFile); + const testFilter = (test: TestCase) => test._only || doNotFilterFiles.has(test._requireFile); return filterSuiteWithOnlySemantics(suite, suiteFilter, testFilter); } -function filterByFocusedLine(suite: Suite, focusedTestFileLines: TestFileFilter[], doNotFilterFiles: Set) { - const filterWithLine = !!focusedTestFileLines.find(f => f.line !== null); - if (!filterWithLine) - return; +function createFileMatcherFromFilter(filter: TestFileFilter) { + const fileMatcher = createFileMatcher(filter.re || filter.exact || ''); + return (testFileName: string, testLine: number, testColumn: number) => + fileMatcher(testFileName) && (filter.line === testLine || filter.line === null) && (filter.column === testColumn || filter.column === null); +} - const testFileLineMatches = (testFileName: string, testLine: number, testColumn: number) => focusedTestFileLines.some(filter => { - const lineColumnOk = (filter.line === testLine || filter.line === null) && (filter.column === testColumn || filter.column === null); - if (!lineColumnOk) - return false; - return createFileMatcherFromFilters([filter])(testFileName); - }); - const suiteFilter = (suite: Suite) => { - return !!suite.location && testFileLineMatches(suite.location.file, suite.location.line, suite.location.column); - }; - // Project setup files are always included. +function filterByFocusedLine(suite: Suite, focusedTestFileLines: TestFileFilter[], doNotFilterFiles: Set) { + if (!focusedTestFileLines.length) + return; + const matchers = focusedTestFileLines.map(createFileMatcherFromFilter); + const testFileLineMatches = (testFileName: string, testLine: number, testColumn: number) => matchers.some(m => m(testFileName, testLine, testColumn)); + const suiteFilter = (suite: Suite) => doNotFilterFiles.has(suite._requireFile) || !!suite.location && testFileLineMatches(suite.location.file, suite.location.line, suite.location.column); const testFilter = (test: TestCase) => doNotFilterFiles.has(test._requireFile) || testFileLineMatches(test.location.file, test.location.line, test.location.column); return filterSuite(suite, suiteFilter, testFilter); } @@ -946,12 +918,6 @@ class ListModeReporter implements Reporter { } } -function fileMatcherFrom(testFileFilters?: TestFileFilter[]): Matcher { - if (testFileFilters?.length) - return createFileMatcherFromFilters(testFileFilters); - return () => true; -} - function createForbidOnlyError(config: FullConfigInternal, onlyTestsAndSuites: (TestCase | Suite)[]): TestError { const errorMessage = [ '=====================================', diff --git a/packages/playwright-test/src/test.ts b/packages/playwright-test/src/test.ts index 0d70b92a46..c1f16db3b6 100644 --- a/packages/playwright-test/src/test.ts +++ b/packages/playwright-test/src/test.ts @@ -104,6 +104,17 @@ export class Suite extends Base implements reporterTypes.Suite { return items; } + _deepClone(): Suite { + const suite = this._clone(); + for (const entry of this._entries) { + if (entry instanceof Suite) + suite._addSuite(entry._deepClone()); + else + suite._addTest(entry._clone()); + } + return suite; + } + _clone(): Suite { const suite = new Suite(this.title, this._type); suite._only = this._only; diff --git a/tests/playwright-test/project-setup.spec.ts b/tests/playwright-test/project-setup.spec.ts index aea1a82218..4787aac6ab 100644 --- a/tests/playwright-test/project-setup.spec.ts +++ b/tests/playwright-test/project-setup.spec.ts @@ -498,6 +498,42 @@ test('should allow describe.only in setup files', async ({ runGroups }, testInfo expect(passed).toBe(2); }); +test('should filter describe line in setup 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('test2', async () => { }); + test('test3', async () => { }); + test('test4', async () => { }); + `, + 'a.setup.ts': ` + const { test } = pwt; + test.describe('main', () => { + test('setup1', async () => { }); + test('setup2', async () => { }); + }); + test('setup3', async () => { }); + `, + }; + + const { exitCode, passed, timeline, output } = await runGroups(files, undefined, undefined, { additionalArgs: ['a.setup.ts:5'] }); + expect(output).toContain('Running 2 tests using 1 worker'); + expect(output).toContain('[p1] › a.setup.ts:6:9 › main › setup1'); + expect(output).toContain('[p1] › a.setup.ts:7:9 › main › setup2'); + expect(fileNames(timeline)).toEqual(['a.setup.ts']); + expect(exitCode).toBe(0); + expect(passed).toBe(2); +}); test('should allow .only in both setup and test files', async ({ runGroups }, testInfo) => { const files = { @@ -785,5 +821,40 @@ test('should run all setup files if only tests match grep filter', async ({ runG expect(output).toContain('[p1] › a.test.ts:7:7 › test2'); }); +test('should apply project.grep filter to both setup and tests', async ({ runGroups }, testInfo) => { + const files = { + 'playwright.config.ts': ` + module.exports = { + projects: [ + { + name: 'p1', + setup: /.*.setup.ts/, + grep: /a.(test|setup).ts.*(test|setup)/, + }, + ] + };`, + 'a.test.ts': ` + const { test } = pwt; + test('test1', async () => { }); + test('test2', async () => { }); + test('foo', async () => { }); + `, + 'a.setup.ts': ` + const { test } = pwt; + test('setup1', async () => { }); + test('setup2', async () => { }); + `, + 'b.setup.ts': ` + const { test } = pwt; + test('setup1', async () => { }); + test('foo', async () => { }); + `, + }; -// TODO: test that grep applies to both setup and tests \ No newline at end of file + const { exitCode, output } = await runGroups(files); + expect(exitCode).toBe(0); + 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'); +}); diff --git a/tests/playwright-test/runner.spec.ts b/tests/playwright-test/runner.spec.ts index 5f4171ba94..db99ee5439 100644 --- a/tests/playwright-test/runner.spec.ts +++ b/tests/playwright-test/runner.spec.ts @@ -479,3 +479,22 @@ test('should not crash with duplicate titles and line filter', async ({ runInlin ` - example.spec.ts:8`, ].join('\n')); }); + +test('should not load tests not matching filter', async ({ runInlineTest }) => { + const result = await runInlineTest({ + 'a.spec.ts': ` + const { test } = pwt; + console.log('in a.spec.ts'); + test('test1', () => {}); + `, + 'example.spec.ts': ` + const { test } = pwt; + console.log('in example.spec.ts'); + test('test2', () => {}); + ` + + }, {}, {}, { additionalArgs: ['a.spec.ts'] }); + expect(result.exitCode).toBe(0); + expect(stripAnsi(result.output)).not.toContain('in example.spec.ts'); + expect(stripAnsi(result.output)).toContain('in a.spec.ts'); +});