--coverage should generate a lcov.info file

[rrousselGit]

Currently when running pub run test --coverage or dart test --coverage, only proprietary JSON files are generated.

Most tools use an lcov.info file instead. It would be great if --coverage could generate that file too.

[grouma]

package:coverage can consume these files and output into the lcov format. I believe the command is format_coverage. See the readme: https://github.com/dart-lang/coverage

[rrousselGit]

But why do we need an extra dependency for it when the flag is built-in?

Flutter doesn't require people to install package:coverage.

[grouma]

What's the workflow like with Flutter? Note that Flutter Tools does depend on package:coverage so it is pulled in implicitly. Note also you don't need package:coverage as a dev dependency, it can be used as a global dependency.

[rrousselGit]

What's the workflow like with Flutter

flutter test --coverage generates a lcov.info file directly instead of all of these json files.

Sure people could use coverage. But why should they have to?
The generated json files are useless to most users, so that shouldn't be the default behavior.

Considering that as part of Dart 2.9, we now have an official dart test command, the current behavior looks sketchy.
It'd be logical to have dart test --coverage and flutter test --coverage behave the same

[grouma]

That's a fair argument. I likely won't be able to get to this in the near future but I'm happy to review PRs. We'll likely have to model this as a breaking change.

[mit-mit]

cc @jwren, and a +1 for supporting lcov in dart test

[natebosch]

The flutter test --coverage and pub run test --coverage flags are already quite different. Outputting to a different format by default won't normalize their behavior.

pub run test:

    --coverage=<directory>            Gathers coverage and outputs it to the specified directory.
                                      Implies --debug.

flutter test:

    --coverage                        Whether to collect coverage information.
    --merge-coverage                  Whether to merge coverage data with "coverage/lcov.base.info".
                                      Implies collecting coverage data. (Requires lcov)
    --coverage-path                   Where to store coverage information (if coverage is enabled).
                                      (defaults to "coverage/lcov.info")

I'd be fine with changing package:test options to match flutter test with --coverage and --coverage-path on the next breaking change release. I don't know what --merge-coverage does here, but if it is useful we could add it too.

I'm not sure that any other changes before then move the needle much in terms of making things feel consistent. I also do not think it is a good idea to try to do argument manipulation in dart test to try to normalize things with some hack in package:test.

[rrousselGit]

Would it make sense to make dart test do some extra logic on the top of package:test?
Such that dart pub run test --coverage would be untouched – but dart test --coverage would match Flutter?

This should make the change less breaking (if breaking at all as 2.9 is not stable yet) and not require hacking on package:test.

dart test could internally run the logic to combine both package:test and package:coverage together.

[natebosch]

Would it make sense to make dart test do some extra logic on the top of package:test?

This is what I was referring to when I said "I also do not think it is a good idea to try to do argument manipulation in dart test to try to normalize things with some hack in package:test."

The argument differences in webdev from pub run build_runner already cause us some headaches and I'm leaning towards that being a bad idea. Having 2 commands that mostly do the same thing, and is documented as "uses package:test to run tests" should behave as close as possible. @mit-mit and @jwren might have different opinions on that.

[natebosch]

If we do think its best to try to normalize the flags we should check whether the 3 flags that flutter test have chosen make sense. It is strange that --merge-coverage implicitly enables --coverage, but --coverage-path doesn't.

[mit-mit]

cc @jonahwilliams @zanderso can you have some background on the flutter test flags?

[jonahwilliams]

I've never used merge-coverage or coverage-path. coverage-path might just be for the fuchsia tooling, and merge-coverage requires having the lcov tool available which requires a bit of hacking on windows. IMO it's not really clear how useful it is.