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

feat: make sequencer public, add option to run tests in random order #1582

Merged
merged 11 commits into from Jul 6, 2022
17 changes: 17 additions & 0 deletions docs/api/index.md
Expand Up @@ -312,6 +312,23 @@ When you use `test` in the top level of file, they are collected as part of the
describe.todo.concurrent(/* ... */) // or describe.concurrent.todo(/* ... */)
```

### describe.random

- **Type:** `(name: string, fn: TestFunction, timeout?: number) => void`

Vitest provides a way to run all tests in random order via CLI flag [`--random`](/guide/cli) or config option [`sequence.random`](/config/#sequence-random), but if you want to have only part of your test suite to run tests in random order, you can mark it with this flag.

```ts
describe.random('suite', () => {
Copy link
Member

@antfu antfu Jul 4, 2022

Choose a reason for hiding this comment

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

If so, should we also support repeat for describe? For example:

describe.repeat(10).random('repect 10 times randomly', () => {
  // ...
})

Copy link
Member Author

Choose a reason for hiding this comment

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

How is this has anything to do with random? 👀

Copy link
Member

@antfu antfu Jul 5, 2022

Choose a reason for hiding this comment

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

No necessary in this PR, just an idea. Using random feels more unstable to identify the ordering bug (like sometimes the change passes the CI because the order does not hit the bug, but will later). Running random tests multiple times could reduce such scenarios and make the tests more stable.

Copy link
Member Author

Choose a reason for hiding this comment

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

No necessary in this PR, just an idea. Using random feels more unstable to identify the ordering bug (like sometimes the change passes the CI because the order does not hit the bug, but will later). Running random tests multiple times could reduce such scenarios and make the tests more stable.

Sure, but this also should be done on the global level, yes? This PR adds vitest --random, so there should also be vitest --random --repeat 10.

It's also not clear what should be done on error. Do we fail the suite when it hits the first error? Do we show summary of all 10 runs? I think if we want to add this, it's better be done in other PR, and if/when people will ask for it.

I really want to switch to @vitest/browser, but all this enhancements PRs stop me 😄

Copy link
Member

Choose a reason for hiding this comment

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

Sure we could do that later 👍

test('random test 1', async () => { /* ... */ })
test('random test 2', async () => { /* ... */ })
test('random test 3', async () => { /* ... */ })
})
// order depends on sequence.seed option in config (Date.now() by default)
```

`.skip`, `.only`, and `.todo` works with random suites.

### describe.todo

- **Type:** `(name: string) => void`
Expand Down
31 changes: 31 additions & 0 deletions docs/config/index.md
Expand Up @@ -582,3 +582,34 @@ Options to configure Vitest cache policy. At the moment Vitest stores cache for
- **Default**: `node_modules/.vitest`

Path to cache directory.

### sequence

- **Type**: `{ sequencer?, random?, seed? }`

Options for how tests should be sorted.

#### sequence.sequencer

- **Type**: `TestSequencerConstructor`
- **Default**: `BaseSequencer`

A custom class that defines methods for sharding and sorting. You can extend `BaseSequencer` from `vitest/node`, if you only need to redefine one of the `sort` and `shard` methods, but both should exist.

Sharding is happening before sorting, and only if `--shard` option is provided.

#### sequence.random

- **Type**: `boolean`
- **Default**: `false`

If you want tests to run randomly, you can enable it with this option, or CLI argument [`--random`](/guide/cli).

Vitest usually uses cache to sort tests, so long running tests start earlier - this makes tests run faster. If your tests will run in random order you will lose this performance improvement, but it may be useful to track tests that accidentally depend on another run previously.

#### sequence.seed

- **Type**: `number`
- **Default**: `Date.now()`

Sets the randomization seed, if tests are running in random order.
5 changes: 1 addition & 4 deletions docs/guide/cli.md
Expand Up @@ -36,10 +36,6 @@ Useful to run with [`lint-staged`](https://github.com/okonet/lint-staged) or wit
vitest related /src/index.ts /src/hello-world.js
```

### `vitest clean cache`

Clears cache folder.

## Options

| Options | |
Expand Down Expand Up @@ -72,6 +68,7 @@ Clears cache folder.
| `--allowOnly` | Allow tests and suites that are marked as `only` (default: false in CI, true otherwise) |
| `--changed [since]` | Run tests that are affected by the changed files (default: false). See [docs](#changed) |
| `--shard <shard>` | Execute tests in a specified shard |
| `--random` | Execute tests in random order |
| `-h, --help` | Display available CLI options |

### changed
Expand Down
1 change: 1 addition & 0 deletions packages/vitest/src/node/cli.ts
Expand Up @@ -35,6 +35,7 @@ cli
.option('--allowOnly', 'Allow tests and suites that are marked as only (default: !process.env.CI)')
.option('--shard <shard>', 'Test suite shard to execute in a format of <index>/<count>')
.option('--changed [since]', 'Run tests that are affected by the changed files (default: false)')
.option('--random', 'Run tests in random order (default: false)')
.help()

cli
Expand Down
13 changes: 13 additions & 0 deletions packages/vitest/src/node/config.ts
Expand Up @@ -9,6 +9,8 @@ import { configDefaults } from '../defaults'
import { resolveC8Options } from '../integrations/coverage'
import { toArray } from '../utils'
import { VitestCache } from './cache'
import { BaseSequencer } from './sequencers/BaseSequencer'
import { RandomSequencer } from './sequencers/RandomSequencer'

const extraInlineDeps = [
/^(?!.*(?:node_modules)).*\.mjs$/,
Expand Down Expand Up @@ -186,5 +188,16 @@ export function resolveConfig(
if (resolved.cache)
resolved.cache.dir = VitestCache.resolveCacheDir(resolved.root, resolved.cache.dir)

// random should have the priority over config, because it is a CLI flag
if (!resolved.sequence?.sequencer || resolved.random) {
resolved.sequence ??= {} as any
// CLI flag has higher priority
if (resolved.random)
resolved.sequence.random = true
resolved.sequence.sequencer = resolved.sequence.random
? RandomSequencer
: BaseSequencer
}

return resolved
}
4 changes: 4 additions & 0 deletions packages/vitest/src/node/core.ts
Expand Up @@ -105,6 +105,10 @@ export class Vitest {
resolveSnapshotPath: undefined,
},
onConsoleLog: undefined!,
sequence: {
...this.config.sequence,
sequencer: undefined!,
},
},
this.configOverride || {} as any,
) as ResolvedConfig
Expand Down
3 changes: 3 additions & 0 deletions packages/vitest/src/node/index.ts
Expand Up @@ -5,3 +5,6 @@ export { startVitest } from './cli-api'

export { VitestRunner } from '../runtime/execute'
export type { ExecuteOptions } from '../runtime/execute'

export type { TestSequencer, TestSequencerContructor } from './sequencers/types'
export { BaseSequencer } from './sequencers/BaseSequencer'
8 changes: 4 additions & 4 deletions packages/vitest/src/node/pool.ts
Expand Up @@ -10,7 +10,6 @@ import type { ResolvedConfig, WorkerContext, WorkerRPC } from '../types'
import { distDir } from '../constants'
import { AggregateError } from '../utils'
import type { Vitest } from './core'
import { BaseSequelizer } from './sequelizers/BaseSequelizer'

export type RunWithFiles = (files: string[], invalidates?: string[]) => Promise<void>

Expand Down Expand Up @@ -86,15 +85,16 @@ export function createPool(ctx: Vitest): WorkerPool {
}
}

const sequelizer = new BaseSequelizer(ctx)
const Sequencer = ctx.config.sequence.sequencer
const sequencer = new Sequencer(ctx)

return async (files, invalidates) => {
const config = ctx.getSerializableConfig()

if (config.shard)
files = await sequelizer.shard(files)
files = await sequencer.shard(files)

files = await sequelizer.sort(files)
files = await sequencer.sort(files)

if (!ctx.config.threads) {
await runFiles(config, files)
Expand Down
Expand Up @@ -2,9 +2,9 @@ import { createHash } from 'crypto'
import { resolve } from 'pathe'
import { slash } from 'vite-node/utils'
import type { Vitest } from '../core'
import type { TestSequelizer } from './types'
import type { TestSequencer } from './types'

export class BaseSequelizer implements TestSequelizer {
export class BaseSequencer implements TestSequencer {
protected ctx: Vitest

constructor(ctx: Vitest) {
Expand Down
12 changes: 12 additions & 0 deletions packages/vitest/src/node/sequencers/RandomSequencer.ts
@@ -0,0 +1,12 @@
import { randomize } from '../../utils'
import { BaseSequencer } from './BaseSequencer'

export class RandomSequencer extends BaseSequencer {
public async sort(files: string[]) {
const { sequence } = this.ctx.config

const seed = sequence?.seed ?? Date.now()

return randomize(files, seed)
}
}
@@ -1,7 +1,7 @@
import type { Awaitable } from '../../types'
import type { Vitest } from '../core'

export interface TestSequelizer {
export interface TestSequencer {
/**
* Slicing tests into shards. Will be run before `sort`.
* Only run, if `shard` is defined.
Expand All @@ -10,6 +10,6 @@ export interface TestSequelizer {
sort(files: string[]): Awaitable<string[]>
}

export interface TestSequelizerContructor {
new (ctx: Vitest): TestSequelizer
export interface TestSequencerContructor {
new (ctx: Vitest): TestSequencer
}
12 changes: 10 additions & 2 deletions packages/vitest/src/runtime/run.ts
Expand Up @@ -2,7 +2,7 @@ import limit from 'p-limit'
import type { File, HookCleanupCallback, HookListener, ResolvedConfig, Suite, SuiteHooks, Task, TaskResult, TaskState, Test } from '../types'
import { vi } from '../integrations/vi'
import { getSnapshotClient } from '../integrations/snapshot/chai'
import { clearTimeout, getFullName, getWorkerState, hasFailed, hasTests, partitionSuiteChildren, setTimeout } from '../utils'
import { clearTimeout, getFullName, getWorkerState, hasFailed, hasTests, partitionSuiteChildren, randomize, setTimeout } from '../utils'
import { takeCoverage } from '../integrations/coverage'
import { getState, setState } from '../integrations/chai/jest-expect'
import { GLOBAL_EXPECT } from '../integrations/chai/constants'
Expand Down Expand Up @@ -210,12 +210,20 @@ export async function runSuite(suite: Suite) {
try {
const beforeAllCleanups = await callSuiteHook(suite, suite, 'beforeAll', [suite])

for (const tasksGroup of partitionSuiteChildren(suite)) {
for (let tasksGroup of partitionSuiteChildren(suite)) {
if (tasksGroup[0].concurrent === true) {
const mutex = limit(workerState.config.maxConcurrency)
await Promise.all(tasksGroup.map(c => mutex(() => runSuiteChild(c))))
}
else {
const { sequence } = workerState.config
if (sequence.random || suite.random) {
// run describe block independently from tests
const suites = tasksGroup.filter(group => group.type === 'suite')
const tests = tasksGroup.filter(group => group.type === 'test')
const groups = randomize([suites, tests], sequence.seed)
tasksGroup = groups.flatMap(group => randomize(group, sequence.seed))
}
for (const c of tasksGroup)
await runSuiteChild(c)
}
Expand Down
17 changes: 12 additions & 5 deletions packages/vitest/src/runtime/suite.ts
@@ -1,6 +1,6 @@
import { format } from 'util'
import type { File, RunMode, Suite, SuiteAPI, SuiteCollector, SuiteFactory, SuiteHooks, Task, Test, TestAPI, TestFunction } from '../types'
import { isObject, noop } from '../utils'
import { getWorkerState, isObject, noop } from '../utils'
import { createChainable } from './chain'
import { collectTask, collectorContext, createTestContext, runWithSuite, withTimeout } from './context'
import { getHooks, setFn, setHooks } from './map'
Expand Down Expand Up @@ -37,8 +37,12 @@ function formatTitle(template: string, items: any[], idx: number) {
export const describe = suite
export const it = test

const workerState = getWorkerState()

// implementations
export const defaultSuite = suite('')
export const defaultSuite = workerState.config.sequence.random
? suite.random('')
: suite('')

export function clearCollectorContext() {
collectorContext.tasks.length = 0
Expand All @@ -59,7 +63,7 @@ export function createSuiteHooks() {
}
}

function createSuiteCollector(name: string, factory: SuiteFactory = () => { }, mode: RunMode, concurrent?: boolean) {
function createSuiteCollector(name: string, factory: SuiteFactory = () => { }, mode: RunMode, concurrent?: boolean, random?: boolean) {
const tasks: (Test | Suite | SuiteCollector)[] = []
const factoryQueue: (Test | Suite | SuiteCollector)[] = []

Expand All @@ -80,6 +84,8 @@ function createSuiteCollector(name: string, factory: SuiteFactory = () => { }, m
} as Omit<Test, 'context'> as Test
if (this.concurrent || concurrent)
test.concurrent = true
if (random)
test.random = true

const context = createTestContext(test)
// create test context
Expand Down Expand Up @@ -117,6 +123,7 @@ function createSuiteCollector(name: string, factory: SuiteFactory = () => { }, m
type: 'suite',
name,
mode,
random,
tasks: [],
}
setHooks(suite, createSuiteHooks())
Expand Down Expand Up @@ -157,10 +164,10 @@ function createSuiteCollector(name: string, factory: SuiteFactory = () => { }, m

function createSuite() {
const suite = createChainable(
['concurrent', 'skip', 'only', 'todo'],
['concurrent', 'random', 'skip', 'only', 'todo'],
function (name: string, factory?: SuiteFactory) {
const mode = this.only ? 'only' : this.skip ? 'skip' : this.todo ? 'todo' : 'run'
return createSuiteCollector(name, factory, mode, this.concurrent)
return createSuiteCollector(name, factory, mode, this.concurrent, this.random)
},
) as SuiteAPI

Expand Down
38 changes: 37 additions & 1 deletion packages/vitest/src/types/config.ts
Expand Up @@ -2,6 +2,7 @@ import type { CommonServerOptions } from 'vite'
import type { PrettyFormatOptions } from 'pretty-format'
import type { FakeTimerInstallOpts } from '@sinonjs/fake-timers'
import type { BuiltinReporters } from '../node/reporters'
import type { TestSequencerContructor } from '../node/sequencers/types'
import type { C8Options, ResolvedC8Options } from './coverage'
import type { JSDOMOptions } from './jsdom-options'
import type { Reporter } from './reporter'
Expand Down Expand Up @@ -369,6 +370,29 @@ export interface InlineConfig {
cache?: false | {
dir?: string
}

/**
* Options for configuring the order of running tests.
*/
sequence?: {
/**
* Class that handles sorting and sharding algorithm.
* If you only need to change sorting, you can extend
* your custom sequencer from `BaseSequencer` from `vitest/node`.
* @default BaseSequencer
*/
sequencer?: TestSequencerContructor
/**
* Should tests run in random order.
* @default false
*/
random?: boolean
/**
* Seed for the random number generator.
* @default Date.now()
*/
seed?: number
}
}

export interface UserConfig extends InlineConfig {
Expand Down Expand Up @@ -413,9 +437,15 @@ export interface UserConfig extends InlineConfig {
* @example --shard=2/3
*/
shard?: string

/**
* If tests should be run in random.
* @default false
*/
random?: boolean
}

export interface ResolvedConfig extends Omit<Required<UserConfig>, 'config' | 'filters' | 'coverage' | 'testNamePattern' | 'related' | 'api' | 'reporters' | 'resolveSnapshotPath' | 'shard' | 'cache'> {
export interface ResolvedConfig extends Omit<Required<UserConfig>, 'config' | 'filters' | 'coverage' | 'testNamePattern' | 'related' | 'api' | 'reporters' | 'resolveSnapshotPath' | 'shard' | 'cache' | 'sequence'> {
base?: string

config?: string
Expand All @@ -439,4 +469,10 @@ export interface ResolvedConfig extends Omit<Required<UserConfig>, 'config' | 'f
cache: {
dir: string
} | false

sequence: {
sequencer: TestSequencerContructor
random?: boolean
seed?: number
}
}
3 changes: 2 additions & 1 deletion packages/vitest/src/types/tasks.ts
Expand Up @@ -10,6 +10,7 @@ export interface TaskBase {
name: string
mode: RunMode
concurrent?: boolean
random?: boolean
suite?: Suite
file?: File
result?: TaskResult
Expand Down Expand Up @@ -113,7 +114,7 @@ void
}

export type SuiteAPI<ExtraContext = {}> = ChainableFunction<
'concurrent' | 'only' | 'skip' | 'todo',
'concurrent' | 'only' | 'skip' | 'todo' | 'random',
[name: string, factory?: SuiteFactory],
SuiteCollector<ExtraContext>
> & {
Expand Down