fix(test runner): make test order stable when fixtures are changing (#7923)

We used to sort based on workerHash, and that changes depending on
the exact worker fixtures list. Now we replace workerHash with
an ordinal when constructing the TestGroup list to preserve the
natural order.

src/test/runner.ts

@@ -449,10 +449,27 @@ function buildItemLocation(rootDir: string, testOrSuite: Suite | TestCase) {
 }
 
 function createTestGroups(rootSuite: Suite): TestGroup[] {
-  const groupById = new Map<string, TestGroup>();
+  // We try to preserve the order of tests when they require different workers
+  // by ordering different worker hashes sequentially.
+  const workerHashToOrdinal = new Map<string, number>();
+  const requireFileToOrdinal = new Map<string, number>();
+
+  const groupById = new Map<number, TestGroup>();
   for (const projectSuite of rootSuite.suites) {
     for (const test of projectSuite.allTests()) {
-      const id = test._workerHash + '::' + test._requireFile;
+      let workerHashOrdinal = workerHashToOrdinal.get(test._workerHash);
+      if (!workerHashOrdinal) {
+        workerHashOrdinal = workerHashToOrdinal.size + 1;
+        workerHashToOrdinal.set(test._workerHash, workerHashOrdinal);
+      }
+
+      let requireFileOrdinal = requireFileToOrdinal.get(test._requireFile);
+      if (!requireFileOrdinal) {
+        requireFileOrdinal = requireFileToOrdinal.size + 1;
+        requireFileToOrdinal.set(test._requireFile, requireFileOrdinal);
+      }
+
+      const id = workerHashOrdinal * 10000 + requireFileOrdinal;
       let group = groupById.get(id);
       if (!group) {
         group = {
@@ -467,9 +484,11 @@ function createTestGroups(rootSuite: Suite): TestGroup[] {
       group.tests.push(test);
     }
   }
-  const groups = Array.from(groupById.values());
+
-  groups.sort((a, b) => a.workerHash.localeCompare(b.workerHash));
+  // Sorting ids will preserve the natural order, because we
-  return groups;
+  // replaced hashes with ordinals according to the natural ordering.
+  const ids = Array.from(groupById.keys()).sort();
+  return ids.map(id => groupById.get(id)!);
 }
 
 class ListModeReporter implements Reporter {

tests/playwright-test/shard.spec.ts

@@ -18,7 +18,7 @@ import { test, expect } from './playwright-test-fixtures';
 
 const tests = {
   'a.spec.ts': `
-    const { test } = pwt;
+    const test = pwt.test.extend({ foo: 'bar' });
     test.use({ headless: false });
     test('test1', async () => {
       console.log('test1-done');
@@ -34,7 +34,7 @@ const tests = {
     });
   `,
   'b.spec.ts': `
-    const { test } = pwt;
+    const test = pwt.test.extend({ bar: 'foo' });
     test('test4', async () => {
       console.log('test4-done');
     });
@@ -49,9 +49,9 @@ test('should respect shard=1/2', async ({ runInlineTest }) => {
   expect(result.exitCode).toBe(0);
   expect(result.passed).toBe(3);
   expect(result.skipped).toBe(0);
-  expect(result.output).toContain('test2-done');
   expect(result.output).toContain('test1-done');
   expect(result.output).toContain('test3-done');
+  expect(result.output).toContain('test2-done');
 });
 
 test('should respect shard=2/2', async ({ runInlineTest }) => {
@@ -69,11 +69,11 @@ test('should respect shard=1/2 in config', async ({ runInlineTest }) => {
     'playwright.config.js': `
       module.exports = { shard: { current: 1, total: 2 } };
     `,
-  }, { shard: '1/2' });
+  });
   expect(result.exitCode).toBe(0);
   expect(result.passed).toBe(3);
   expect(result.skipped).toBe(0);
-  expect(result.output).toContain('test2-done');
   expect(result.output).toContain('test1-done');
   expect(result.output).toContain('test3-done');
+  expect(result.output).toContain('test2-done');
 });