From 3e6bba0b7909326c3f61b682d77b2746b776fdcf Mon Sep 17 00:00:00 2001
From: Simon Knott <info@simonknott.de>
Date: Fri, 16 Aug 2024 17:12:45 +0200
Subject: [PATCH] fix(only changed): make only-changed work together with list
 mode (#32196)

Closes https://github.com/microsoft/playwright/issues/32161

Turns out we were wrong in
https://github.com/microsoft/playwright/pull/31727#discussion_r1685793870!

Adds support for `--only-changed` in combination with `--list` by
removing our code to prevent that.
---
 packages/playwright/src/runner/tasks.ts    | 12 +++++------
 tests/playwright-test/only-changed.spec.ts | 25 +++++++++++++++++++++-
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/packages/playwright/src/runner/tasks.ts b/packages/playwright/src/runner/tasks.ts
index 4220291a08..73eb31f1e7 100644
--- a/packages/playwright/src/runner/tasks.ts
+++ b/packages/playwright/src/runner/tasks.ts
@@ -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;
 }
@@ -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;
 }
@@ -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;
 }
@@ -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);
diff --git a/tests/playwright-test/only-changed.spec.ts b/tests/playwright-test/only-changed.spec.ts
index 355bcf977d..901ec7511c 100644
--- a/tests/playwright-test/only-changed.spec.ts
+++ b/tests/playwright-test/only-changed.spec.ts
@@ -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');
-});
\ No newline at end of file
+});
+
+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');
+});