Closed yusufkandemir closed 1 year ago
I've noticed it's not even loading the .nycrc
file, working on it.
I'm pretty sure it used to work fine, but we recently bumped vite-plugin-istanbul
from 2.x to 3.x in latest beta branch, which also bumps Cypress to v11
Could that be the cause? I checked their changelog and couldn't see hints to breaking changes, but an undocumented one could be the source of the problem
Any idea on how we could automate testing that code-coverage is working correctly? I guess we could commit our test projects coverage files and check if any changed via a git diff, or something similar
I've found the problem, it's about vite-plugin-istanbul
:
https://github.com/iFaxity/vite-plugin-istanbul/blob/a8f86218ecf379a605d6e965250b2b0b396a428a/src/index.ts#L60-L71
const nycConfig = loadNycConfig({
cwd,
nycrcPath,
});
// Only instrument when we want to, as we only want instrumentation in test
// By default the plugin is always on
return new TestExclude({
cwd,
include: include ?? nycConfig.include,
exclude: exclude ?? nycConfig.exclude,
extension: extension ?? nycConfig.extension ?? DEFAULT_EXTENSION,
loadNycConfig
is an async function, but it's used as if it's a sync one.
https://github.com/istanbuljs/load-nyc-config/blob/2033a007672d90669c48c79e6a2d63a3cd0851a7/index.js#L139-L164
Reading from .nyrc
have been first introduced in https://github.com/iFaxity/vite-plugin-istanbul/releases/tag/v2.9.0
Will open an issue/PR in their repository. It will need another major release on their side because the plugin initialization will need to be async, e.g. await istanbul(...)
instead of just istanbul()
.
Any idea on how we could automate testing that code-coverage is working correctly? I guess we could commit our test projects coverage files and check if any changed via a git diff, or something similar
Since we don't have any automation yet, that sounds fine to me. In an automated way, we could run the tests and upload coverage artifacts to GitHub actions. Then, every time we run the tests, we can download the previous artifacts and compare them with the newly generated coverage results. In a matrix, we can run these procedures for test-project-app
, test-project-webpack
, and test-project-vite
. This would allow us no test not only the coverage but whether the app-extension is invoked correctly, whether the test commands are working fine, etc. as well.
Also, it's not possible to override extends
until https://github.com/istanbuljs/load-nyc-config/pull/19 gets merged. It doesn't look like it's going to be merged any time soon. So, we should leave a note in our README. If our users want to override the stuff we set up in the nycrc
preset, they would need to remove "extends": "@quasar/quasar-app-extension-testing-e2e-cypress/nyc-config-preset",
embed it's contents, then update it.
Seems like istanbul maintainer isn't maintaining it that much https://github.com/istanbuljs/load-nyc-config/pull/19#issuecomment-1269041701
Here's a workaround to allow overriding properties defined by an extended configuration https://github.com/istanbuljs/nyc/issues/1286#issuecomment-926077635
We can probably pin vite-plugin-istanbul
to 2.8.0 as a workaround, if later versions are faulty
That package not picking up .nycrc
config is why exclude
was manually set into Vite plugin
IIRC another part of the toolchain then picks up the correct configuration at a later stage, but I worked on it more than 8 months ago, so I'm not really sure about this
Logically, "no .nycrc
loading" === "broken .nyrc
loading", so I don't think pinning it back will do any good. As you said, you probably had to pass exclude
to the plugin because it wasn't picking up .nycrc
before. It was picking and is still picking other options like reporter
, so this logic is probably only related to include
, exclude
, and extension
which the Vite plugin also use it for itself.
I've created the PR: https://github.com/iFaxity/vite-plugin-istanbul/pull/53
It doesn't need breaking changes, I previously forgot that you can do the one-time async stuff within the config
hook.
We need to wait until https://github.com/iFaxity/vite-plugin-istanbul/pull/53 and then bump the version. After that, we should be able to merge this.
@yusufkandemir the new version with your fix has been released, can you bump the dependency and try out if everything is now working correctly? I'll then merge and release a new version, possibly the last one before v5 stable version
@IlCallo it's ready
What kind of change does this PR introduce?
Does this PR introduce a breaking change?
The PR fulfills these requirements:
dev
branch and not themaster
branchOther information: When specifying the
exclude
option invite-plugin-istanbul
, it uses that instead of the ones in.nycrc
. The ones specified inexclude
option isn't accurate either. So, this PR updates theexclude
in.nycrc
to cover everything, then removes thevite-plugin-istanbul
'sexclude
option. Now, the excludes are more accurate, and the users will be able to add more excludes in their.nycrc
files as they wish.