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

coverage-istanbul does not work with workspaces and SvelteKit #5856

Closed
6 tasks done
Ernxst opened this issue Jun 7, 2024 · 4 comments · Fixed by #6044
Closed
6 tasks done

coverage-istanbul does not work with workspaces and SvelteKit #5856

Ernxst opened this issue Jun 7, 2024 · 4 comments · Fixed by #6044
Labels
feat: coverage Issues and PRs related to the coverage feature p3-minor-bug An edge case that only affects very specific usage (priority)

Comments

@Ernxst
Copy link

Ernxst commented Jun 7, 2024

Describe the bug

In a monorepo with a simple SvelteKit app, running coverage with @vitest/coverage-istanbul does not work. Tests pass but collecting coverage fails with the following error:

SyntaxError: /Users/ernest/Projects/Repros/vitest-workspaces-istanbul/packages/web/src/routes/+page.svelte: Support for the experimental syntax 'jsx' isn't currently enabled (1:1):

> 1 | <h1>Welcome to SvelteKit</h1>
    | ^
  2 | <p>Visit <a href="https://kit.svelte.dev">kit.svelte.dev</a> to read the documentation</p>
  3 |

Add @babel/preset-react (https://github.com/babel/babel/tree/main/packages/babel-preset-react) to the 'presets' section of your Babel config to enable transformation.
If you want to leave it as-is, add @babel/plugin-syntax-jsx (https://github.com/babel/babel/tree/main/packages/babel-plugin-syntax-jsx) to the 'plugins' section to enable parsing.

If you already added the plugin for this syntax to your config, it's possible that your config isn't being loaded.
You can re-run Babel with the BABEL_SHOW_CONFIG_FOR environment variable to show the loaded configuration:
        npx cross-env BABEL_SHOW_CONFIG_FOR=/Users/ernest/Projects/Repros/vitest-workspaces-istanbul/packages/web/src/routes/+page.svelte <your build command>
See https://babeljs.io/docs/configuration#print-effective-configs for more info.

1  |  <h1>Welcome to SvelteKit</h1>
   |  ^
2  |  <p>Visit <a href="https://kit.svelte.dev">kit.svelte.dev</a> to read the documentation</p>
3  |  

Works fine with @vitest/coverage-v8 but I need istanbul (required by @cloudflare/vitest-pool/workers)

Reproduction

  1. Clone repo
  2. Install dependencies (bun install)
  3. Run test script (bun run test) - everything passes
  4. Run coverage script (bun run coverage)

System Info

System:
    OS: macOS 14.5
    CPU: (8) arm64 Apple M1
    Memory: 40.45 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.12.2 - ~/Library/Caches/fnm_multishells/86163_1717766938803/bin/node
    npm: 10.5.0 - ~/Library/Caches/fnm_multishells/86163_1717766938803/bin/npm
    pnpm: 8.9.0 - ~/Library/pnpm/pnpm
    bun: 1.1.12 - ~/.bun/bin/bun
  Browsers:
    Chrome: 125.0.6422.77
    Edge: 125.0.2535.92
    Safari: 17.5
  npmPackages:
    @vitest/coverage-istanbul: 1.5.0 => 1.5.0 
    vitest: 1.5.0 => 1.5.0

Used Package Manager

npm

Validations

@AriPerkkio
Copy link
Member

As work-around for now you can set coverage.all: false or configure Svelte plugin in the root vitest.config.ts.

I think this is caused by the root vitest.config.ts not having Svelte Vite plugin, and it fails when transforming uncovered Svelte files here:

await this.ctx.vitenode.transformRequest(`${filename}?v=${cacheKey}`)

@Ernxst
Copy link
Author

Ernxst commented Jun 9, 2024

I'd rather have coverage for all my source files so I went with adding the Svelte plugin to the root vitest.config.ts. It worked in the repro but in my actual project it doesn't - I have an api package which uses @cloudflare/vitest-pool-workers and the api tests fail with:

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Unhandled Errors ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

Vitest caught 1 unhandled error during the test run.
This might cause false positive tests. Resolve unhandled errors to make sure your tests are not affected.

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Unhandled Error ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
TypeError: Cannot use require() to import an ES Module.
 ❯ node_modules/chai/lib/chai/utils/inspect.js node_modules/chai/index.mjs%3Fmf_vitest_no_cjs_esm_shim:217:17
 ❯ __require node_modules/chai/index.mjs%3Fmf_vitest_no_cjs_esm_shim:8:50
 ❯ node_modules/chai/lib/chai/utils/objDisplay.js node_modules/chai/index.mjs%3Fmf_vitest_no_cjs_esm_shim:235:19
 ❯ __require node_modules/chai/index.mjs%3Fmf_vitest_no_cjs_esm_shim:8:50
 ❯ node_modules/chai/lib/chai/utils/getMessage.js node_modules/chai/index.mjs%3Fmf_vitest_no_cjs_esm_shim:262:22
 ❯ __require node_modules/chai/index.mjs%3Fmf_vitest_no_cjs_esm_shim:8:50
 ❯ node_modules/chai/lib/chai/utils/index.js node_modules/chai/index.mjs%3Fmf_vitest_no_cjs_esm_shim:749:26
 ❯ __require node_modules/chai/index.mjs%3Fmf_vitest_no_cjs_esm_shim:8:50
 ❯ node_modules/chai/lib/chai.js node_modules/chai/index.mjs%3Fmf_vitest_no_cjs_esm_shim:2725:17
 ❯ __require node_modules/chai/index.mjs%3Fmf_vitest_no_cjs_esm_shim:8:50

When running vitest from the monorepo root - the tests pass when I run it from the packages/api directory. I created a feature/api branch in the repro and found the same issue there with a minimal setup. After some investigation, it seems that the loupe package chai imports has invalid ESM exports (see this issue and the PR). Removing the module field in its package.json fixes everything so I'm not sure whether this is a vitest issue, a chai issue, a loupe issue or a vitest-pool-workers issue as everything works perfectly when I don't add the Svelte plugin to the root vitest.config.ts.

@Ernxst
Copy link
Author

Ernxst commented Jun 9, 2024

After some more testing, it seems adding the plugin to the root breaks anything that doesn't have exactly the correct package exports. For example, I have a package using unplugin-icons and running coverage on it now fails with:

Error: No known conditions for "./types/svelte" specifier in "unplugin-icons" package

Looking in the package.json of the module, I see it has the following export:

"./types/svelte": {
  "types": "./types/svelte.d.ts"
}

With no import condition which sounds reasonable since it's for types only. However, it breaks because of this so the fix is to add "import": "./types/svelte.d.ts". Next, I have a package which uses msw and coverage now fails with:

Error: No known conditions for "./browser" specifier in "msw" package

Which has the following package.json export:

 "./browser": {
    "types": "./lib/browser/index.d.ts",
    "browser": {
      "require": "./lib/browser/index.js",
      "import": "./lib/browser/index.mjs"
    },
    "node": null,
    "require": "./lib/browser/index.js",
    "import": "./lib/browser/index.mjs",
    "default": "./lib/browser/index.js"
}

And this is fixed by simply moving the import condition up, above the browser condition. I doubt patching every failing package.json is the solution here

AriPerkkio added a commit to AriPerkkio/vitest that referenced this issue Jun 9, 2024
@AriPerkkio AriPerkkio added feat: coverage Issues and PRs related to the coverage feature p3-minor-bug An edge case that only affects very specific usage (priority) and removed pending triage labels Jun 9, 2024
@sheremet-va
Copy link
Member

I just want to add that this is documented: https://vitest.dev/guide/workspace.html#coverage

AriPerkkio added a commit to AriPerkkio/vitest that referenced this issue Jul 6, 2024
AriPerkkio added a commit to AriPerkkio/vitest that referenced this issue Jul 7, 2024
AriPerkkio added a commit to AriPerkkio/vitest that referenced this issue Jul 7, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Sep 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feat: coverage Issues and PRs related to the coverage feature p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants