aspect-build / aspect-build/rules_js

[Bug]: js_test + split coverage with custom aggregator won't pick up lcov files

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

Description

### What happened?

I'm currently using `rules_js` to invoke `Playwright`. In doing so, I want to collect coverage from my tests and output them to `COVERAGE_DIR`. However, as cited by https://github.com/aspect-build/rules_js/issues/965 there can be issues when using `--experimental_split_coverage_postprocessing` flag. What I have found is that when writing to `COVERAGE_DIR`, the post processing done by `merge.sh` in `rules_js` won't find the files I'm writing and therefore will generate an empty coverage report. To get around this, I had to make a modification to the `coverage.js` bundle with the following. This hard codes the assumption that I will produce a specially named file `split_coverage.dat` which the coverage collector will find and move to the proper place. Once I did this, I was able to read the coverage I generated under the `playwright_test` target via `c8`.

```diff
+// If we are running in split coverage mode, we need to move the files
+// from COVERAGE_DIR to COVERAGE_OUTPUT_FILE
+// Since we patched this ourselves, we are using a unique filename
+// that is deterministic so we can leverage this logic
+if (process.env.SPLIT_COVERAGE_POST_PROCESSING == "1" && require$$0$1.existsSync(require$$0.join(process.env.COVERAGE_DIR, 'split_coverage.dat'))) {
+ const stats = require$$0$1.statSync(require$$0.join(process.env.COVERAGE_DIR, 'split_coverage.dat'));
+ if (stats.size != 0) {
+ require$$0$1.copyFileSync(require$$0.join(process.env.COVERAGE_DIR, 'split_coverage.dat'), process.env.COVERAGE_OUTPUT_FILE)
+ // early exit here does not affect the outcome of the tests.
+ // bazel will only execute _lcov_merger when tests pass.
+ process.exit(0);
+ }
+}
```

One alternative solution might be to allow a user to pass their own lcov merge logic instead being bound by the one that rules_js makes us use since it's not overridable.

CC @joeljeske

### Version

Development (host) and target OS/architectures:

Output of `bazel --version`: 7.1.1

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

Language(s) and/or frameworks involved: rules_js, playwright
1.41.0 of rules_js
1.44 of playwright

### How to reproduce

_No response_

### Any other information?

_No response_

Contributor guide

Open the contributing guide

Research direction

Start by examining the split-coverage handling in merge.sh and the coverage.js bundle, then review the related rules_js issue 965. Reproduce the Playwright coverage flow with Bazel and verify that lcov files written under COVERAGE_DIR are discovered by post-processing and produce a non-empty report.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, playwright
Domain
build-system, testing-qa
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.