aspect-build / aspect-build/rules_js

bazel coverage empty for first-party code imported by name through an npm_package

Open
#2,933 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Starlark
Stars
378
Forks
183
Avg merge
1d 9h
Merged PRs (30d)
32

Description

Split off from #2901 / #2932 ([comment](https://github.com/aspect-build/rules_js/pull/2932#issuecomment-5041637627)). The split-postprocessing fix in #2932 does **not** cover this; it is a distinct root cause.

### Symptom

A `js_test` that imports first-party code **by package name**, where the package is provided via `npm_package` + `npm_link_package`, produces an empty (0%) `coverage.dat` — even though the code runs. The same code imported via a `js_library` linked by name, or via a relative `require("./…")`, reports correctly.

### Root cause

The coverage manifest lists the instrumented source path (e.g. `pkg/lib.js`), but `npm_package` **repackages** the sources into the `.aspect_rules_js` store, so node executes — and V8 records — the store realpath:

```
…/runfiles/_main/node_modules/.aspect_rules_js/@repro+lib@0.0.0/node_modules/@repro/lib/lib.js
```

c8 drops it twice over: it is under `node_modules` (excluded by default) and its path shares no token with the manifest entry (`pkg/lib.js`) beyond the basename. A `js_library` avoids this because its store entry is a symlink to the real directory and node (no `--preserve-symlinks`) resolves back to the manifest path. Contrast confirmed from live V8 data:

| Provisioning | V8 URL (relative to c8 `src`) | Result |
| --- | --- | --- |
| relative `require("./lib.js")` | `lib.js` | counted |
| first-party `js_library` | `pkg/lib.js` | counted |
| first-party `npm_package` | `node_modules/.aspect_rules_js/@repro+lib@0.0.0/node_modules/@repro/lib/lib.js` | dropped |

### Why it cannot be fixed in `coverage.js` alone

Attributing the store file to `pkg/lib.js` needs a store→source path mapping, which is not derivable from the paths (package name `@repro/lib` ≠ bazel package `pkg`) and is not present in the test's runfiles (only the repackaged copy is). The fix is rule-level.

### Fix directions (needs a decision on the `SF:` convention)

1. **Report against the (normalized) store/package path** — the rule passes the first-party package-store directories to `coverage.js`, which includes them and overrides the `node_modules` exclusion for exactly those. `SF:` becomes the package path rather than `pkg/lib.js`.
2. **Remap to the source path** — thread a store→source map and add the first-party sources to the coverage inputs so c8 can read them for line/branch structure.

### Repro

`e2e/coverage` (`//:first_party_npmpkg_test`), currently a non-fatal `KNOWN FAILURE` in `e2e/coverage/test.sh` alongside the passing relative and `js_library` cases.

Contributor guide

Open the contributing guide

Research direction

Start with e2e/coverage/test.sh and the //:first_party_npmpkg_test reproduction, then compare its coverage.dat with the relative and js_library cases. Read coverage.js and the rule-level coverage wiring before choosing between the two listed SF: conventions. Done means the npm_package case reports nonzero first-party coverage and no longer remains a KNOWN FAILURE, while the passing cases stay correct.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, node.js
Domain
build-system, testing
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.