Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jest-circus runs children in shuffled order #12922

Merged
merged 92 commits into from Feb 23, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
92 commits
Select commit Hold shift + click to select a range
e43ca99
jest-circus runs children in shuffled order
jhwang98 Jun 7, 2022
bbb1cf3
added shuffle and pseudorandom number generator
jhwang98 Jun 8, 2022
4c1d07c
merged master
Joshua-Hwang Aug 25, 2022
99ac4cd
added cli argv
Joshua-Hwang Sep 3, 2022
882f920
removed unnecessary dependencies
Joshua-Hwang Sep 3, 2022
4df23a9
add console logs about the seed
Joshua-Hwang Sep 3, 2022
dc6e64f
More readable Math.pow()
Joshua-Hwang Sep 3, 2022
3d09cfb
Used recommended logging approach
Joshua-Hwang Sep 3, 2022
f8860e2
Used recommended logging approach
Joshua-Hwang Sep 3, 2022
0bec19f
added prando rng
Joshua-Hwang Sep 3, 2022
3a1d712
bug where rng was recreated every run
Joshua-Hwang Sep 3, 2022
95590dc
swapped order of logs
Joshua-Hwang Sep 3, 2022
14349bd
added typing for the rng
Joshua-Hwang Sep 5, 2022
549988d
Merge branch 'main' of github.com:facebook/jest into feat-randomise-t…
Joshua-Hwang Sep 5, 2022
4ae8ff2
linting issues fixed
jhwang98 Sep 29, 2022
967b32e
unit tests are green
jhwang98 Sep 29, 2022
561146c
used snapshots instead
jhwang98 Sep 30, 2022
2f95b65
added e2e tests
jhwang98 Sep 30, 2022
50bada0
changed valid seeds
jhwang98 Sep 30, 2022
b78fdc2
added changelog message
jhwang98 Sep 30, 2022
3f33508
improved cli help and added documentation
jhwang98 Sep 30, 2022
d84caa1
Merge branch 'main' into feat-randomise-tests
jhwang98 Sep 30, 2022
b5c34fe
Update docs/CLI.md
jhwang98 Sep 30, 2022
b9c5139
Update docs/CLI.md
jhwang98 Sep 30, 2022
1b412b4
skipped jasmine
jhwang98 Sep 30, 2022
c45244c
updated cli help and docs for randomize
jhwang98 Sep 30, 2022
e1e73a0
merged
jhwang98 Sep 30, 2022
2152031
updated documentation order
jhwang98 Sep 30, 2022
6bb9449
Update docs/CLI.md
jhwang98 Sep 30, 2022
164d6fa
Update docs/CLI.md
jhwang98 Sep 30, 2022
79401d1
removed whitespace
jhwang98 Sep 30, 2022
b6c4da4
added copyright headers
jhwang98 Oct 4, 2022
19cf2b5
modifed snapshots to not rely on the actual lines
jhwang98 Oct 4, 2022
ca3a582
Merge branch 'main' into feat-randomise-tests
SimenB Oct 4, 2022
2007d47
move changelog entry
SimenB Oct 4, 2022
9361b80
Merge branch 'facebook:main' into feat-randomise-tests
Joshua-Hwang Oct 6, 2022
4d61ba2
merged master
Joshua-Hwang Oct 12, 2022
a087ce6
added documentation for configuration
Joshua-Hwang Oct 14, 2022
7b4bd2b
merged main
Joshua-Hwang Oct 14, 2022
b175652
using pure-rand
Joshua-Hwang Oct 14, 2022
54cc38c
updated cli message
Joshua-Hwang Oct 14, 2022
974bc5b
added randomize to config
Joshua-Hwang Oct 14, 2022
b4e448f
randomize gets passed to config objects
Joshua-Hwang Oct 14, 2022
816b8e6
lint fix
Joshua-Hwang Oct 14, 2022
4ba33d4
group options returns randomize
Joshua-Hwang Oct 14, 2022
7b40ca5
removed unnecessary messages
Joshua-Hwang Oct 14, 2022
ebf01d5
improved docs
Joshua-Hwang Oct 14, 2022
9172451
updated types
Joshua-Hwang Oct 14, 2022
0797346
changed the type of the rng
Joshua-Hwang Oct 14, 2022
9c0ff4e
unit tests green
Joshua-Hwang Oct 14, 2022
604499d
added unit tests for the shuffling of tests
Joshua-Hwang Oct 14, 2022
001d150
e2e tests green
Joshua-Hwang Oct 14, 2022
30b3452
small change to docs
Joshua-Hwang Oct 14, 2022
8661cad
e2e test for the config
Joshua-Hwang Oct 14, 2022
81d526b
tweaked changelog
Joshua-Hwang Oct 14, 2022
825fdd9
linting fix
Joshua-Hwang Oct 14, 2022
7512738
factorised test
Joshua-Hwang Oct 14, 2022
888f565
Merge branch 'main' into feat-randomise-tests
Joshua-Hwang Oct 17, 2022
a6f6982
Merge branch 'main' into feat-randomise-tests
Joshua-Hwang Oct 20, 2022
57e9836
linting fix
Joshua-Hwang Oct 21, 2022
b844e1b
yarn dedupe
Joshua-Hwang Oct 21, 2022
bbc70a1
show seed true for randomize
Joshua-Hwang Oct 21, 2022
d4509cb
updated docs about randomize
Joshua-Hwang Oct 21, 2022
6ce659d
e2e tests also test randomize
Joshua-Hwang Oct 21, 2022
7bdad9f
updated error message
Joshua-Hwang Oct 21, 2022
08ed42c
Merge branch 'main' of github.com:facebook/jest into feat-randomise-t…
Joshua-Hwang Oct 22, 2022
5fd2e2a
move shuffleArray to jest-circus
Joshua-Hwang Oct 22, 2022
2bffe52
prettier
SimenB Oct 22, 2022
f5b919c
improved randomize docs
Joshua-Hwang Oct 22, 2022
2688290
merged master
Joshua-Hwang Oct 28, 2022
ec85fb8
Merge branch 'main' of github.com:facebook/jest into feat-randomise-t…
Joshua-Hwang Oct 31, 2022
94c0740
Merge branch 'main' of github.com:facebook/jest into feat-randomise-t…
Joshua-Hwang Nov 3, 2022
e212ed9
Merge branch 'main' into feat-randomise-tests
SimenB Nov 7, 2022
5946e84
lint fix
jhwang98 Nov 8, 2022
ebe2485
merged main
jhwang98 Nov 8, 2022
c5145a6
Merge branch 'main' of https://github.com/facebook/jest into feat-ran…
jhwang98 Nov 14, 2022
4705449
merged main
jhwang98 Jan 12, 2023
af913e2
bumped pure-rand package
jhwang98 Jan 13, 2023
a6b4d06
Merge branch 'main' into feat-randomise-tests
jhwang98 Jan 25, 2023
e4f0b24
Merge branch 'main' into feat-randomise-tests
jhwang98 Feb 13, 2023
c4baaaa
yarn.lock updated
jhwang98 Feb 14, 2023
87547f3
fixed copyright headers
jhwang98 Feb 14, 2023
8bfa4c8
Merge branch 'main' into feat-randomise-tests
Joshua-Hwang Feb 16, 2023
0202230
Merge branch 'main' into feat-randomise-tests
SimenB Feb 23, 2023
3a0a4f1
Update packages/jest-config/src/normalize.ts
jhwang98 Feb 23, 2023
43d6f8b
Update packages/jest-core/src/cli/index.ts
jhwang98 Feb 23, 2023
7bd3fe1
Update packages/jest-circus/src/shuffleArray.ts
jhwang98 Feb 23, 2023
cff3995
remove bounds check as it's being done at the normalize level
jhwang98 Feb 23, 2023
7c8bfc1
changed error message slightly
jhwang98 Feb 23, 2023
c3c6516
swapped order for generating rng
jhwang98 Feb 23, 2023
d57bf1c
Update packages/jest-circus/src/run.ts
SimenB Feb 23, 2023
1137ce1
Update packages/jest-circus/src/run.ts
SimenB Feb 23, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -62,6 +62,9 @@ export const initialize = async ({
getRunnerState().testTimeout = globalConfig.testTimeout;
}
getRunnerState().maxConcurrency = globalConfig.maxConcurrency;
if (globalConfig.seed) {
getRunnerState().seed = globalConfig.seed;
}

// @ts-expect-error: missing `concurrent` which is added later
const globalsObject: Global.TestFrameworkGlobals = {
Expand Down
16 changes: 13 additions & 3 deletions packages/jest-circus/src/run.ts
Expand Up @@ -7,6 +7,7 @@

import pLimit = require('p-limit');
import type {Circus} from '@jest/types';
import {rngBuilder, shuffleArray} from 'jest-util';
import {dispatch, getState} from './state';
import {RETRY_TIMES} from './types';
import {
Expand All @@ -19,9 +20,13 @@ import {
} from './utils';

const run = async (): Promise<Circus.RunResult> => {
const {rootDescribeBlock} = getState();
const {rootDescribeBlock, seed} = getState();
await dispatch({name: 'run_start'});
await _runTestsForDescribeBlock(rootDescribeBlock, true);
let rng = undefined;
if (seed) {
rng = rngBuilder(seed).next;
}
await _runTestsForDescribeBlock(rootDescribeBlock, rng, true);
await dispatch({name: 'run_finish'});
return makeRunResult(
getState().rootDescribeBlock,
Expand All @@ -31,6 +36,7 @@ const run = async (): Promise<Circus.RunResult> => {

const _runTestsForDescribeBlock = async (
describeBlock: Circus.DescribeBlock,
rng: (() => number) | undefined,
isRootBlock = false,
) => {
await dispatch({describeBlock, name: 'run_describe_start'});
Expand Down Expand Up @@ -68,10 +74,14 @@ const _runTestsForDescribeBlock = async (
const retryTimes = parseInt(global[RETRY_TIMES], 10) || 0;
const deferredRetryTests = [];

// jhwang I did this :)
if (rng) {
describeBlock.children = shuffleArray(describeBlock.children, rng);
}
for (const child of describeBlock.children) {
switch (child.type) {
case 'describeBlock': {
await _runTestsForDescribeBlock(child);
await _runTestsForDescribeBlock(child, rng);
break;
}
case 'test': {
Expand Down
1 change: 1 addition & 0 deletions packages/jest-circus/src/state.ts
Expand Up @@ -33,6 +33,7 @@ const createState = (): Circus.State => {
testNamePattern: null,
testTimeout: 5000,
unhandledErrors: [],
seed: undefined,
};
};

Expand Down
13 changes: 13 additions & 0 deletions packages/jest-cli/src/cli/args.ts
Expand Up @@ -10,6 +10,10 @@ import type {Config} from '@jest/types';
import {constants, isJSONString} from 'jest-config';

export function check(argv: Config.Argv): true {
if (argv.seed && !argv.randomize) {
throw new Error('--seed requires --randomize to be specified.');
}

if (
argv.runInBand &&
Object.prototype.hasOwnProperty.call(argv, 'maxWorkers')
Expand Down Expand Up @@ -449,6 +453,10 @@ export const options: {[key: string]: Options} = {
string: true,
type: 'array',
},
randomize: {
description: 'Randomise the order of the tests in each describe block',
type: 'boolean',
},
reporters: {
description: 'A list of custom reporters for the test suite.',
string: true,
Expand Down Expand Up @@ -510,6 +518,11 @@ export const options: {[key: string]: Options} = {
"Allows to use a custom runner instead of Jest's default test runner.",
type: 'string',
},
seed: {
description:
'Must be used with the randomize flag. Specify the seed to randomize with.',
type: 'number',
},
selectProjects: {
description:
'Run the tests of the specified projects. ' +
Expand Down
1 change: 1 addition & 0 deletions packages/jest-config/src/index.ts
Expand Up @@ -153,6 +153,7 @@ const groupOptions = (
reporters: options.reporters,
rootDir: options.rootDir,
runTestsByPath: options.runTestsByPath,
seed: options.seed,
shard: options.shard,
silent: options.silent,
skipFilter: options.skipFilter,
Expand Down
5 changes: 5 additions & 0 deletions packages/jest-config/src/normalize.ts
Expand Up @@ -1152,6 +1152,11 @@ export default async function normalize(
newOptions.shard = parseShardPair(argv.shard);
}

if (argv.randomize) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should always set seet even if randomize isn't passed, right? Otherwise the types lie

// at time of writing we use mulberry32 pseudorandom number generator so 32 bits
newOptions.seed = argv.seed ?? Math.floor(Math.random() * 4294967296);
Joshua-Hwang marked this conversation as resolved.
Show resolved Hide resolved
}

return {
hasDeprecationWarnings,
options: newOptions,
Expand Down
5 changes: 5 additions & 0 deletions packages/jest-core/src/cli/index.ts
Expand Up @@ -279,6 +279,11 @@ const runWithoutWatch = async (
if (!globalConfig.listTests) {
preRunMessagePrint(outputStream);
}

jhwang98 marked this conversation as resolved.
Show resolved Hide resolved
if (globalConfig.seed) {
console.log("Seed is", globalConfig.seed);
Joshua-Hwang marked this conversation as resolved.
Show resolved Hide resolved
}

return runJest({
changedFilesPromise,
contexts,
Expand Down
Expand Up @@ -92,6 +92,7 @@ exports[`prints the config object 1`] = `
"onlyFailures": false,
"passWithNoTests": false,
"projects": [],
"randomize": false,
"reporters": [],
"rootDir": "/test_root_dir/",
"runTestsByPath": false,
Expand Down
1 change: 1 addition & 0 deletions packages/jest-core/src/runJest.ts
Expand Up @@ -200,6 +200,7 @@ export default async function runJest({
allTests = await sequencer.shard(allTests, globalConfig.shard);
}

// jhwang allTests actually means all test suites
SimenB marked this conversation as resolved.
Show resolved Hide resolved
allTests = await sequencer.sort(allTests);

if (globalConfig.listTests) {
Expand Down
3 changes: 3 additions & 0 deletions packages/jest-core/src/watch.ts
Expand Up @@ -287,6 +287,9 @@ export default async function watch(

testWatcher = new TestWatcher({isWatchMode: true});
isInteractive && outputStream.write(specialChars.CLEAR);
if (globalConfig.seed) {
console.log("Seed is", globalConfig.seed);
Joshua-Hwang marked this conversation as resolved.
Show resolved Hide resolved
}
preRunMessagePrint(outputStream);
isRunning = true;
const configs = contexts.map(context => context.config);
Expand Down
1 change: 1 addition & 0 deletions packages/jest-types/src/Circus.ts
Expand Up @@ -215,6 +215,7 @@ export type State = {
originalGlobalErrorHandlers?: GlobalErrorHandlers;
parentProcess: Process | null; // process object from the outer scope
rootDescribeBlock: DescribeBlock;
seed?: number;
testNamePattern?: RegExp | null;
testTimeout: number;
unhandledErrors: Array<Exception>;
Expand Down
3 changes: 3 additions & 0 deletions packages/jest-types/src/Config.ts
Expand Up @@ -394,6 +394,7 @@ export type GlobalConfig = {
reporters?: Array<ReporterConfig>;
runTestsByPath: boolean;
rootDir: string;
seed?: number;
shard?: ShardConfig;
silent?: boolean;
skipFilter: boolean;
Expand Down Expand Up @@ -529,6 +530,7 @@ export type Argv = Arguments<
preset: string | null | undefined;
prettierPath: string | null | undefined;
projects: Array<string>;
randomize: boolean;
reporters: Array<string>;
resetMocks: boolean;
resetModules: boolean;
Expand All @@ -537,6 +539,7 @@ export type Argv = Arguments<
rootDir: string;
roots: Array<string>;
runInBand: boolean;
seed: number;
selectProjects: Array<string>;
setupFiles: Array<string>;
setupFilesAfterEnv: Array<string>;
Expand Down
1 change: 1 addition & 0 deletions packages/jest-util/src/index.ts
Expand Up @@ -28,3 +28,4 @@ export {default as pluralize} from './pluralize';
export {default as formatTime} from './formatTime';
export {default as tryRealpath} from './tryRealpath';
export {default as requireOrImportModule} from './requireOrImportModule';
export {default as shuffleArray, rngBuilder} from './shuffleArray';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we move this into jest-circus directly rather than having it in jest-util? Not used outside of it, and I don't think we want to maintain a public API for it

38 changes: 38 additions & 0 deletions packages/jest-util/src/shuffleArray.ts
@@ -0,0 +1,38 @@
// Mulberry 32 taken from
SimenB marked this conversation as resolved.
Show resolved Hide resolved
// https://stackoverflow.com/questions/521295/seeding-the-random-number-generator-in-javascript
export function mulberry32(seed: number): {next: () => number} {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SimenB instead of coding a dedicated random generator within jest, we may leverage existing libraries such as pure-rand (the library backing fast-check) or any other one?

Copy link
Contributor

@dubzzz dubzzz Oct 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It comes with a pretty optimized and random rng called xoroshiro (the exact same ring as math random of V8 but in a seeded version)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable 👍

Copy link
Contributor

@dubzzz dubzzz Oct 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just seen the head of the PR leveraged prando.

But it has far less traction than pure-rand 🤔 (pure-rand downloads being mostly due to fast-check)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tbh I found the first rng package that seemed to fit the needs. It won't be hard to swap to pure-rand.
As you're the owner of pure-rand and a contributor to jest it makes a lot of sense to use pure-rand instead.

I'll make the change after I create a new PR just exposing the seed argument.

let state = seed;

function next(): number {
state |= 0;
state = (state + 0x6d2b79f5) | 0;
let t = Math.imul(state ^ (state >>> 15), state | 1);
t ^= t + Math.imul(t ^ (t >>> 7), t | 61);
return ((t ^ (t >>> 14)) >>> 0) / 4294967296;
}

return {next};
}

export const rngBuilder = mulberry32;

// Fisher-Yates shuffle
// This is performed in-place
export default function shuffleArray<T>(
array: Array<T>,
random: () => number = Math.random,
): Array<T> {
const length = array == null ? 0 : array.length;
if (!length) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (!length) {
if (length === 0) {

return [];
}
let index = -1;
const lastIndex = length - 1;
while (++index < length) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be a for loop?

const rand = index + Math.floor(random() * (lastIndex - index + 1));
const value = array[index];
array[index] = array[rand];
array[rand] = value;
}
return array;
}
2 changes: 2 additions & 0 deletions packages/test-utils/src/config.ts
Expand Up @@ -43,10 +43,12 @@ const DEFAULT_GLOBAL_CONFIG: Config.GlobalConfig = {
outputFile: undefined,
passWithNoTests: false,
projects: [],
randomize: false,
replname: undefined,
reporters: [],
rootDir: '/test_root_dir/',
runTestsByPath: false,
seed: undefined,
silent: false,
skipFilter: false,
snapshotFormat: {},
Expand Down