SwiftPM: HTML Coverage Report #2976
Conversation
| The command line option will be defined as | ||
|
|
||
| ```swift | ||
| @Option( |
There was a problem hiding this comment.
You don't need to include the source code you're adding to SwiftPM here. Just describe the command-line argument in terms of what the user will deal with (e.g. show the console output for it when you write swift test --help).
|
|
||
| ### Coverage Report configuration | ||
|
|
||
| `llvm-cov show` has several report configurability options. In order to |
There was a problem hiding this comment.
I don't think this paragraph describes a response file per se, just a configuration file. A response file would be a file whose contents are injected into the command line string, which this ain't, right?
There was a problem hiding this comment.
The HTML report configuration is achieve via a response file. I'll clarify this.
| `llvm-cov show` has several report configurability options. In order to | ||
| prevent a "command line arguments" explosion to `swift test`, a response file | ||
| will be supported. If the response file is found in a location relaltive to the | ||
| Swift package, namely in `<repo>/.swiftpm/configuration/coverage.html.report.args.txt`, |
There was a problem hiding this comment.
Be prepared to bikeshed this name: I'd offer something in the Alternatives considered section indicating other names you thought about using and why you went with this one.
There was a problem hiding this comment.
I'm not keen on the exact file name coverage.html.report.args.txt. I'll leave this up to the community. However, the .swiftpm/configuration path would likely remain as it's already used by SwiftPM as it's where the mirror.json configuration is located.
| Swift package, namely in `<repo>/.swiftpm/configuration/coverage.html.report.args.txt`, | ||
| the contents of the file will be used. | ||
|
|
||
| The only `llvm-cov show` command-line argument SwiftPM will enforce is |
There was a problem hiding this comment.
What does this paragraph mean?
There was a problem hiding this comment.
if the response file has --format json, it will be "ignored" as SwiftPM will inject "--format=html" after there response file.
llvm-cov show @<responsefile> --format=html
|
|
||
| ### Coverage report location | ||
|
|
||
| By default, the HTML report will be created in location under the Scratch Path. |
There was a problem hiding this comment.
What's the Scratch Path? Is that just an implementation detail of SwiftPM? If it's the .swiftpm directory, should you consider defaulting to the temporary directory instead (or at least discussing why you didn't use it?) Or another directory?
There was a problem hiding this comment.
The scratch-path is essentially the .build directory. The developer can override the default location via the --scratch-path command line argument
--scratch-path <scratch-path>
Specify a custom scratch directory path. (default .build)
There was a problem hiding this comment.
Might be good to include this explanation since it's not a term people might be using frequently.
| a single time (or is not specified at all), there is no change to the output. | ||
|
|
||
|
|
||
| if `--coverage-format` is specified multiple times, the output must reflect this. |
There was a problem hiding this comment.
I think we can simplify this behaviour to just something like:
If
--coverage-formatis specified multiple times, the output is one path per line in the order the options were specified. For example, if the developer passes--coverage-format html --coverage-format json, the output would be:[...]/path1.html [...]/path1.json
Thoughts?
There was a problem hiding this comment.
I don't have any real preference, though it might be visually more appealing to include the key. I'm happy to let the community weigh in here :)
|
|
||
| ## Security | ||
|
|
||
| There are no security implication. |
There was a problem hiding this comment.
| There are no security implication. | |
| There are no security implications. |
You sure? Could there be an exploitable bug in llvm-cov --format=html? HTML itself is a whole new attack surface, no?
bkhouri
force-pushed
the
t/main/swiftpm_html_coverage_report_proposal
branch
from
a5d633d to
5ee8fff
Compare
No description provided.