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

Test addon should install coverage provider out of the box #29797

Closed
Tracked by #29531
shilman opened this issue Dec 4, 2024 · 2 comments · Fixed by #29993
Closed
Tracked by #29531

Test addon should install coverage provider out of the box #29797

shilman opened this issue Dec 4, 2024 · 2 comments · Fixed by #29993

Comments

@shilman
Copy link
Member

shilman commented Dec 4, 2024

Currently when you install the test addon, it does not install a coverage provider. When you try to run coverage it fails. To debug you need to look at the console, which is non-intuitive. Then you need to add a coverage provider (it suggests V8) and then restart.

We should just install the coverage provider out of the box. And if we think istanbul is the better choice, we should install that and configure Vitest accordingly. (I'm still on the fence about v8 vs instanbul!).

@ndelangen
Copy link
Member

If we do this, than there's no point in this PR: #29763 right?

@shilman What's the decision here?

  • Do we try to setup with istanbul and only install that?
  • Do we install both into the user's project?
  • Do we auto-install v8, because that's the default?

I have to assume there's a reason why the vitest team decided to make both providers are opt-in to install.

@JReinhold
Copy link
Contributor

Decision:

  • During the addon's postinstall, detect if either @vitest/coverage-v8 or @vitest/coverage-istanbul is installed. If neither of those are, automatically install @vitest/coverage-v8, with a log line explaining why.
  • ... That means we won't try to be smart and detect which provider is potentially configured already, we assume that if a package is installed, it is correctly configured.
  • This can still fail in a few edgy scenarios, which is why we still wan't to move forward with Addon Test: Clarify message when vitest detects missing deps #29763 to ensure a good error message at runtime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants