refactor: common vitest configurations #1107
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,22 +1,17 @@ | ||||||||||||||
| /// <reference types="vitest" /> | ||||||||||||||
| import { createE2eConfig } from '../../testing/test-setup-config/src/lib/vitest-setup-presets.js'; | ||||||||||||||
|
|
||||||||||||||
| export default createE2eConfig( | ||||||||||||||
| 'ci-e2e', | ||||||||||||||
| { | ||||||||||||||
| projectRoot: new URL('../../', import.meta.url), | ||||||||||||||
| cacheKey: 'ci-e2e', | ||||||||||||||
| }, | ||||||||||||||
| { | ||||||||||||||
| test: { | ||||||||||||||
| testTimeout: 60_000, | ||||||||||||||
|
There was a problem hiding this comment. this could be a default for the e2e target There was a problem hiding this comment. I checked other files and there are many different values, like 20k, 40k, 80k, 60k, 120k. There was a problem hiding this comment. Hmmm.... I would put 20 as default. Or if there are more projects with 40 lets go with 40. And for the projects with higher or lower times go with a override. Did not realise this is already an overwrite.. There was a problem hiding this comment. I checked that and:
I suggest then use 20s as default (20_000ms) There was a problem hiding this comment. The timeouts might actually be a justifiable exception to keeping everything unified. 🤔 Some E2E tests require a generous test timeout, but it wouldn't be optimal to have a high test timeout everywhere. I would handle these kinds of situations with a limited options object. For example:
However, it would be worth considering if we really need to configure a custom timeout for a whole project. It's also possible to override timeouts per individual |
||||||||||||||
| globalSetup: ['./global-setup.ts'], | ||||||||||||||
| coverage: { enabled: false }, | ||||||||||||||
| }, | ||||||||||||||
BioPhoton marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||
| }, | ||||||||||||||
| ); | ||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,26 +1,16 @@ | ||
| /// <reference types="vitest" /> | ||
| import { createE2eConfig } from '../../testing/test-setup-config/src/lib/vitest-setup-presets.js'; | ||
|
|
||
| export default createE2eConfig( | ||
| 'plugin-typescript-e2e', | ||
| { | ||
| projectRoot: new URL('../../', import.meta.url), | ||
| cacheKey: 'plugin-typescript-e2e', | ||
| }, | ||
| { | ||
| test: { | ||
| testTimeout: 20_000, | ||
| coverage: { enabled: true }, | ||
| }, | ||
| }, | ||
| ); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,30 +1,17 @@ | ||
| /// <reference types="vitest" /> | ||
| import { | ||
| createIntConfig, | ||
| setupPresets, | ||
| } from '../../testing/test-setup-config/src/lib/vitest-setup-presets.js'; | ||
|
|
||
| export default createIntConfig( | ||
| 'core', | ||
| { | ||
| projectRoot: new URL('../../', import.meta.url), | ||
|
There was a problem hiding this comment. The "project root" terminology is inconsistent with how Nx uses it. For this example, the project root would be the In fact, since the workspace root is the same for each project, I don't see any reason this needs to be a parameter for the factory function. It would be more encapsulated and reliable if the factory function created the absolute path to the workspace root internally. |
||
| }, | ||
| { | ||
| test: { | ||
| setupFiles: [...setupPresets.int.base, ...setupPresets.int.portalClient], | ||
| }, | ||
| }, | ||
| ); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,36 +1,22 @@ | ||
| /// <reference types="vitest" /> | ||
| import { | ||
| createUnitConfig, | ||
| setupPresets, | ||
| } from '../../testing/test-setup-config/src/lib/vitest-setup-presets.js'; | ||
|
|
||
| export default createUnitConfig( | ||
| 'core', | ||
| { | ||
| projectRoot: new URL('../../', import.meta.url), | ||
| }, | ||
| { | ||
| test: { | ||
| setupFiles: [ | ||
| ...setupPresets.unit.base, | ||
| ...setupPresets.unit.git, | ||
| ...setupPresets.unit.portalClient, | ||
| ...setupPresets.unit.matchersCore, | ||
| ], | ||
| }, | ||
| }, | ||
| ); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,30 +1,18 @@ | ||
| /// <reference types="vitest" /> | ||
| import { | ||
| createIntConfig, | ||
| setupPresets, | ||
| } from '../../testing/test-setup-config/src/lib/vitest-setup-presets.js'; | ||
|
|
||
| export default createIntConfig( | ||
| 'utils', | ||
| { | ||
| projectRoot: new URL('../../', import.meta.url), | ||
| }, | ||
| { | ||
| test: { | ||
| coverage: { exclude: ['perf/**'] }, | ||
| setupFiles: [...setupPresets.int.base, ...setupPresets.int.cliui], | ||
| }, | ||
| }, | ||
| ); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,38 +1,24 @@ | ||
| /// <reference types="vitest" /> | ||
| import { | ||
| createUnitConfig, | ||
| setupPresets, | ||
| } from '../../testing/test-setup-config/src/lib/vitest-setup-presets.js'; | ||
|
|
||
| export default createUnitConfig( | ||
| 'utils', | ||
| { | ||
| projectRoot: new URL('../../', import.meta.url), | ||
| }, | ||
| { | ||
| test: { | ||
| include: ['src/**/*.{unit,type}.test.{js,mjs,cjs,ts,mts,cts,jsx,tsx}'], | ||
| typecheck: { include: ['**/*.type.test.ts'] }, | ||
|
Comment on lines
+14
to
+15
There was a problem hiding this comment. In the interest of avoiding overrides per project (see previous comment), I would apply this wider file pattern for each project. Most projects don't happen to use |
||
| coverage: { exclude: ['perf/**'] }, | ||
|
There was a problem hiding this comment. I would prefer to limit how much variation we have between each project's Vitest configs. Because they were all separate files until now, there are some minor inconsistencies between them. However, in most cases, I don't believe there's a compelling reason for those variations; it just happened to evolve that way. One such example is the |
||
| setupFiles: [ | ||
| ...setupPresets.unit.base, | ||
| ...setupPresets.unit.matchersCore, | ||
| ...setupPresets.unit.matcherPath, | ||
| ], | ||
| }, | ||
| }, | ||
| ); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| ## Vitest config factory and setup presets | ||
|
|
||
| Utilities to centralize and standardize Vitest configuration across the monorepo. | ||
|
|
||
| - `vitest-config-factory.ts`: builds typed Vitest configs with sensible defaults | ||
| - `vitest-setup-presets.ts`: provides create functions and exportable setup file groups | ||
|
Comment on lines
+5
to
+6
There was a problem hiding this comment. If I'm only interested in using this project, not maintaining it, then do I need to know about these files? I would think not; they look like implementation details. The project README should first document usage (i.e., your Examples section). Any internal information should be moved to the bottom of the README, as it's least likely to be relevant. That's if it's even needed - IMHO, the file names are pretty self-explanatory, and I can look them up in |
||
|
|
||
| The create functions (`createUnitConfig`, `createIntConfig`, `createE2eConfig`) automatically include appropriate setup files for each test type. See the unit tests for detailed documentation of defaults, coverage settings, and setup file presets. | ||
|
There was a problem hiding this comment. I'd rename the exported functions to |
||
|
|
||
| ### Examples | ||
|
|
||
| **Using defaults:** | ||
|
|
||
| ```ts | ||
| export default createUnitConfig('my-package', import.meta.url); | ||
| ``` | ||
|
|
||
| **Extending default setup files:** | ||
|
|
||
| ```ts | ||
| export default createIntConfig('my-package', import.meta.url, { | ||
| setupFiles: [...setupPresets.int.base, ...setupPresets.int.git, './custom-setup.ts'], | ||
| }); | ||
| ``` | ||
|
Comment on lines
+12
to
+24
There was a problem hiding this comment. I would include |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| import tseslint from 'typescript-eslint'; | ||
| import baseConfig from '../../eslint.config.js'; | ||
|
|
||
| export default tseslint.config(...baseConfig, { | ||
| files: ['**/*.ts'], | ||
| languageOptions: { | ||
| parserOptions: { | ||
| projectService: true, | ||
| tsconfigRootDir: import.meta.dirname, | ||
| }, | ||
| }, | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| { | ||
| "name": "test-setup-config", | ||
| "$schema": "../../node_modules/nx/schemas/project-schema.json", | ||
| "sourceRoot": "testing/test-setup/src", | ||
| "projectType": "library", | ||
| "targets": { | ||
| "build": {}, | ||
| "lint": {}, | ||
| "unit-test": {} | ||
| }, | ||
| "tags": ["scope:shared", "type:testing"] | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| export { | ||
| createVitestConfig, | ||
| type TestKind, | ||
| type VitestConfigFactoryOptions, | ||
| type VitestOverrides, | ||
| type ConfigRestParams, | ||
| } from './lib/vitest-config-factory.js'; | ||
|
|
||
| export { | ||
| setupPresets, | ||
| createUnitConfig, | ||
| createIntConfig, | ||
| createE2eConfig, | ||
| } from './lib/vitest-setup-presets.js'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which tool uses
.coveragefolders? 🤨There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one could be used to exclude colocated coverage folders of each project.