Run frontend unit tests using vitest instead of jest #246
Conversation
There was a problem hiding this comment.
LGTM so far. Regarding the questions. I think that moving the add-on to be module is not bad, in fact, it's a good thing and ready for ESM brave new world. If your add-on is a module then all scripts are ESM, and you have to mark the ones that are not.
This is needed because we are importing the ESM package @plone/volto/vitest.config.mjs and there's no other way to do it. ESM only imports from ESM too. Newer Node.js versions have more interop support, but it's what it is. Having the add-on as ESM module is good.
The deps are not avoidable, the package is using vitest locally.
There's still left what to communicate (and how and when too) if the user is upgrading the add-on to latest boilerplate, and have Jest tests in place. We have it documented, and it's an easy task, but we need to make it noticeable.
|
@davisagli Now I think it's ready from the implementation point of view. |
|
@sneridagh I agree, I was pleasantly surprised by how little needed to change in existing tests. I guess changes are mostly needed if you are doing something more complicated with custom mocks. I'll add a release note which refers to https://6.docs.plone.org/volto/development/add-ons/test-add-ons-18.html (which needs some updates, based on what we learned here). |
|
We also have: #85 But there is not using the Volto configuration. Should we close it there then? |
| // 'promise-file-reader': require.resolve('promise-file-reader') // Add to identify dependency from package | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Can't the test configuration be in the root folder of the monorepo, as is done with Jest? This way it is not necessary to repeat the configuration when we have more than one package in the monorepo.
| "build:deps": "pnpm --filter @plone/registry --filter @plone/components build", | ||
| "i18n": "pnpm --filter {{ cookiecutter.__npm_package_name }} i18n && VOLTOCONFIG=$(pwd)/volto.config.js pnpm --filter @plone/volto i18n", | ||
| "test": "RAZZLE_JEST_CONFIG=$(pwd)/jest-addon.config.js pnpm --filter @plone/volto test -- --passWithNoTests", | ||
| "test": "pnpm --filter {{ cookiecutter.__npm_package_name }} exec vitest", |
There was a problem hiding this comment.
What if we have more than one package in the monorepo? Can't we run all the tests at once?
There was a problem hiding this comment.
@wesleybl we are just replicating how the ecosystem works. If one package has tests, it should depend on vitest, hence, you have to be able to run it in the package itself. The monorepo is not a buildout. It's a way of tie together NodeJS packages.
You can still run them all at once with pnpm, that's why the --filter option exist.
pnpm --filter mypackage1 --filter mypackage2 exec vitest
The monorepo managers (as in turborepo, etc) also does this, they orchestrate commands through different packages who are meant to act independently.
There was a problem hiding this comment.
@sneridagh But generally, in monorepos, it's common for the root test script to run tests for all packages. Shouldn't we expect this here? If not, and someone wants to have more than one package in the monorepo, they would have to "customize" the package.json created by cookieplone. It could be something like:
"test": "pnpm --filter './packages/*' exec vitest"
Fixes #207
This is based on what @sneridagh did in https://github.com/kitconcept/volto-bm3-compat
There are a bunch of things I am not sure about...
vitest and @testing-library/react have to be explicitly listed as devDependencies of the add-on.
The add-on has to havetype: "module"in order to be able to import @plone/volto/vitest.config.mjs ... does this have side effects?If a Volto 18 add-on that already has Jest tests updates its boilerplate after this change, the developer is forced to rewrite the tests with vitest (or take care not to update the lines that switch to vitest)
I didn't touch the frontend_project subtemplate which is used by https://github.com/plone/plone-frontend ... not sure what should happen there