Skip to content

Commit

Permalink
cherry-pick(#32094): fix(test runner): run project dependencies of `-…
Browse files Browse the repository at this point in the history
…-only-changed` test files (#32172)

Closes #32070. We were
applying `additionalFileMatcher` not just to `filteredProjectSuites`,
but also to `projectSuites`. `projectSuites` is where we take dependency
projects from, though - so `--only-changed` led to empty dependency
projects, resulting in the reported bug.
    
The fix is to only apply `additionalFileMatcher` on
`filteredProjectSuites`.
  • Loading branch information
Skn0tt committed Aug 15, 2024
1 parent d78ae01 commit 7cf7aec
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 7 deletions.
16 changes: 9 additions & 7 deletions packages/playwright/src/runner/loadUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
const filteredFileSuites = additionalFileMatcher ? fileSuites.filter(fileSuite => additionalFileMatcher(fileSuite.location!.file)) : fileSuites;
const projectSuite = createProjectSuite(project, filteredFileSuites);
const projectSuite = createProjectSuite(project, fileSuites);
projectSuites.set(project, projectSuite);
const filteredProjectSuite = filterProjectSuite(projectSuite, { cliFileFilters, cliTitleMatcher, testIdMatcher: config.testIdMatcher });

const filteredProjectSuite = filterProjectSuite(projectSuite, { cliFileFilters, cliTitleMatcher, testIdMatcher: config.testIdMatcher, additionalFileMatcher });
filteredProjectSuites.set(project, filteredProjectSuite);
}
}
Expand Down Expand Up @@ -200,8 +200,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, projectSuites.get(project)!));
}
}
Expand All @@ -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();
Expand All @@ -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;
Expand Down
49 changes: 49 additions & 0 deletions tests/playwright-test/only-changed.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -364,4 +364,53 @@ 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', {
annotation: {
type: 'issue',
description: 'https://github.com/microsoft/playwright/issues/32070',
},
}, async ({ runInlineTest, git, writeFiles }) => {
await writeFiles({
'playwright.config.ts': `
module.exports = {
projects: [
{ name: 'setup', testMatch: 'setup.spec.ts', },
{ name: 'main', dependencies: ['setup'] },
],
};
`,
'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';
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(1);

expect(result.output).toContain('setup test is executed');
});

0 comments on commit 7cf7aec

Please sign in to comment.