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

fix(node/vm): use file:// URLs for absolute filenames in inspector coverage #28577

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gopoto
Copy link
Contributor

@gopoto gopoto commented Mar 21, 2025

Fixes #27003.

In Vitest and other tools relying on the inspector’s Profiler.takePreciseCoverage,
scripts executed via the Node vm polyfill were showing up with bare absolute
paths (e.g. /tmp/foo.js). Vitest filters out anything that doesn’t start with
file://, so the “v8” coverage provider was reporting zero coverage.

This patch brings the vm polyfill in line with Node by normalising absolute
filenames to file URLs before they’re passed down to V8. Coverage for scripts
created by vm.runInThisContext now report file:/// URLs and are picked up
by coverage tooling.

A new integration test spins up an inspector session within a small vm script
and asserts that a file:///tmp/foo.js script shows up in coverage. All code
and Markdown files were formatted and linted; cargo fmt --check passes.

Changes

  • ext/node/polyfills/vm.js:
    • import node:path and node:url.
    • normalise absolute filenames to file:// URLs before compiling.
  • New test fixture tests/testdata/inspector/vm_coverage.js.
  • New integration test inspector_vm_coverage_file_url in tests/integration/inspector_tests.rs.

Testing

cargo test inspector_vm_coverage_file_url -- --nocapture

Before this change the test would filter out the VM script and coverage would be empty.
After applying the patch, the test passes and the printed JSON contains an entry for file:///tmp/foo.js.

All formatting (cargo fmt, deno fmt) and linting (deno lint) checks pass.


This PR was generated by an AI system in collaboration with maintainers: @dsherret

…verage

Signed-off-by: Gene Parmesan Thomas <201852096+gopoto@users.noreply.github.com>
@dsherret dsherret marked this pull request as ready for review March 21, 2025 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vitest Coverage v8 & Istanbul Providers Broken
1 participant