aspect-build / aspect-build/rules_js

[Bug]: `js_test` instruments Node for code coverage even if excluded by `--instrumentation_filter`

Open
#1,487 4 comments 0 reactions 0 assignees View on GitHub
bug need: funding
Dominant language
Starlark
Stars
378
Forks
183
Avg merge
1d 9h
Merged PRs (30d)
32

Description

### What happened?

When running tests under `bazel coverage` (or `bazel test --collect_coverage`), Node running via `js_test` is being instrumented regardless of the [--instrument_test_targets](https://bazel.build/reference/command-line-reference#flag--instrument_test_targets) and [--instrumentation_filter](https://bazel.build/reference/command-line-reference#flag--instrumentation_filter) flags.

This is causing an issue for us where we have a Jest test suite that is running out of memory when Node is instrumented (using over 16gb somehow). The `NODE_V8_COVERAGE` environment variable is basically always set when coverage is enabled (https://github.com/aspect-build/rules_js/blob/v1.37.1/js/private/js_binary.sh.tpl#L362), whereas it should really only be set if both coverage is enabled _and_ the target is supposed to be instrumented.

Probably need to use this feature in the `js_test` rule to determine when to instrument: https://bazel.build/rules/lib/builtins/ctx#coverage_instrumented

And this is actually a great use case for excluding Jest targets from instrumentation because Jest is usually set up to create its own coverage report anyway.

### Version

Development (host) and target OS/architectures: Mac, Linux

Output of `bazel --version`: 6.2.0

Version of the Aspect rules, or other relevant rules from your
`WORKSPACE` or `MODULE.bazel` file: rules_js@1.37.1

Language(s) and/or frameworks involved: JS

### How to reproduce

Run `bazel coverage //some/package:some_js_test_target --no_instrument_test_targets`. Node will still be instrumented (via the `NODE_V8_COVERAGE` env var) and a coverage report will be generated.

### Any other information?

Some docs:

* `--instrument_test_targets`: https://bazel.build/reference/command-line-reference#flag--instrument_test_targets

* `--instrumentation_filter`: https://bazel.build/reference/command-line-reference#flag--instrumentation_filter

Contributor guide

Open the contributing guide

Research direction

Start with js/private/js_binary.sh.tpl around line 362 and the js_test rule; inspect how ctx.coverage_instrumented can control NODE_V8_COVERAGE. Reproduce with bazel coverage on a target using --no_instrument_test_targets, then verify that Node is not instrumented and no coverage report is generated for the excluded target.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
build-system, testing
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.