Skip to content

Commit

Permalink
fix(only changed): make only-changed work together with list mode (#3…
Browse files Browse the repository at this point in the history
…2196)

Closes #32161

Turns out we were wrong in
#31727 (comment)!

Adds support for `--only-changed` in combination with `--list` by
removing our code to prevent that.
  • Loading branch information
Skn0tt authored Aug 16, 2024
1 parent c4bb24f commit 3e6bba0
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 7 deletions.
12 changes: 6 additions & 6 deletions packages/playwright/src/runner/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export class TestRun {
export function createTaskRunner(config: FullConfigInternal, reporters: ReporterV2[]): TaskRunner<TestRun> {
const taskRunner = TaskRunner.create<TestRun>(reporters, config.config.globalTimeout);
addGlobalSetupTasks(taskRunner, config);
taskRunner.addTask('load tests', createLoadTask('in-process', { filterOnly: true, filterOnlyChanged: true, failOnLoadErrors: true }));
taskRunner.addTask('load tests', createLoadTask('in-process', { filterOnly: true, failOnLoadErrors: true }));
addRunTasks(taskRunner, config);
return taskRunner;
}
Expand All @@ -76,14 +76,14 @@ export function createTaskRunnerForWatchSetup(config: FullConfigInternal, report

export function createTaskRunnerForWatch(config: FullConfigInternal, reporters: ReporterV2[], additionalFileMatcher?: Matcher): TaskRunner<TestRun> {
const taskRunner = TaskRunner.create<TestRun>(reporters);
taskRunner.addTask('load tests', createLoadTask('out-of-process', { filterOnly: true, filterOnlyChanged: true, failOnLoadErrors: false, doNotRunDepsOutsideProjectFilter: true, additionalFileMatcher }));
taskRunner.addTask('load tests', createLoadTask('out-of-process', { filterOnly: true, failOnLoadErrors: false, doNotRunDepsOutsideProjectFilter: true, additionalFileMatcher }));
addRunTasks(taskRunner, config);
return taskRunner;
}

export function createTaskRunnerForTestServer(config: FullConfigInternal, reporters: ReporterV2[]): TaskRunner<TestRun> {
const taskRunner = TaskRunner.create<TestRun>(reporters);
taskRunner.addTask('load tests', createLoadTask('out-of-process', { filterOnly: true, filterOnlyChanged: true, failOnLoadErrors: false, doNotRunDepsOutsideProjectFilter: true }));
taskRunner.addTask('load tests', createLoadTask('out-of-process', { filterOnly: true, failOnLoadErrors: false, doNotRunDepsOutsideProjectFilter: true }));
addRunTasks(taskRunner, config);
return taskRunner;
}
Expand All @@ -108,7 +108,7 @@ function addRunTasks(taskRunner: TaskRunner<TestRun>, config: FullConfigInternal

export function createTaskRunnerForList(config: FullConfigInternal, reporters: ReporterV2[], mode: 'in-process' | 'out-of-process', options: { failOnLoadErrors: boolean }): TaskRunner<TestRun> {
const taskRunner = TaskRunner.create<TestRun>(reporters, config.config.globalTimeout);
taskRunner.addTask('load tests', createLoadTask(mode, { ...options, filterOnly: false, filterOnlyChanged: false }));
taskRunner.addTask('load tests', createLoadTask(mode, { ...options, filterOnly: false }));
taskRunner.addTask('report begin', createReportBeginTask());
return taskRunner;
}
Expand Down Expand Up @@ -222,14 +222,14 @@ function createListFilesTask(): Task<TestRun> {
};
}

function createLoadTask(mode: 'out-of-process' | 'in-process', options: { filterOnly: boolean, filterOnlyChanged: boolean, failOnLoadErrors: boolean, doNotRunDepsOutsideProjectFilter?: boolean, additionalFileMatcher?: Matcher }): Task<TestRun> {
function createLoadTask(mode: 'out-of-process' | 'in-process', options: { filterOnly: boolean, failOnLoadErrors: boolean, doNotRunDepsOutsideProjectFilter?: boolean, additionalFileMatcher?: Matcher }): Task<TestRun> {
return {
setup: async (reporter, testRun, errors, softErrors) => {
await collectProjectsAndTestFiles(testRun, !!options.doNotRunDepsOutsideProjectFilter, options.additionalFileMatcher);
await loadFileSuites(testRun, mode, options.failOnLoadErrors ? errors : softErrors);

let cliOnlyChangedMatcher: Matcher | undefined = undefined;
if (testRun.config.cliOnlyChanged && options.filterOnlyChanged) {
if (testRun.config.cliOnlyChanged) {
for (const plugin of testRun.config.plugins)
await plugin.instance?.populateDependencies?.();
const changedFiles = await detectChangedTestFiles(testRun.config.cliOnlyChanged, testRun.config.configDir);
Expand Down
25 changes: 24 additions & 1 deletion tests/playwright-test/only-changed.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -413,4 +413,27 @@ test('should run project dependencies of changed tests', {
expect(result.passed).toBe(1);

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

test('should work with list mode', async ({ runInlineTest, git, writeFiles }) => {
await writeFiles({
'a.spec.ts': `
import { test, expect } from '@playwright/test';
test('fails', () => { expect(1).toBe(2); });
`,
});

git(`add .`);
git(`commit -m init`);

const result = await runInlineTest({
'b.spec.ts': `
import { test, expect } from '@playwright/test';
test('fails', () => { expect(1).toBe(2); });
`
}, { 'only-changed': true, 'list': true });

expect(result.exitCode).toBe(0);
expect(result.output).toContain('b.spec.ts');
expect(result.output).not.toContain('a.spec.ts');
});

0 comments on commit 3e6bba0

Please sign in to comment.