Feature request: custom coverage reporting

[paullewis]

I've managed to get Jest running via:

load("@npm//:jest/package_json.bzl", jest_bin = "bin")

jest_bin.jest_test(
    name = "src_tests",
    args = [
        "--roots=%s" % native.package_name(),
        "--config=$(rootpath //:copy_jest_config)",
    ],
    data = [
        ":src",
        "//:copy_jest_config",
    ],
)

Due to some quirks of Jest it does require that a config is sent over (hence the target in the data attribute). However, Jest also wants to be the one to generate coverage reports. So in the jest.config.js I have the following:

module.exports = {
  cache: false,
  collectCoverage: true,
  coverageDirectory: process.env.COVERAGE_DIR,
  collectCoverageFrom: undefined,
  coverageReporters: ['text', ['lcovonly', {file: 'lcov.info'}]],
  modulePathIgnorePatterns: [],
  testMatch: ['**/*.test.js'],
  testPathIgnorePatterns: ['/node_modules/'],
};

Which means that Jest is now happily outputting lcov.info to the COVERAGE_DIR. However, the coverage reporter for js_test seems to kick in and overwrite the lcov.info file based on the V8 data (coverage.js, I think?).

Would it be possible to either add an attribute indicating a path to find existing lcov data (as in this case, where I can generate the lcov via Jest into the COVERAGE_DIR directly), or skip the generation of lcov.info if something else has already written the file?

[Note: I edited this description once I figured out it was the lcov.info file I needed to have Jest create.]

[paullewis]

Ah so I've seen that COVERAGE_OUTPUT_FILE is one of the env variables that I could use to configure Jest (or any other lcov-enabled test runner).

So my request becomes either:

1. If something already exists at COVERAGE_OUTPUT_FILE can we skip the lcov generation/compilation in js_test/coverage.js?
2. Can there be an attribute added to js_test that disables lcov generation/compilation entirely?

I feel like 2 might be clearer, but you tell me 😄

[gregmagolan]

Sounds like something we could do. jest_test in rules_jest could get a feature to enable jest coverage. @thesayyn wdyt?

[paullewis]

Thanks! It would be great to have the escape hatch for all js_test rules, just in case something other than Jest is underpinning the test run. (Though adding it to rules_jest is also awesome.)

[thesayyn]

Sounds like something we could do. jest_test in rules_jest could get a feature to enable jest coverage. @thesayyn wdyt?

That one should do for both jasmine and jest rule. However, we should document how to disable the builtin coverage reporter.

[thesayyn]

2. Can there be an attribute added to js_test that disables lcov generation/compilation entirely?

It's already there actually but only available if you are a rule author (aka if you are using js_binary_lib and implementing your own rule). we never intended this to be controllable on a per-target basis.

I believe there should be a switch to disable coverage per target like the stamp attribute.

Relevant line https://github.com/aspect-build/rules_js/blob/8b0bcd8e92c96551d2186d46b0f46076b50bff94/js/private/js_binary.bzl#L437

[gregmagolan]

+1 to adding an easy opt-out to js_test for use cases like this

[thesayyn]

This turned out to be the real challenge. any attribute passed down to js_test is not accessible to _lcov_merger by any way env, custom launcher etc. Bazel spawns the lcov merger as a standalone process so it gets its own env. The only way to disable the built-in lcov merger is to define your own rule that overrides _lcov_merger attribute.

Though we could always halt the built-in coverage tool if some other tool is already written to the coverage file. @paullewis does that sound sensible for your use case?

[paullewis]

Hey, sorry this caused so many headaches! Yes, for the use-case I am looking at an early exit from the built-in coverage tool if it discovers an existing file would be suitable.

[thesayyn]

Cool. I'll make a PR shortly today.