playwright icon indicating copy to clipboard operation
playwright copied to clipboard

[Feature]Is it possibile to provide a new shard strategy by test.describe() granularity not test()?

Open Jabbar2010 opened this issue 1 year ago • 3 comments

Playwright version: 1.30 Scenario: One test. describe has involved several test cases, one test.describe will start a meeting in beforeAll hook which will take us 2 minutes, and then several test cases will be executed one by one, so totally the whole test.describe will take us 2.5 minutes

test.describe('test a meeting', () => {
  test.beforeAll(() => {
    startMeeting(); // 2mins
  });
  test('test host', async () => {
    // do something, 10s
  });
  test('test participants1', async () => {
    // do something, 10s
  });
 test('test participants2', async () => {
    // do something, 10s
  });
 test('test participants3', async () => {
    // do something, 10s
  });
});
test.describe('test a meeting2', () => {
  test.beforeAll(() => {
    startMeeting(); // 2mins
  });
  test('test host2', async () => {
    // do something, 10s
  });
  test('test participants1', async () => {
    // do something, 10s
  });
test('test participants1', async () => {
    // do something, 10s
  });
});

test.describe('test a meeting3', () => {
  test.beforeAll(() => {
    startMeeting(); // 2mins
  });
  test('test host2', async () => {
    // do something, 10s
  });
  test('test participants1', async () => {
    // do something, 10s
  });
test('test participants1', async () => {
    // do something, 10s
  });
});

I found when we use npx playwright test --shard=1/3, playwright will group all the test cases and execute them by the current default group strategy.

But now I want to group by test.describe, so the three test.describe will be assigned into three shards, and every shard will take 2 mins

My temporary solution: I have to change playwright source code and add an external command line parameter:--shard-by-test-describe,so I can achieve it by use npx playwright test --project=chrome --shard-by-test-describe --shard=1/3

diff --git a/node_modules/@playwright/test/lib/cli.js b/node_modules/@playwright/test/lib/cli.js
index aab6f57..cfef262 100755
--- a/node_modules/@playwright/test/lib/cli.js
+++ b/node_modules/@playwright/test/lib/cli.js
@@ -66,6 +66,7 @@ function addTestCommand(program) {
   command.option('--trace <mode>', `Force tracing mode, can be ${kTraceModes.map(mode => `"${mode}"`).join(', ')}`);
   command.option('-u, --update-snapshots', `Update snapshots with actual results (default: only create missing snapshots)`);
   command.option('-x', `Stop after the first failure`);
+  command.option('--shard-by-test-describe', `Customized shard strategy`);
   command.action(async (args, opts) => {
     try {
       await runTests(args, opts);
@@ -167,7 +168,8 @@ async function runTests(args, opts) {
     testFileFilters,
     testTitleMatcher,
     projectFilter: opts.project || undefined,
-    passWithNoTests: opts.passWithNoTests
+    passWithNoTests: opts.passWithNoTests,
+    cmdOptions: opts,
   });
   await (0, _profiler.stopProfiling)(undefined);
   if (result.status === 'interrupted') process.exit(130);
diff --git a/node_modules/@playwright/test/lib/runner.js b/node_modules/@playwright/test/lib/runner.js
index 885fb35..1bc36e8 100644
--- a/node_modules/@playwright/test/lib/runner.js
+++ b/node_modules/@playwright/test/lib/runner.js
@@ -293,9 +293,88 @@ class Runner {
     }
     return (0, _testLoader.loadTestFilesInProcess)(this._configLoader.fullConfig(), [...testFiles], this._fatalErrors);
   }
-  _filterForCurrentShard(rootSuite, testGroups) {
+
+  _filterForCurrentShardByTestDescribe(rootSuite, testGroups) {
+    const shard = this._configLoader.fullConfig().shard;
+    let shardableTotal = 0;
+    let skippedCount = 0;
+    let descibeTotal = 0;
+    for (const suite of rootSuite._entries){
+      for (const testFile of suite._entries) {
+        for (const testDescibe of testFile._entries) {
+          descibeTotal ++;
+          if (!testDescibe._skipped){
+            shardableTotal++;
+          }else{
+            skippedCount ++;
+          }
+        }
+      }
+    }
+
+    // Each shard gets some tests.
+    const shardSize = Math.floor(shardableTotal / shard.total);
+    // First few shards get one more test each.
+    const extraOne = shardableTotal - shardSize * shard.total;
+    const currentShard = shard.current - 1; // Make it zero-based for calculations.
+    const from = shardSize * currentShard + Math.min(extraOne, currentShard);
+    const to = from + shardSize + (currentShard < extraOne ? 1 : 0);
+    let current = 0;
+    const shardTests = new Set();
+    const shardTestGroups = [];
+    const shardTestDescribes = [];
+    const createGroup = test => {
+      return {
+        workerHash: test._workerHash,
+        requireFile: test._requireFile,
+        repeatEachIndex: test.repeatEachIndex,
+        projectId: test._projectId,
+        tests: []
+      };
+    };
+    for (const projectSuite of rootSuite._entries) {
+      for (const testFile of projectSuite._entries) {
+        for (const testDescibe of testFile._entries) {
+          if (!testDescibe._skipped){
+            if (current >= from && current < to) {
+              shardTestDescribes.push(testDescibe);
+              if (testDescibe._entries.length > 0){
+                const testGroup = createGroup(testDescibe._entries[0]);
+                for (const testCase of testDescibe._entries) {
+                  shardTests.add(testCase);
+                  testGroup.tests.push(testCase);
+                }
+              shardTestGroups.push(
+                  testGroup
+                );
+              }
+            }
+            current++;
+          }
+        }
+      }
+    }
+    testGroups.length = 0;
+    console.log(`current shard index ${currentShard + 1}, describes count:${shardTestDescribes.length},testCase count:${shardTests.size}`);
+    console.log(`descibe total:${descibeTotal},skipped total:${skippedCount}, excute total:${shardableTotal}`);
+    console.log(`test case total:${rootSuite.allTests().length}`);
+    testGroups.push(...shardTestGroups);
+    if (!shardTests.size) {
+      // Filtering with "only semantics" does not work when we have zero tests - it leaves all the tests.
+      // We need an empty suite in this case.
+      rootSuite._entries = [];
+    } else {
+      (0, _suiteUtils.filterSuiteWithOnlySemantics)(rootSuite, () => false, test => shardTests.has(test));
+    }
+  }
+
+  _filterForCurrentShard(rootSuite, testGroups, cmdOptions) {
     const shard = this._configLoader.fullConfig().shard;
     if (!shard) return;
+    if (cmdOptions.shardByTestDescribe) {
+      this._filterForCurrentShardByTestDescribe(rootSuite, testGroups);
+      return;
+    }
 
     // Each shard includes:
     // - its portion of the regular tests
@@ -303,7 +382,6 @@ class Runner {
     let shardableTotal = 0;
     for (const group of testGroups) shardableTotal += group.tests.length;
     const shardTests = new Set();
-
     // Each shard gets some tests.
     const shardSize = Math.floor(shardableTotal / shard.total);
     // First few shards get one more test each.
@@ -346,7 +424,7 @@ class Runner {
 
     // Fail when no tests.
     if (!rootSuite.allTests().length && !options.passWithNoTests) this._fatalErrors.push(createNoTestsError());
-    this._filterForCurrentShard(rootSuite, testGroups);
+    this._filterForCurrentShard(rootSuite, testGroups, options.cmdOptions);
     config._maxConcurrentTestGroups = testGroups.length;
 
     // Report begin

Jabbar2010 avatar May 09 '23 03:05 Jabbar2010

Do I read it right that you have

  • 2 files
  • first file has 1 describe with 5 tests
  • second file has 2 describes, 4 tests each

Your beforeAlls are heavy and you want shards 1-3/3 to each run one describe?

pavelfeldman avatar May 09 '23 16:05 pavelfeldman

@pavelfeldman Yeah, due to the heavy beforeAlls, I just want to run my test cases group by describe, is it possible to add a command parameter for this case, not just group by test cases?

Jabbar2010 avatar May 10 '23 06:05 Jabbar2010

Thanks a lot, that's a great news for me

Jabbar2010 avatar May 12 '23 08:05 Jabbar2010

@Jabbar2010 In v1.35, you should be able to do the following:

// example.spec.ts

test.describe.configure({ mode: 'parallel' });  // This will allow describe to run in parallel and be sharded.

test.describe('suite1', () => {
  test.describe.configure({ mode: 'default' });  // This makes tests inside the describe run in sequence, on a single shard.
  test.beforeAll(() => {});
  test('test1', async () => {});
  test('test2', async () => {});
  test('test3', async () => {});
  test('test4', async () => {});
});

test.describe('suite2', () => {
  test.describe.configure({ mode: 'default' });  // This makes tests inside the describe run in sequence, on a single shard.
  test.beforeAll(() => {});
  test('test1', async () => {});
  test('test2', async () => {});
  test('test3', async () => {});
  test('test4', async () => {});
});

With such a setup, top-level describes will run in parallel and could be assigned to different shards, while tests inside each describe will continue to run as usual, on a single shard. You can mix and match 'parallel' and 'default' modes to achieve a combination of parallelism/sharding that you'd like.

dgozman avatar May 18 '23 20:05 dgozman

@dgozman Hi, do I have to use test.describe.configure({ mode: 'parallel' }) on the global and test.describe.configure({ mode: 'default' }) inside simultaneously?

Jabbar2010 avatar May 22 '23 05:05 Jabbar2010

@dgozman Hi, do I have to use test.describe.configure({ mode: 'parallel' }) on the global and test.describe.configure({ mode: 'default' }) inside simultaneously?

Yes, this way you designate your describes to be run in parallel, but tests inside describes to be run in order. Note this is not available to day in the stable release, but you can play with it in the canary release of Playwright.

dgozman avatar May 22 '23 15:05 dgozman

Hi @dgozman , I am trying to apply your solution, but in combination with test parametrisation. In my case, I have N describes that are wrapped in a cycle and I want to shard those (basically sharding by test parameter). Proto code:

const params = ['param1', 'param2']
test.describe.configure({ mode: 'parallel' })

for (const param of params) {
  test.describe(`Show pages - ${param}`, () => { // Want to run each describe on different shard
    test.describe.configure({ mode: 'default' }) // While running nested tests on each parametrised shard sequentially

    test.beforeAll(async () => {})
    test.beforeEach(async () => {})
    test('test1', async () => {})
    test('test2', async () => {})
  })
}  

What seems to happen is while sharding does happen, it does not happen by parameter. Any advice?

realviktornedelko avatar Jan 10 '24 12:01 realviktornedelko