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: add caching to run failed and longer tests first #1541

Merged
merged 17 commits into from
Jul 3, 2022
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions docs/config/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -564,3 +564,16 @@ RegExp pattern for files that will return en empty CSS file.
A number of tests that are allowed to run at the same time marked with `test.concurrent`.

Test above this limit will be queued to run when available slot appears.

### cache

- **Type**: `false | { dir? }`

Options to configure Vitest cache policy. At the moment Vitest stores cache for test results to run the longer and failed tests first.

#### cache.dir

- **Type**: `string`
- **Default**: `node_modules/.vitest`

Path to cache directory.
4 changes: 4 additions & 0 deletions docs/guide/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ Useful to run with [`lint-staged`](https://github.com/okonet/lint-staged) or wit
vitest related /src/index.ts /src/hello-world.js
```

### `vitest clearCache`
Copy link
Member

@antfu antfu Jul 2, 2022

Choose a reason for hiding this comment

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

Suggested change
### `vitest clearCache`
### `vitest purge`

I am not a fan of having pascal case in CLI. Maybe purge, WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

Why purge tho? Don't see any connection to cache. We can do clear-cache, I guess.

Copy link
Member

Choose a reason for hiding this comment

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

Just want to find a single word to represent it. As for reference, here is what package managers are doing:

npm cache clean --force
yarn cache clean
pnpm store prune

And for Vite: https://vitejs.dev/guide/dep-pre-bundling.html#file-system-cache

/cc @patak-dev in case we are also going to add a clear cache command for Vite, I think we better align the commands

Copy link
Member Author

Choose a reason for hiding this comment

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

We can do vitect clean, and pass cache as an argument. In future we might add option to clear only some specific caches.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've updated to proposed solution. What do you think, @antfu, @patak-dev?

Copy link
Member Author

@sheremet-va sheremet-va Jul 3, 2022

Choose a reason for hiding this comment

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

it is something you use a lot so it is better if it is a single word

Hardly disagree with using a lot - I don't know why you would even use it in Vitest, only to clear malformed cache maybe.

Also single words only look good, but confuse a lot. What --clean means? What happens when you call it? I want to have cache in the name somewhere.

Copy link
Member

Choose a reason for hiding this comment

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

I use --force a lot because ya, I'm debugging Vite cache itself 👀
So I agree with you that normally isn't something that final users will be reaching a lot. I'm fine with --clearCache (maybe good to check how it will look later for --clearCache="parts of it")

Copy link
Member Author

Choose a reason for hiding this comment

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

maybe good to check how it will look later

It will look something like this:

vitest run --clearCache results

We use cac's notation

Copy link
Member Author

Choose a reason for hiding this comment

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

@antfu what do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

I will remove command for now, since I want this feature to be released today. Command will be in another PR.


Clears cache folder.

## Options

| Options | |
Expand Down
23 changes: 23 additions & 0 deletions packages/vitest/src/node/cache/files.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import fs, { type Stats } from 'fs'

type FileStatsCache = Pick<Stats, 'size'>

export class FilesStatsCache {
public cache = new Map<string, FileStatsCache>()

public getStats(fsPath: string): FileStatsCache | undefined {
return this.cache.get(fsPath)
}

public async updateStats(fsPath: string) {
if (!fs.existsSync(fsPath))
return

const stats = await fs.promises.stat(fsPath)
this.cache.set(fsPath, { size: stats.size })
}

public removeStats(fsPath: string) {
this.cache.delete(fsPath)
}
}
41 changes: 41 additions & 0 deletions packages/vitest/src/node/cache/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import fs from 'fs'
import { findUp } from 'find-up'
import { resolve } from 'pathe'
import { loadConfigFromFile } from 'vite'
import { configFiles } from '../../constants'
import type { CliOptions } from '../cli-api'
import { slash } from '../../utils'

export class VitestCache {
static resolveCacheDir(root: string, dir: string | undefined) {
return resolve(root, slash(dir || 'node_modules/.vitest'))
}

static async clearCache(options: CliOptions) {
const root = resolve(options.root || process.cwd())

const configPath = options.config
? resolve(root, options.config)
: await findUp(configFiles, { cwd: root } as any)

const config = await loadConfigFromFile({ command: 'serve', mode: 'test' }, configPath)

if (!config)
throw new Error(`[vitest] Not able to load config from ${configPath}`)

const cache = config.config.test?.cache

if (cache === false)
throw new Error('[vitest] Cache is disabled')

const cachePath = VitestCache.resolveCacheDir(root, cache?.dir)

let cleared = false

if (fs.existsSync(cachePath)) {
fs.rmSync(cachePath, { recursive: true, force: true })
cleared = true
}
return { dir: cachePath, cleared }
}
}
74 changes: 74 additions & 0 deletions packages/vitest/src/node/cache/results.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import fs from 'fs'
import { dirname, resolve } from 'pathe'
import type { File, ResolvedConfig } from '../../types'
import { version } from '../../../package.json'

export interface SuiteResultCache {
failed: boolean
duration: number
}

export class ResultsCache {
private cache = new Map<string, SuiteResultCache>()
private cachePath: string | null = null
private version: string = version
private root = '/'

setConfig(root: string, config: ResolvedConfig['cache']) {
this.root = root
if (config)
this.cachePath = resolve(config.dir, 'results.json')
}

getResults(fsPath: string) {
return this.cache.get(fsPath?.slice(this.root.length))
}

async readFromCache() {
if (!this.cachePath)
return

if (fs.existsSync(this.cachePath)) {
const resultsCache = await fs.promises.readFile(this.cachePath, 'utf8')
const { results, version } = JSON.parse(resultsCache)
this.cache = new Map(results)
this.version = version
}
}

updateResults(files: File[]) {
files.forEach((file) => {
const result = file.result
if (!result)
return
const duration = result.duration || 0
// store as relative, so cache would be the same in CI and locally
const relativePath = file.filepath?.slice(this.root.length)
this.cache.set(relativePath, {
duration: duration >= 0 ? duration : 0,
failed: result.state === 'fail',
})
})
}

removeFromCache(filepath: string) {
this.cache.delete(filepath)
}

async writeToCache() {
if (!this.cachePath)
return

const resultsCache = Array.from(this.cache.entries())

const cacheDirname = dirname(this.cachePath)

if (!fs.existsSync(cacheDirname))
await fs.promises.mkdir(cacheDirname, { recursive: true })

await fs.promises.writeFile(this.cachePath, JSON.stringify({
version: this.version,
results: resultsCache,
}))
}
}
13 changes: 13 additions & 0 deletions packages/vitest/src/node/cli.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import cac from 'cac'
import c from 'picocolors'
import { version } from '../../package.json'
import { VitestCache } from './cache'
import type { CliOptions } from './cli-api'
import { startVitest } from './cli-api'
import { divider } from './reporters/renderers/utils'
Expand Down Expand Up @@ -57,8 +58,20 @@ cli
.command('[...filters]')
.action(start)

cli.command('clearCache')
.action(clearCache)

cli.parse()

async function clearCache(args: CliOptions) {
const { dir, cleared } = await VitestCache.clearCache(args)

if (cleared)
console.log(c.bgGreen(' VITEST '), `Cache cleared at ${dir}`)
else
console.log(c.bgRed(' VITEST '), `No cache found at ${dir}`)
}

async function runRelated(relatedFiles: string[] | string, argv: CliOptions) {
argv.related = relatedFiles
argv.passWithNoTests ??= true
Expand Down
5 changes: 5 additions & 0 deletions packages/vitest/src/node/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { defaultPort } from '../constants'
import { configDefaults } from '../defaults'
import { resolveC8Options } from '../integrations/coverage'
import { toArray } from '../utils'
import { VitestCache } from './cache'

const extraInlineDeps = [
/^(?!.*(?:node_modules)).*\.mjs$/,
Expand Down Expand Up @@ -181,5 +182,9 @@ export function resolveConfig(
if (typeof resolved.css === 'object')
resolved.css.include ??= [/\.module\./]

resolved.cache ??= { dir: '' }
if (resolved.cache)
resolved.cache.dir = VitestCache.resolveCacheDir(resolved.root, resolved.cache.dir)

return resolved
}
22 changes: 18 additions & 4 deletions packages/vitest/src/node/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@ export class Vitest {

if (resolved.coverage.enabled)
await cleanCoverage(resolved.coverage, resolved.coverage.clean)

this.state.results.setConfig(resolved.root, resolved.cache)
await this.state.results.readFromCache()
}

getSerializableConfig() {
Expand Down Expand Up @@ -133,6 +136,9 @@ export class Vitest {
process.exit(exitCode)
}

// populate once, update cache on watch
await Promise.all(files.map(file => this.state.stats.updateStats(file)))

await this.runFiles(files)

if (this.config.coverage.enabled)
Expand Down Expand Up @@ -205,7 +211,7 @@ export class Vitest {
return runningTests
}

async runFiles(files: string[]) {
async runFiles(paths: string[]) {
await this.runningPromise

this.runningPromise = (async () => {
Expand All @@ -217,16 +223,21 @@ export class Vitest {
this.snapshot.clear()
this.state.clearErrors()
try {
await this.pool.runTests(files, invalidates)
await this.pool.runTests(paths, invalidates)
}
catch (err) {
this.state.catchError(err, 'Unhandled Error')
}

if (hasFailed(this.state.getFiles()))
const files = this.state.getFiles()

if (hasFailed(files))
process.exitCode = 1

await this.report('onFinished', this.state.getFiles(), this.state.getUnhandledErrors())
await this.report('onFinished', files, this.state.getUnhandledErrors())

this.state.results.updateResults(files)
await this.state.results.writeToCache()
})()
.finally(() => {
this.runningPromise = undefined
Expand Down Expand Up @@ -352,6 +363,8 @@ export class Vitest {

if (this.state.filesMap.has(id)) {
this.state.filesMap.delete(id)
this.state.results.removeFromCache(id)
this.state.stats.removeStats(id)
this.changedTests.delete(id)
this.report('onTestRemoved', id)
}
Expand All @@ -360,6 +373,7 @@ export class Vitest {
id = slash(id)
if (await this.isTargetFile(id)) {
this.changedTests.add(id)
await this.state.stats.updateStats(id)
this.scheduleRerun(id)
}
}
Expand Down
30 changes: 8 additions & 22 deletions packages/vitest/src/node/pool.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import { MessageChannel } from 'worker_threads'
import { pathToFileURL } from 'url'
import { cpus } from 'os'
import { createHash } from 'crypto'
import { resolve } from 'pathe'
import type { Options as TinypoolOptions } from 'tinypool'
import { Tinypool } from 'tinypool'
import { createBirpc } from 'birpc'
import type { RawSourceMap } from 'vite-node'
import type { ResolvedConfig, WorkerContext, WorkerRPC } from '../types'
import { distDir } from '../constants'
import { AggregateError, slash } from '../utils'
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,29 +86,15 @@ export function createPool(ctx: Vitest): WorkerPool {
}
}

const sequelizer = new BaseSequelizer(ctx)

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

if (config.shard) {
const { index, count } = config.shard
const shardSize = Math.ceil(files.length / count)
const shardStart = shardSize * (index - 1)
const shardEnd = shardSize * index
files = files
.map((file) => {
const fullPath = resolve(slash(config.root), slash(file))
const specPath = fullPath.slice(config.root.length)
return {
file,
hash: createHash('sha1')
.update(specPath)
.digest('hex'),
}
})
.sort((a, b) => (a.hash < b.hash ? -1 : a.hash > b.hash ? 1 : 0))
.slice(shardStart, shardEnd)
.map(({ file }) => file)
}
if (config.shard)
files = await sequelizer.shard(files)

files = await sequelizer.sort(files)

if (!ctx.config.threads) {
await runFiles(config, files)
Expand Down
Loading