Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to me as the review manager by email or DM. When contacting the review manager directly, please put "SE-0501" in the subject line.
Trying it out
If you'd like to try this proposal out, you can download a toolchain supporting for various platforms:
The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:
What is your evaluation of the proposal?
Is the problem being addressed significant enough to warrant a change to Swift?
Does this proposal fit well with the feel and direction of Swift?
If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
More information about the Swift evolution process is available at:
As the proposal author, I wanted to inform the readers that the toolchain listed in the review process contains two arguments for displaying the coverage path, instead of one.
The single --show-coverage-path arguments as described in the proposal, and any feedback from the review, will be incorporated into the final feature.
+1 to the proposal. Having an easier workflow to generate a human-readable, HTML formatted code coverage report for a Swift package seems very useful. An earlier version of this proposal was presented to the Testing Workgroup and I supported the idea at that time too.
I appreciate that the proposal includes modifications to the --show-coverage-path flag to facilitate programmatically querying the location of the generated report, even when more than one format is requested.
The optional response file will be located in <repo>/.swiftpm/configuration/coverage.html.report.args.txt . The response file will be supported.
In that response file name (coverage.html.report.args.txt), will the html component be substituted to json if the requested format is JSON? Mostly just curious if that's intended to be a file name pattern, or whether that is the sole supported response file name, currently.
The response file will only be supported for the HTML report. But we can certainly modify the proposal to support a response file for the JSON coverage report.
Also, I'm happy to use a different response file name if folks have other suggestions.
Overall this proposal looks sensible to me and it makes sense to make HTML coverage output built-in. The only thing that confused me a bit is why we need to define three different ways to pass the argument instead of choosing one:
--codecov-format, --code-coverage-format, --coverage-format
Format of the code coverage output. Can be specified multiple times. (default: json)
json - Produces a JSON coverage report.
html - Produces an HTML report produced by llvm-cov.
Great observation @FranzBusch . Currently, there are three different ways to print the coverage report location
--show-codecov-path, --show-code-coverage-path, --show-coverage-path
Print the path of the exported code coverage JSON file.
I order for consistency, it was thought to support the three styles in all coverage options. This also removes cognitive overload trying to remember "is is --coverage-format or --code-coverage-format.
The implementation also updates the --enable-code-coverage/--disable-code-coverage to support all three styles:
Interesting. I am wondering if we would like to go the opposite path here and reduce even the existing ones down into one per argument over time. There are certainly ways how we can phase them out graciously without breaking users right away. I personally find it very confusing if there are multiple different "long form" ways to spell an argument.
Thanks for pointing this out. It wasn't entirely clear from the proposal that it also proposes to add these. It currently makes it sounds like they already exist.
That's a very good question. I think this is something that we can ask the @build-and-packaging-workgroup to discuss since we are lacking an overall "style guide" for CLI commands and arguments. Doesn't mean that we should block this or any other proposal on having such a style guide defined since we can always align them in the future.
I tend to want to have repeatable and reproducible builds. Adding a configurable path to the response file could cause behavioural differences in various systems, leading one to wonder "Why is my report different than the one generated in CI? ".
With that said, if the community wants a configurable response file, I will be more than happy to include it.
In general, we should not hard-code paths to asset files as developers' repos may not be configured the way we expect or the way we'd configure our own repos.
SwiftPM stores mirrors and registration information in the repository root under .swiftpm/configuration/. The response file is using the same configuration path used by other SwiftPM features.
I agree with this 1000% and would take it even further: the design shouldn't require the use of a response file at all.
This is emblematic of a larger problem: SwiftPM itself should take response files, but it doesn'tâAFAICT, something like swift build @args.txt fails with error: Unexpected argument '@args.txt'. It should load and parse that file for args, whether they're related to coverage or anything else.
Then, we already have a pattern that we use to pass arguments to sub-tools: -X* prefix flags like -Xswiftc, -Xlinker, etc. We could add -Xcov to say which flags should be passed to llvm-cov.
Then, combining those two features, people have more flexibilityâthey can just pass -Xcov -whatever on the command line if they want, or they can put those specific flags into a response file, or they can put any combination of flags they want in a response file. This is the design that the compiler uses (and is a common practice for many compilers and build systems these days), so we should reproduce that here.
If folks don't want to block this proposal on generalized SwiftPM support for response files, I think that's fineâwe should just do the -Xcov approach now and have SwiftPM add response file support later. It doesn't feel right to "solve" command line explosion for one specific niche feature.
Hi @allevato , thank you very much for your feedback.
The design does not require usage of response file. Enabling HTML coverage report will provide a HTML report. However, customizing the HTML report will likely require usage of the response file in order to provide a repeatable and reproducible HTML repo across the repository.
First, this is outside the scope of this proposal. Also, for clarity, the response file in this proposal only related to when SwiftPM invokes the underlying llvm-cov command to generate the HTML report.
In addition to the response file, the coverage report generation can support a command line argument similar to -Xlinker, -Xcc and others, which will pass the arguments to llvm-cov show and override the values in the response file. This has not been implemented in the PR.
I don't agree with this until we can encode the llvm-cov arguments in the repository itself so we can get a repeatable HTML coverage report. In addition, since llvm-cov has many subcommands, how would we handle the case where we demand JSON and HTML report, but the associated llvm-cov subcommand does not support all -Xcov arguments passed via swift test.
The intent is that when demand an HTML coverage report, the same HTML report is produced for all users. Granted, this does not prevent a user from modifying the response file arguments and create a "temporary HTML" report, but the repository will be the source of truth for these HTML report command options.
We discussed this proposal with @bkhouri in the build and packaging workgroup meeting today. Overall, the consensus around the feature was that it was well-motivated and would make a good addition to SwiftPM's feature set. Most of the feedback and discussion was around the design of the command line interface.
Spelling of options
Current code coverage options have 3 variants: --show-codecov-path, --show-code-coverage-path, and --show-coverage-path. The general consensus from the workgroup was that having multiple synonyms for the same functionality is inconsistent with most of SwiftPM's other functionality and would be best avoided when introducing the new options. Most of us preferred standardizing on code-coverage over codecov or coverage as the clearer spelling to use going forward.
Response files vs alternatives
We spent quite a bit of time discussing various ways of providing custom flags to the code coverage tool when generating a report, I've done my best to accurately summarize the main points of the discussion below:
Users may want to check in a standardized report configuration which can, for example, be easily shared between CI and desktop builds without passing many additional command line flags to swift build. This is an argument in favor of not providing only an -Xcov option.
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.
(This is my personal review, not the opinion of the build and packaging workgroup)
What is your evaluation of the proposal?
+1. I think this is a great test tooling improvement that makes will make it easier for users to integrate test coverage in their CI and desktop dev workflows.
Does this proposal fit well with the feel and direction of Swift?
I think code coverage report generation fits well with the rest of SwiftPM's feature set and is an important part of providing a great set of testing functionality.
My main concern is around the proposed response file used to provide a default set of args to the code coverage tool. While I understand the desire to allow packages to ship a default set of report generation flags which work "out of the box", personally I don't think it justifies a bespoke response file checked in at a known location. I think I'd prefer to both add generalized response file support to SwiftPM, and an -Xcov flag. This would still allow users to check in a shared response file if they want (and use it with e.g. swift test @code-coverage-report-args.txt), but it would do so without introducing a new special case. At the same time, it would generalize to other types of shared configuration unrelated to code coverage.
If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
Some experience using code coverage tools with other languages/IDEs
How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
