Skip to content

Commit

Permalink
ci: vitest workspaces (#998)
Browse files Browse the repository at this point in the history
## 🧰 Changes

> [!IMPORTANT]
> **this continues to be a vitest supremacy account 😤**

In this repo, we have tests that use `process.chdir()`, so we maintain a
separate Vitest config for running those via a
[`forks`](https://vitest.dev/config/#forks) pool vs. the standard
[`threads`](https://vitest.dev/config/#threads) pool — see
#870 for more background on this.

We've used two separate npm scripts (i.e., `npm run test:multi && npm
run test:single`) to run our tests sequentially. I discovered this
morning that [Vitest's workspaces
feature](https://vitest.dev/guide/workspace) isn't just for monorepos —
it's actually the recommended way to maintain multiple test
configurations no matter the context.

In this PR, I consolidated our two test configs into a single workspace
config. We get a few nice benefits by leveraging workspaces:
- No longer needing to manually pass in the correct config file when
running certain test files (e.g., `npx vitest --config
vitest.single-threaded.config.ts [file]`). In other words, `npx vitest
[file]` ✨ just works ✨, which should make it a lot easier for new
contributors to run tests
- Much cleaner test output — we now can see coverage across all files,
rather than two separate coverage reports[^1]
- Easier to share options between the two configs
- A single config file to rule them all
- Tests seem to be faster (but hard to say)

## 🧬 QA & Testing

Do all the tests run? And do they still run and pass properly? Here's
the latest test run in `next` for comparison:
https://github.com/readmeio/rdme/actions/runs/8910638856

[^1]: Small downside to this — [workspaces don't allow you to configure
coverage options](https://vitest.dev/guide/workspace#coverage). Not a
huge deal since our coverage is great in this repo and we can stick to
the defaults and it still addresses our needs.
  • Loading branch information
kanadgupta authored May 1, 2024
1 parent 8cb6acf commit 1713d6b
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 45 deletions.
4 changes: 1 addition & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,7 @@
"prettier": "prettier --check .",
"schemas:check": "./bin/json-schema-store.js",
"schemas:write": "./bin/json-schema-store.js --update",
"test": "npm run test:multi && npm run test:single",
"test:multi": "vitest run --coverage",
"test:single": "vitest run --coverage --config vitest.single-threaded.config.ts"
"test": "vitest run --coverage"
},
"commitlint": {
"extends": [
Expand Down
19 changes: 0 additions & 19 deletions vitest.config.ts

This file was deleted.

23 changes: 0 additions & 23 deletions vitest.single-threaded.config.ts

This file was deleted.

48 changes: 48 additions & 0 deletions vitest.workspace.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// eslint-disable-next-line import/no-extraneous-dependencies
import { defineWorkspace } from 'vitest/config';

const sharedExclude = [
'**/dist/**',
'**/node_modules/**',
'**/__fixtures__/**',
'**/helpers/**',
'**/__snapshots__/**',
];

const sharedOpts = {
globalSetup: ['./__tests__/setup.js'],
setupFiles: ['./__tests__/helpers/vitest.matchers.ts'],
};

export default defineWorkspace([
{
/**
* Our default test suite
*/
test: {
exclude: [...sharedExclude, '**/single-threaded/**'],
name: 'default',
...sharedOpts,
},
},
{
/**
* Our single-threaded test suite.
* Only tests that use `process.chdir()` should be run using this config.
*/
test: {
exclude: sharedExclude,
include: ['**/single-threaded/**'],
name: 'single-threaded',
/**
* We can't run tests with `threads` on because we use `process.chdir()` in some tests and
* that isn't available in worker threads, and it's way too much work to mock out an entire
* filesystem and `fs` calls for the tests that use it.
*
* @see {@link https://github.com/vitest-dev/vitest/issues/566}
*/
pool: 'forks',
...sharedOpts,
},
},
]);

0 comments on commit 1713d6b

Please sign in to comment.