Add code coverage

[remcohaszing]

No description provided.

[netlify]

✅ Deploy Preview for monaco-yaml ready!

Name	Link
🔨 Latest commit	d106465
🔍 Latest deploy log	https://app.netlify.com/sites/monaco-yaml/deploys/67aa1756e851290008948ce0
😎 Deploy Preview	https://deploy-preview-263--monaco-yaml.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[AriPerkkio]

Looks like monaco-editor is running your code on Worker. Is it possible to force it load in main thread instead?

• Add flag to Disable Worker microsoft/monaco-editor#380
• [Bug] How to disable worker loading microsoft/monaco-editor#4795

Adding following raises a warning from monaco but it still runs into some errors:

window.MonacoEnvironment ||= {}
window.MonacoEnvironment.getWorker = undefined

> Could not create web worker(s). Falling back to loading web worker code in main thread, which might cause UI freezes. Please see https://github.com/microsoft/monaco-editor#faq
> Error: Cannot read properties of undefined (reading 'toUrl')

Vitest's browser mode is unable to collect coverage from Workers. This is also not possible in Playwright - how did you do this before using Vitest? It should be possible to add support for this in Vitest, we just haven't implemented it yet.

• @vitest/web-worker error when using browser environment vitest-dev/vitest#4899
• [Feature] CDPSessions for Web Workers microsoft/playwright#22992

Maybe using packages like https://www.npmjs.com/package/pseudo-worker could work here?

[remcohaszing]

Thanks! It is possible to run builtin workers on the main thread, but not monaco-yaml. I don’t know why this is. However, running the builtin workers on the main thread makes the entire UI sluggish. IMO developers should’t trade that UX for a slight developer convenience. I understand this might be useful for code coverage in tests for now, but this isn’t what I intend to focus on.

Code coverage isn’t working for code outside workers either. This may have to do with the fact that I’m testing build output, but I’m interested in coverage for source code. It looks like Vitest doesn’t use the source map to map coverage of the build output back to the original source code. This is my main goal. Improving coverage to include web worker support is a nice to have follow-up.

I believe the tests should be as close to the user as possible. This means testing the code that’s published, not the code that’s authored. This is especially true for monaco-yaml, as it has poorly packaged dependencies, custom module mappings, and stubs.

Before Vitest with Playwright I didn’t have code coverage either. Having code coverage at all would be a great improvement, even if it doesn’t work for web workers yet.

[AriPerkkio]

It looks like Vitest doesn’t use the source map to map coverage of the build output back to the original source code.

It does use. You'll need to add the built files in coverage.includes so that they won't get excluded before remapping happens - exactly as in nyc and c8. Many projects are running transpiled code with Vitest and getting their coverage reports pointing back to source files.

[remcohaszing]

Ooh I see! It works, thanks! So I need to:

• Include source files to tell Vitest to display coverage for those
• Include build output to tell Vitest to gather coverage for those
• Exclude node_modules to tell Vitest not to display coverage for bundled dependencies
• Enable excludeAfterRemap to tell vitest to apply the exclude pattern after mapping back to the source files.

The coverage is wrong though. It’s pretty easy to spot, because it reports lines 429-436 of src/index.ts are uncovered, but that file only has 432 lines.

[AriPerkkio]

Ooh I see! It works, thanks! So I need to:

Yes, exactly. This should be 100% same workflow as you've needed previously with other testing tools. 💯

The coverage is wrong though. It’s pretty easy to spot, because it reports lines 429-436 of src/index.ts are uncovered, but that file only has 432 lines.

Is that just locally? On CI it's showing different results.

[remcohaszing]

Ooh I see! It works, thanks! So I need to:

Yes, exactly. This should be 100% same workflow as you've needed previously with other testing tools. 💯

It feels a bit unintuitive that coverage.include means two things. Of course this makes perfect sense if you don’t test source-mapped build output, which I imagine is the majority of use cases. Anyway, I’m really happy it works! I feel like I can now confidently start adding more tests. Thanks for your help!

The coverage is wrong though. It’s pretty easy to spot, because it reports lines 429-436 of src/index.ts are uncovered, but that file only has 432 lines.

Is that just locally? On CI it's showing different results.

This was indeed locally. I removed and reinstalled node_modules. Now everything works perfectly. It must have been a caching issue.

---

These warnings do concern me though:

The missing source files are probably due to badly packaged dependencies. But the optimized dependencies look like they might be bad for reasons I don’t understand. It seems to point to all runtime dependencies I use. What does the warning mean? Can I tell Vitest to optimize all deps?

[AriPerkkio]

Those dependencies should be added in the config. It's not ideal but as far as I know, there are no better solutions at the moment.

I'm not sure what tool is causing those source map warnings. 🤔
You could maybe try removing third party modules from source maps and see if it goes away. Maybe some dependency is not shipping their source maps in the package?

[remcohaszing]

I'm not sure what tool is causing those source map warnings. 🤔

I’m not sure what tools, but the violating package is yaml-language-server. It ships source maps, but not source code. It’s annoying, but not really a problem. :)