Skip to content

Commit

Permalink
Revert "feat(test runner): shuffle order of tests with sharding seed (m…
Browse files Browse the repository at this point in the history
…icrosoft#30817)"

This reverts commit 825e0e4.

API review notes: sounds like this change did not solve the problem
for the contributor, there is a new approach under development in
microsoft#30962
  • Loading branch information
pavelfeldman committed Jun 11, 2024
1 parent e07b468 commit 7f465f0
Show file tree
Hide file tree
Showing 8 changed files with 0 additions and 118 deletions.
21 changes: 0 additions & 21 deletions docs/src/test-api/class-testconfig.md
Original file line number Diff line number Diff line change
Expand Up @@ -482,27 +482,6 @@ export default defineConfig({
```


## property: TestConfig.shardingSeed

* since: v1.45
- type: ?<[string]>

Shuffle the order of test groups with a seed. By default tests are run in the order they are discovered, which is mostly alphabetical. This could lead to an uneven distribution of slow and fast tests. Shuffling the order of tests in a deterministic way can help to distribute the load more evenly.

The sharding seed is a string that is used to initialize a random number generator.

Learn more about [parallelism and sharding](../test-parallel.md) with Playwright Test.

**Usage**

```js title="playwright.config.ts"
import { defineConfig } from '@playwright/test';

export default defineConfig({
shardingSeed: 'string value'
});
```

## property: TestConfig.testDir
* since: v1.10
- type: ?<[string]>
Expand Down
6 changes: 0 additions & 6 deletions docs/src/test-sharding-js.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,6 @@ Now, if you run these shards in parallel on different computers, your test suite

Note that Playwright can only shard tests that can be run in parallel. By default, this means Playwright will shard test files. Learn about other options in the [parallelism guide](./test-parallel.md).

## Randomizing test order in a deterministic way

By default tests are run in the order they are discovered, which is mostly alphabetical. This could lead to an uneven distribution of slow and fast tests. For example, if the first half of your tests are slower than the rest of your tests and you are using 4 shards it means that shard 1 and 2 will take significantly more time then shard 3 and 4.

To aid with this problem you can pass `--sharding-seed=string-value` to randomize the order of tests in a deterministic way, which could yield better distribution of slow and fast tests across all shards.

## Merging reports from multiple shards

In the previous example, each test shard has its own test report. If you want to have a combined report showing all the test results from all the shards, you can merge them.
Expand Down
2 changes: 0 additions & 2 deletions packages/playwright/src/common/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ export class FullConfigInternal {
cliFailOnFlakyTests?: boolean;
testIdMatcher?: Matcher;
defineConfigWasUsed = false;
shardingSeed: string | null;

constructor(location: ConfigLocation, userConfig: Config, configCLIOverrides: ConfigCLIOverrides) {
if (configCLIOverrides.projects && userConfig.projects)
Expand Down Expand Up @@ -93,7 +92,6 @@ export class FullConfigInternal {
workers: 0,
webServer: null,
};
this.shardingSeed = takeFirst(configCLIOverrides.shardingSeed, userConfig.shardingSeed, null);
for (const key in userConfig) {
if (key.startsWith('@'))
(this.config as any)[key] = (userConfig as any)[key];
Expand Down
1 change: 0 additions & 1 deletion packages/playwright/src/common/ipc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ export type ConfigCLIOverrides = {
reporter?: ReporterDescription[];
additionalReporters?: ReporterDescription[];
shard?: { current: number, total: number };
shardingSeed?: string;
timeout?: number;
ignoreSnapshots?: boolean;
updateSnapshots?: 'all'|'none'|'missing';
Expand Down
2 changes: 0 additions & 2 deletions packages/playwright/src/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,6 @@ function overridesFromOptions(options: { [key: string]: any }): ConfigCLIOverrid
retries: options.retries ? parseInt(options.retries, 10) : undefined,
reporter: resolveReporterOption(options.reporter),
shard: shardPair ? { current: shardPair[0], total: shardPair[1] } : undefined,
shardingSeed: options.shardingSeed ? options.shardingSeed : undefined,
timeout: options.timeout ? parseInt(options.timeout, 10) : undefined,
ignoreSnapshots: options.ignoreSnapshots ? !!options.ignoreSnapshots : undefined,
updateSnapshots: options.updateSnapshots ? 'all' as const : undefined,
Expand Down Expand Up @@ -359,7 +358,6 @@ const testOptions: [string, string][] = [
['--reporter <reporter>', `Reporter to use, comma-separated, can be ${builtInReporters.map(name => `"${name}"`).join(', ')} (default: "${defaultReporter}")`],
['--retries <retries>', `Maximum retry count for flaky tests, zero for no retries (default: no retries)`],
['--shard <shard>', `Shard tests and execute only the selected shard, specify in the form "current/all", 1-based, for example "3/5"`],
['--sharding-seed <seed>', `Seed string for randomizing the test order before sharding. Defaults to not randomizing the order.`],
['--timeout <timeout>', `Specify test timeout threshold in milliseconds, zero for unlimited (default: ${defaultTimeout})`],
['--trace <mode>', `Force tracing mode, can be ${kTraceModes.map(mode => `"${mode}"`).join(', ')}`],
['--ui', `Run tests in interactive UI mode`],
Expand Down
4 changes: 0 additions & 4 deletions packages/playwright/src/runner/loadUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import { createTestGroups, filterForShard, type TestGroup } from './testGroups';
import { dependenciesForTestFile } from '../transform/compilationCache';
import { sourceMapSupport } from '../utilsBundle';
import type { RawSourceMap } from 'source-map';
import { shuffleWithSeed } from './shuffle';

export async function collectProjectsAndTestFiles(testRun: TestRun, doNotRunTestsOutsideProjectFilter: boolean, additionalFileMatcher?: Matcher) {
const config = testRun.config;
Expand Down Expand Up @@ -180,9 +179,6 @@ export async function createRootSuite(testRun: TestRun, errors: TestError[], sho
for (const projectSuite of rootSuite.suites)
testGroups.push(...createTestGroups(projectSuite, config.config.workers));

if (config.shardingSeed)
shuffleWithSeed(testGroups, config.shardingSeed);

// Shard test groups.
const testGroupsInThisShard = filterForShard(config.config.shard, testGroups);
const testsInThisShard = new Set<TestCase>();
Expand Down
59 changes: 0 additions & 59 deletions packages/playwright/src/runner/shuffle.ts

This file was deleted.

23 changes: 0 additions & 23 deletions packages/playwright/types/test.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1426,29 +1426,6 @@ interface TestConfig<TestArgs = {}, WorkerArgs = {}> {
total: number;
};

/**
* Shuffle the order of test groups with a seed. By default tests are run in the order they are discovered, which is
* mostly alphabetical. This could lead to an uneven distribution of slow and fast tests. Shuffling the order of tests
* in a deterministic way can help to distribute the load more evenly.
*
* The sharding seed is a string that is used to initialize a random number generator.
*
* Learn more about [parallelism and sharding](https://playwright.dev/docs/test-parallel) with Playwright Test.
*
* **Usage**
*
* ```js
* // playwright.config.ts
* import { defineConfig } from '@playwright/test';
*
* export default defineConfig({
* shardingSeed: 'string value'
* });
* ```
*
*/
shardingSeed?: string;

/**
* **NOTE** Use
* [testConfig.snapshotPathTemplate](https://playwright.dev/docs/api/class-testconfig#test-config-snapshot-path-template)
Expand Down

0 comments on commit 7f465f0

Please sign in to comment.