Coverage json file pattern used when a directory is provided as the input to the formatter

[evanweible-wf]

The coverage:format_coverage requires an --in option to find the coverage json input(s) to be formatted. As currently implemented, the value can be either:

1. A path to a single file
2. A path to a directory within which one or more coverage json files exist

@willdrach-wk is currently working on integrating coverage into the test runner, and in that scenario, a coverage output is created for each test file (and probably one per-environment, e.g. VM or Chrome). In this scenario, using dart-lang/coverage#2 is obviously much more useful because we could point the formatter towards a directory with all of the coverage outputs and receive a single lcov artifact.

Side note: it was recommended to us that we don't try to combine/merge coverage collected on different platforms, like VM and Chrome, because the lines/hitmaps might not line up perfectly. However, at least for VM coverage collection, merging multiple inputs works fine. TBD on Chrome coverage collection, but it seems like we could process the VM inputs and the Chrome inputs separately.

The way dart-lang/coverage#2 is currently implemented, it will only look for files in the given directory that match this pattern:

final filePattern = RegExp(r'^dart-cov-\d+-\d+.json$');

Is there any particular reason this filename pattern was chosen? I ask because it unfortunately limits the readability of the files that the coverage collection step outputs. Will's current branch of the test pkg that collects VM coverage outputs one file per test and adds a .vm.json extension to the filename, which is nice because it's very clear which coverage artifact corresponds to which test, but it doesn't get picked up by this pattern.

I'm not sure if relaxing this pattern would even be acceptable or if that would be viewed as a breaking change – just thought I'd ask.

@cbracken

[willdrach-wk]

For more context: the PR in question is dart-lang/test#1088

If I run the test package in the following way:

pub run test --coverage hello_world test/vm/simple_repo_test.dart

A coverage .json file gets outputted to hello_world/test/vm/simple_repo_test.dart.vm.json, and if there's multiple test suites/files, we get multiple coverage files out of it. It would be awesome to just point to that hello_world directory like Evan suggested.

[evanweible-wf]

Any thoughts on this @cbracken?

[cbracken]

Generally, I'd love for our tools to get out of the business of dealing with the intermediate JSON altogether (it's mostly just historical baggage and should be an implementation detail) -- we should just have APIs that people could implement to aggregate/filter what they care about.

That said, this work is mostly a 20% project of a 20% project at this point, and the integration internally at Google goes to some of the deepest darkest corners of our language infrastructure basement, so doing so may be pretty finicky and I don't have a huge amount of time to dedicate to refactoring to 1.0.0 quality in my current role. @grouma or @vsmenon may have thoughts on the staffing side.

If we don't plan to do any significant cleanup/refactoring of this code, then what you're proposing seems like a reasonable change.

As I say, I'd really like to avoid exposing (or further exposing) the intermediate JSON altogether if we can. Skipping the intermediate JSON and always emitting LCOV would be one approach -- I believe the only hitch there is that the JSON preserves a bit more source URL data than LCOV, making it easier to pretty-print from (plus the formatter already supports emitting to that format).

[cedx]

+1 It's quite annoying to add a rename step before to be able to format the coverage as LCOV.