aspect-build / aspect-build/rules_js
[Bug]: `js_test` instruments Node for code coverage even if excluded by `--instrumentation_filter`
- 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
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