SE-0501: HTML Coverage Report

[bkhouri (Bassam (Sam) Khouri)]

@owenv

Most of us preferred standardizing on code-coverage over codecov or coverage as the clearer spelling to use going forward.

Maybe I misunderstood, but I got the impression coverage was the favoured spelling over codecov or code-coverage, but I'm fine either way.

Could @build-and-packaging-workgroup members provide their input to ensure it's captured properly

@owenv

• Should we pursue more general support for response files in SwiftPM as proposed by @allevato earlier in the thread?
  • There was some support for this, although concerns were also raised abut response files in general "hiding" arguments from command lines in logs and such, which might be an argument in favor of -Xcov (or both generalized response files and -Xcov)
  • Adding code coverage flags directly to the manifest, or providing them in a toolset.json were also proposed alternatives to introducing a response file with a known name.

It seems like the community is favouring -Xcov instead of a dedicated response file. I will update the proposal to indicate this and will move the response file out of the proposal. I will bring mention to it in the "Future directions" section.

[bkhouri (Bassam (Sam) Khouri)]

The proposal offers the option of specifying generating multiple coverage formats in a single invocation. If we support the -Xcov argument, we need to think of a way support which arguments must be passed down to one format and which to the other.

Take this example

swift test --enable-coverage --coverage-format html --coverage-format json -Xcov --title -Xcov "My title"

The -Xcov arguments are only supported in the HTML format

What do folks think about having the following syntax as a value to -Xcov?

-Xcov [<coverage-format>:]<value>

Here are some example:

• -Xcov html:--title : arguments --title is only sent to the HTML coverage report generation
• -Xcov json:myarg: argument myarg is only sent to the JSON coverage report
• -Xcov commonArg: the argument commonArg is sent to all coverage format reports.

The downside I find is the lack of consistency with the other -Xarg arguments. I'm open to hearing feedback and suggestions.

[bkhouri (Bassam (Sam) Khouri)]

@allevato : Could I get your thoughts on SE-0501: HTML Coverage Report - #22 by bkhouri ?

[allevato (Tony Allevato)]

I think that sounds like an elegant way to deal with multiple formats simultaneously.

The only other options I can think of would be:

• Use flags like -Xcov-html, -Xcov-json, but that forces you to hardcode specific formats in the argument spec, so you can't support new formats without updating SwiftPM. That's worse compared to just peeling off the format name and matching it with the values of --coverage-format.
• Use = instead of :, like some of LLVM/Clang's other multi-part arguments (-fmodule-file Name=file.pcm, -ffile-prefix-map x=y). But in both of those cases, the x=y is defining a mapping from one value to another, which isn't what's happening with these coverage format flags.

I guess the only concern would be, are there any -Xcov arguments that would contain colons that could make parsing the values difficult? Windows file paths come to mind (-Xcov -some-flag=C:/...). That could be solved by only peeling off the first part of the string if it matches one of the known formats, but now you're back to hardcoding that list of formats again...

[bkhouri (Bassam (Sam) Khouri)]

@allevato

I think that sounds like an elegant way to deal with multiple formats simultaneously.

Thanks.

@allevato

• Use flags like -Xcov-html, -Xcov-json, but that forces you to hardcode specific formats in the argument spec, so you can't support new formats without updating SwiftPM. That's worse compared to just peeling off the format name and matching it with the values of --coverage-format.

That's exactly what I was thinking. I'll make sure to update this in the proposal.

@allevato

• Use = instead of :, like some of LLVM/Clang's other multi-part arguments (-fmodule-file Name=file.pcm, -ffile-prefix-map x=y). But in both of those cases, the x=y is defining a mapping from one value to another, which isn't what's happening with these coverage format flags.

Perfect. I'll use = instead of :

@allevato

I guess the only concern would be, are there any -Xcov arguments that would contain colons that could make parsing the values difficult? Windows file paths come to mind (-Xcov -some-flag=C:/...). That could be solved by only peeling off the first part of the string if it matches one of the known formats, but now you're back to hardcoding that list of formats again...

The list of known formats can be retrieved via CoverageFormat.allCases (or a variation thereof). As such, I don't think this would be a problem, but I'm happy to hear your thoughts.

[allevato (Tony Allevato)]

Ah, I figured you were just passing the format directly to llvm-cov without other processing. But if you're already encoding a list of the valid coverage formats elsewhere in SwiftPM, then absolutely, using that as part of the logic would be fine.

[bkhouri (Bassam (Sam) Khouri)]

We currently main a list of supported coverage report as the HTML coverage report leverages llvm-cov show while the JSON coverage report leverages llvm-cov export

[bkhouri (Bassam (Sam) Khouri)]

Hi folks. I posted a PR that update the proposal based on this discussion. Could folks kindly review to ensure capture everyone properly/correctly?

SE-0501: Update proposal base on review

[daveyc123 (David Cummings)]

Thank you all for the wonderful discussion on this thread.

Given the continued feedback and iteration regarding the coverage command line options, we’ll be extending this review until Friday January 30th.

[hi2gage]

+1 for this functionality!

The -Xcov syntax with = delimiter and CoverageFormat.allCases check looks good.

Minor clarification request: llvm-cov options like --output-dir and --project-title use = in their values:

-Xcov html=--output-dir=".coverage"
-Xcov html=--project-title="SwiftPM"

Could an example like this be added to clarify that parsing splits on the first = only? (i.e., format: html, value: --output-dir=".coverage")

[bkhouri (Bassam (Sam) Khouri)]

@hi2gage

Could an example like this be added to clarify that parsing splits on the first = only? (i.e., format: html, value: --output-dir=".coverage")

Good idea. I'll explicitly indicate this.