From d9eda7eb849adc29dbda1848c3387afdf6480c56 Mon Sep 17 00:00:00 2001 From: ridwanabdillahi <91507758+ridwanabdillahi@users.noreply.github.com> Date: Fri, 3 Jun 2022 13:22:50 -0700 Subject: [PATCH 1/4] Write up an RFC for integrating native Rust support for code coverage into Cargo. --- text/0000-cargo-test-coverage.md | 249 +++++++++++++++++++++++++++++++ 1 file changed, 249 insertions(+) create mode 100644 text/0000-cargo-test-coverage.md diff --git a/text/0000-cargo-test-coverage.md b/text/0000-cargo-test-coverage.md new file mode 100644 index 00000000000..ba80b2c1a2e --- /dev/null +++ b/text/0000-cargo-test-coverage.md @@ -0,0 +1,249 @@ +- Feature Name: `cargo_test_coverage` +- Start Date: 2022-06-01 +- RFC PR: [rust-lang/rfcs#0000](https://github.com/rust-lang/rfcs/pull/0000) +- Rust Issue: [rust-lang/rust#0000](https://github.com/rust-lang/rust/issues/0000) + +# Summary +[summary]: #summary + +This RFC aims to improve the process of collecting code coverage data for +Rust libraries. By including Cargo in the process of instrumenting Rust +libraries and running the unit tests, the sequence of steps to get coverage +results will be simplified. This RFC also proposes adding support for cargo +to selectivly choose which crates get instrumented for gathering coverage results. + +# Motivation +[motivation]: #motivation + +Why are we doing this? What use cases does it support? What is the expected outcome? + +The motivation behind this feature is to allow for a simple way for a Rust developer +to run and obtain code coverage results for a specific set of crates to ensure confidence +in code quality and correctness. Currently, in order to get instrumentation based code coverage, +a Rust developer would have to either update the `RUSTFLAGS` environment variable or cargo +manifest keys with `-C instrument-coverage`. This would automatically enable instrumentation +of all Rust crates within the dependency graph, not just the top level crate. Instrumenting +all crates including transitive dependencies does not help the developer ensure test coverage +for the crates they actually want to test. This also adds unnecessary work for both the Rust +compiler and codegen backend, `LLVM` in this case, to instrument all libraries as opposed to +the subset a Rust developer actually cares about. + +# Guide-level explanation +[guide-level-explanation]: #guide-level-explanation + +This section examines the features proposed by this RFC: + +## CLI option + +A new flag for the `cargo test` command would be added. The new flag `--coverage` would instruct Cargo to +add the Rust compiler flag `-C instrument-coverage`, for the given crate only. This would mean that only the top-level +crate would be instrumented and code coverage results would only run against this crate. As an example, lets take the +following crate `foo`: + +```text +/Cargo.toml + +-- src + +-- lib.rs +``` + +Where crate `foo` has a dependency on the `regex`: +```toml +[dependencies] +regex = "*" +``` + +And `lib.rs` contains: + +```rust +use regex::*; + +pub fn match_regex(re: Regex, text: &str) -> bool { + let Some(caps) = re.captures(text) else { + return false + }; + + let matches = caps.iter().filter_map(|s| s).collect::>(); + matches.len() > 0 +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn find_match() { + let result = match_regex(Regex::new(".*").unwrap(), "Hello World"); + assert_eq!(result, true); + } + + #[test] + fn find_no_match() { + let result = match_regex(Regex::new("a+").unwrap(), "Hello World"); + assert_eq!(result, false); + } +} +``` + +Now running `cargo test --coverage` would build the `foo` crate and instrument all libraries created by this crate. +Next, the tests for the crate `foo` will be run and produce code coverage results for this crate only and ignore +all code coverage for any function defined outside of this crate. + +The `cargo test` subcommand also supports the `--package` and `--workspace` flags. This instructs cargo to run the +tests for those specific packages. When combined with the new `--coverage` flag, a Rust developer will be able to +selectively choose which Rust crates will be instrumented and have tests run. Since potentially unit tests from +multiple crates will be run, the code coverage results will include coverage results for a crate that is being +covered by a test from another crate. + +# Reference-level explanation +[reference-level-explanation]: #reference-level-explanation + +As mentioned earlier this RFC proposes adding a new flag to the `cargo test` command. This flag, `--coverage` +would be responsible for setting the `-C instrument-coverage` flag that Cargo would pass on to the rustc invocation of the +top-level crate. In the previous example, `foo` would be the top level crate and `regex` would be upstream an dependency. + +Using the `--coverage` flag, Cargo would only set the `-C instrument-coverage` flag for the crate `foo`. If the `RUSTFLAGS` +environment variable had already been set to include the `-C instrument-coverage` flag, then Cargo would still pass that +flag to all crates within the dependency graph, including the `regex` crate and any transitive dependencies. + +This should not break any existing workflows and is strictly an opt-in feature. + +To use this new feature do the following: + +`cargo test --coverage` + +This flag would also be responsible for setting the `LLVM_PROFILE_FILE` environment variable which is read by LLVM +to generate a `.profraw` file for each test executable that is run. Once again if the environment variable is already set, +then Cargo would not make any changes and would leave the value as is to use the user-defined file name. If the environment +variable is not set, Cargo would set it to ensure a unique naming scheme is used for each `.profraw` file that would be +generated. + +Once the tests have finished running, cargo would leverage LLVM tooling to analyze profraw files and generate +coverage reports for a Rust developer. These tools would be `llvm-profdata` and `llvm-cov` and can be used by +adding the `llvm-tools-preview` component to the current toolchain, `rustup component add llvm-tools-preview`. +This would be a requirement to use this new feature since these are the tools necessary to analyze coverage results +and generate a code coverage report. + +These updates to Cargo would be sufficient enough to ensure that a Rust developer would have control over what crates are +instrumented and code coverage results are generated. This would also allow the Rust developer to no longer have to +set environment variables manually to ensure crates are instrumented for gathering coverage data. + +# Drawbacks +[drawbacks]: #drawbacks + +A drawback of this feature would be that Cargo would need to enable the `LLVM_PROFILE_FILE` +environment variable in order to ensure unique profile data is generated for each test +executable. I am not aware of any other Cargo features that set environment variables today +so this could potentially create new issues in Cargo. + +# Rationale and alternatives +[rationale-and-alternatives]: #rationale-and-alternatives + +## Rationale + +This design provides a simple mechanism to integrate collecting code coverage +results for a given crate. Allowing Cargo to be part of the coverage process +would reduce the need for setting environment variables manually. Simply running +`cargo test --coverage` would automatically run a build setting the `-C instrument-coverage` +Rust flag only for the top level crate and set the `LLVM_PROFILE_FILE` environment +variable to ensure each test run produces a unique `profraw` file. Once the tests have +finished running, cargo would leverage LLVM tooling to analyze profraw files and generate +coverage reports for a Rust developer. + +This design does not break any existing usage of Rust or Cargo. This new feature would +be strictly opt-in. A Rust developer would still be able to manually set the +`-C instrument-coverage` Rust flag and instrument all binaries within the dependency +graph. Since this is a Cargo specific feature, the Rust compiler will not need any updates. + +## Alternatives + +### Alternative 1: leave the existing feature + +Supporting this alternative would mean that no changes would be necessary to either Cargo +or Rust. Getting instrumentation-based code coverge is still supported and would continue +to work as it does today. + +The drawback for this option is that it would require setting the flag for all crates in +the dependency graph, including upstream dependencies. This would also instrument all +binaries and report on coverage for functions and branches that are not defined by the +current crate with the potential of skewing coverage results. + +### Alternative 2: use a new manifest key instead of a cli option + +Supporting this alternative would mean that changes would need to be made to the existing +manifest format to possibly include a new section and/or key. A new `coverage` key could +be added to the target section, `coverage = true`. This still has the added benefit of not +requiring any changes to the Rust compiler itself and the feature could be scoped to Cargo only. + +The drawback for this option is that it could potentially add clutter to the `Cargo.toml` +files. Adding this new section to every crate that a Rust developer wants to have instrumented +would only add to all the data that is already contained within a toml file. + +### Alternative 3: use a RUSTC_WRAPPER program to selectively choose which crates to profile + +Supporting this alternative would mean that there wouldn't need to be any changes to Cargo +at all. + +This would require creating a RUSTC_WRAPPER program specifically for selecting which crates to profile. +This means more boiler plate code for each Rust developer that simply wants to profile their own crate. +I believe the feature this RFC proposes would both be a cleaner solution long term and more in line +with the Cargo workflow of potentially reading these kinds of behaviors from the `Cargo.toml` manifest +file. + +### Alternative 4: use existing custom subcommands to run code coverage analysis + +Supporting this alternative would mean that there wouldn't need to be any changes to Cargo +at all. + +There are multiple custom subcommands that exist today to achieve instrumenting Rust libraries +analyzing code coverage metrics. A couple of examples would be cargo-llvm-cov and cargo-tarpaulin. +Both of these custom subcommands are available via crates.io and support running tests with code +coverage enabled and creating a coverage report to be viewed by a Rust developer. + +Cargo-llvm-cov currently leverages the `cargo-config` subcommand which is still unstable. To do so, +cargo-llvm-cov sets the `RUSTC_BOOTSTRAP` environment variable to allow its usage from a stable +toolchain. This is not a recommended approach especially for Rust developers that want to use +such tools in production code. + +Tarpaulin has a great set of features for collecting and analyzing coverage metrics but it only +supports x86_64 processors running Linux which limits how this can be used by Rust developers +working on other platforms such as Windows. + +# Prior art +[prior-art]: #prior-art + +## VSInstr + +Visual Studio ships with a tool `vsinstr.exe` which has support for instrumenting binaries after +they have already been built. Since LLVMs instrumentation-based code coverage hooks into each object +file it generates this scenario is a bit different than the feature this RFC proposes. `vsinstr` does +allow for excluding namespaces of functions to skip over so that everything within a binary does not +get instrumented. + + - [vsinstr](https://docs.microsoft.com/en-us/previous-versions/visualstudio/visual-studio-2017/profiling/vsinstr?view=vs-2017) + +## gcov based coverage + +Rust also has support for another type of code coverage, a GCC-compatible, gcov-based coverage implementation. +This can be enabled through the `-Z profile` flag. This uses debuginfo to derive code coverage. This is different +than the source-based code coverage which allows for a more precise instrumentation to be done. + + - [Source-Based Code Coverage](https://blog.rust-lang.org/inside-rust/2020/11/12/source-based-code-coverage.html) + +# Unresolved questions +[unresolved-questions]: #unresolved-questions + +- Are there any drawbacks from having Cargo set the `LLVM_PROFILE_FILE` environment variable that LLVM uses to name each +of the generated `profraw` files? This is used to ensure each test run generates unique profiling data as opposed to overwriting +the previous run. + +# Future possibilities +[future-possibilities]: #future-possibilities + +## Specifying multiple crates to instrument for Code Coverage + +This would allow for Rust developers to specify each of the crates they want to instrument in +advance and Cargo would be able to pass on the `-C instrument-coverage` flag for only the +crates specified. This would allow a more targeted approach of getting code coverage results +and not for developers to instrument the entire dependency graph. This could either be in the form +of a manifest key in the toml file which would take a `,` separated list of crates to include in +the code coverage analysis or by specifying each crate at the command line using `--coverage:crate_name`. From 68368197b9517356a8af63462a17903f4ffe8d18 Mon Sep 17 00:00:00 2001 From: ridwanabdillahi <91507758+ridwanabdillahi@users.noreply.github.com> Date: Fri, 15 Jul 2022 12:13:02 -0700 Subject: [PATCH 2/4] Update RFC with added details for overriding defaults and cargo flags. --- text/0000-cargo-test-coverage.md | 81 +++++++++++++++++++++++++------- 1 file changed, 64 insertions(+), 17 deletions(-) diff --git a/text/0000-cargo-test-coverage.md b/text/0000-cargo-test-coverage.md index ba80b2c1a2e..feef86b744d 100644 --- a/text/0000-cargo-test-coverage.md +++ b/text/0000-cargo-test-coverage.md @@ -9,7 +9,7 @@ This RFC aims to improve the process of collecting code coverage data for Rust libraries. By including Cargo in the process of instrumenting Rust libraries and running the unit tests, the sequence of steps to get coverage -results will be simplified. This RFC also proposes adding support for cargo +results will be simplified. This RFC also proposes adding support for Cargo to selectivly choose which crates get instrumented for gathering coverage results. # Motivation @@ -20,7 +20,7 @@ Why are we doing this? What use cases does it support? What is the expected outc The motivation behind this feature is to allow for a simple way for a Rust developer to run and obtain code coverage results for a specific set of crates to ensure confidence in code quality and correctness. Currently, in order to get instrumentation based code coverage, -a Rust developer would have to either update the `RUSTFLAGS` environment variable or cargo +a Rust developer would have to either update the `RUSTFLAGS` environment variable or Cargo manifest keys with `-C instrument-coverage`. This would automatically enable instrumentation of all Rust crates within the dependency graph, not just the top level crate. Instrumenting all crates including transitive dependencies does not help the developer ensure test coverage @@ -35,10 +35,10 @@ This section examines the features proposed by this RFC: ## CLI option -A new flag for the `cargo test` command would be added. The new flag `--coverage` would instruct Cargo to -add the Rust compiler flag `-C instrument-coverage`, for the given crate only. This would mean that only the top-level -crate would be instrumented and code coverage results would only run against this crate. As an example, lets take the -following crate `foo`: +Three new flags will be added for the `cargo test` command, `--coverage`, `--coverage-format` and `--coverage-output-directory` +The `--coverage` command would instruct Cargo to add the Rust compiler flag `-C instrument-coverage`, for the given +crate only. This would mean that only the top-level crate would be instrumented and code coverage results would only +run against this crate. As an example, lets take the following crate `foo`: ```text /Cargo.toml @@ -85,21 +85,32 @@ mod tests { ``` Now running `cargo test --coverage` would build the `foo` crate and instrument all libraries created by this crate. -Next, the tests for the crate `foo` will be run and produce code coverage results for this crate only and ignore -all code coverage for any function defined outside of this crate. +Each test executable run will generate a unique `*.profraw` file. At the end of all test execution, Cargo will be +responsible for merging the generated `profraw` files, and by default would generate code coverage results in HTML +format for simple viewing within a browser. The code coverage results produced would be only for the crate `foo` +and ignore all code coverage for any function defined outside of this crate. -The `cargo test` subcommand also supports the `--package` and `--workspace` flags. This instructs cargo to run the +LLVM supports exporting code coverage results in a number of formats. `--coverage-format` would be responsible for selecting +the code coverage results format and `--coverage-output-directory` would allow a Rust developer to select the +output directory for the code coverage report. The default for `--coverage-format` is `html` and the default for +`--coverage-output-directory` would be `target/coverage/` + +The `cargo test` subcommand also supports the `--package` and `--workspace` flags. This instructs Cargo to run the tests for those specific packages. When combined with the new `--coverage` flag, a Rust developer will be able to selectively choose which Rust crates will be instrumented and have tests run. Since potentially unit tests from multiple crates will be run, the code coverage results will include coverage results for a crate that is being covered by a test from another crate. +With this feature, existing custom Cargo commands can leverage Cargo to do all of the heavy work to instrument specific +crates and generate coverage results in a multitude of formats. This will allow in more flexibilty in generating +for custom commands that generate code coverage for crates today. + # Reference-level explanation [reference-level-explanation]: #reference-level-explanation -As mentioned earlier this RFC proposes adding a new flag to the `cargo test` command. This flag, `--coverage` +As mentioned earlier this RFC proposes adding multiple flags to the `cargo test` command. The flag `--coverage` would be responsible for setting the `-C instrument-coverage` flag that Cargo would pass on to the rustc invocation of the -top-level crate. In the previous example, `foo` would be the top level crate and `regex` would be upstream an dependency. +top-level crate. In the previous example, `foo` would be the top level crate and `regex` would be an upstream dependency. Using the `--coverage` flag, Cargo would only set the `-C instrument-coverage` flag for the crate `foo`. If the `RUSTFLAGS` environment variable had already been set to include the `-C instrument-coverage` flag, then Cargo would still pass that @@ -117,15 +128,41 @@ then Cargo would not make any changes and would leave the value as is to use the variable is not set, Cargo would set it to ensure a unique naming scheme is used for each `.profraw` file that would be generated. -Once the tests have finished running, cargo would leverage LLVM tooling to analyze profraw files and generate +Once the tests have finished running, Cargo would leverage LLVM tooling to analyze profraw files and generate coverage reports for a Rust developer. These tools would be `llvm-profdata` and `llvm-cov` and can be used by adding the `llvm-tools-preview` component to the current toolchain, `rustup component add llvm-tools-preview`. This would be a requirement to use this new feature since these are the tools necessary to analyze coverage results -and generate a code coverage report. +and generate a code coverage report. If the component is not installed for the current toolchain, an error will occur +with a message stating the the `llvm-tools-preview` component is required to generate a code coverage report. + +For example, a Rust developer can invoke the following cargo command to generate an HTML coverage report +for the top-level crate or all crates in the workspace: + +``` +cargo test --coverage +``` + +This will generate a code coverage report in HTML format in the `target/coverage/` directory. + +To override the default output format and directory, the `--coverage-format` and `--coverage-output-directory` +can be passed to the cargo CLI: + +``` +cargo test --coverage --coverage-format=lcov --coverage-output-directory=src/coverage +``` + +This will generate a code coverage report in lcov format in the `src/coverage/` directory. + +The supported options for the `--coverage-format` option are: + +1. `html` +2. `lcov` +3. `json` These updates to Cargo would be sufficient enough to ensure that a Rust developer would have control over what crates are -instrumented and code coverage results are generated. This would also allow the Rust developer to no longer have to -set environment variables manually to ensure crates are instrumented for gathering coverage data. +instrumented, the format in which code coverage results are generated and where they are stored. This would also allow +the Rust developer to no longer have to set environment variables manually to ensure crates are instrumented +for gathering coverage data and can generate code coverage results with a single command. # Drawbacks [drawbacks]: #drawbacks @@ -146,7 +183,7 @@ would reduce the need for setting environment variables manually. Simply running `cargo test --coverage` would automatically run a build setting the `-C instrument-coverage` Rust flag only for the top level crate and set the `LLVM_PROFILE_FILE` environment variable to ensure each test run produces a unique `profraw` file. Once the tests have -finished running, cargo would leverage LLVM tooling to analyze profraw files and generate +finished running, Cargo would leverage LLVM tooling to analyze profraw files and generate coverage reports for a Rust developer. This design does not break any existing usage of Rust or Cargo. This new feature would @@ -244,6 +281,16 @@ the previous run. This would allow for Rust developers to specify each of the crates they want to instrument in advance and Cargo would be able to pass on the `-C instrument-coverage` flag for only the crates specified. This would allow a more targeted approach of getting code coverage results -and not for developers to instrument the entire dependency graph. This could either be in the form +and not instrumenting the entire dependency graph. This could either be in the form of a manifest key in the toml file which would take a `,` separated list of crates to include in the code coverage analysis or by specifying each crate at the command line using `--coverage:crate_name`. + +## A single command to generate coverage results and open in a browser + +Running `cargo test --coverage --open` would greatly simplify the experience of viewing coverage results. +With a simple command, a Rust developer can run all of their tests with source-based instrumentation +enabled, and view the coverage results in a browser. This would need a new flag `--open` flag to be +added to the `cargo test` command and would only be valid if the `--coverage` flag was enabled. This +would produce a very similar experience to how `cargo doc --open` works today. `cargo doc` compiles all +of the documentation for a crate and the `--open` flag automatically opens a browser showing all of the +generated documentation. This would be a great feature to have when generating code coverage. From b6ffec134732aff8283e794caec74f91d740aecd Mon Sep 17 00:00:00 2001 From: ridwanabdillahi <91507758+ridwanabdillahi@users.noreply.github.com> Date: Wed, 20 Jul 2022 10:45:51 -0700 Subject: [PATCH 3/4] Remove the usage of `top-level` crate. Add the ability to override the default crates instrumented via the `--coverage` command to the current RFC and remove from the future possibilities section. --- text/0000-cargo-test-coverage.md | 95 ++++++++++++++++++++++++-------- 1 file changed, 71 insertions(+), 24 deletions(-) diff --git a/text/0000-cargo-test-coverage.md b/text/0000-cargo-test-coverage.md index feef86b744d..f1062904515 100644 --- a/text/0000-cargo-test-coverage.md +++ b/text/0000-cargo-test-coverage.md @@ -22,11 +22,12 @@ to run and obtain code coverage results for a specific set of crates to ensure c in code quality and correctness. Currently, in order to get instrumentation based code coverage, a Rust developer would have to either update the `RUSTFLAGS` environment variable or Cargo manifest keys with `-C instrument-coverage`. This would automatically enable instrumentation -of all Rust crates within the dependency graph, not just the top level crate. Instrumenting +of all Rust crates within the dependency graph, not just the current crate. Instrumenting all crates including transitive dependencies does not help the developer ensure test coverage for the crates they actually want to test. This also adds unnecessary work for both the Rust compiler and codegen backend, `LLVM` in this case, to instrument all libraries as opposed to -the subset a Rust developer actually cares about. +the subset a Rust developer actually cares about. This support is currently limited to the +`LLVM` codegen backend and will have no effect when another codegen backend is used. # Guide-level explanation [guide-level-explanation]: #guide-level-explanation @@ -36,14 +37,20 @@ This section examines the features proposed by this RFC: ## CLI option Three new flags will be added for the `cargo test` command, `--coverage`, `--coverage-format` and `--coverage-output-directory` -The `--coverage` command would instruct Cargo to add the Rust compiler flag `-C instrument-coverage`, for the given -crate only. This would mean that only the top-level crate would be instrumented and code coverage results would only -run against this crate. As an example, lets take the following crate `foo`: +The `--coverage` flag would instruct Cargo to add the rustc flag `-C instrument-coverage`, for the current +crate only. Cargo also supports two types of workspaces, a workspace with a root package and one without called a +virtual manifest. In the case where the `cargo test --coverage` command is run from a Cargo workspace which has a root package, +the `-C instrument-coverage` rustc flag would only be enabled for the root package. In the case where the `cargo test --coverage` +command is run from a Cargo workspace without a root package, the `-C instrument-coverage` rustc flag would be enabled for +all default members of the workspace. This would mean that only the crates selected would be instrumented and code +coverage results would only be collected for those crates and not all crates in the dependency graph. + +As an example, let's take the following crate `foo`: ```text /Cargo.toml - +-- src - +-- lib.rs +/src + +-- lib.rs ``` Where crate `foo` has a dependency on the `regex`: @@ -101,6 +108,48 @@ selectively choose which Rust crates will be instrumented and have tests run. Si multiple crates will be run, the code coverage results will include coverage results for a crate that is being covered by a test from another crate. +The `--coverage` flag will also support a comma separated list of crates so that a Rust developer can control +the exact set of crates which will be instrumented during a build and test run. + +For example, let's take the following workspace with default members: + +```text +/Cargo.toml +/member1 + +-- src + +-- lib.rs +/member2 + +-- src + +-- lib.rs +/member3 + +-- src + +-- lib.rs +``` + +Where crate `foo` has a virtual manifest: +```toml +[workspace] +members = ["path/to/member1", "path/to/member2", "path/to/member3"] +default-members = ["path/to/member2"] +``` + +Running the following cargo invocation would only instrument the workspace member +`member2` and run the tests for `member2`: + +``` +cargo test --coverage +``` + +To instrument a different workspace member, the following command is also supported: + +``` +cargo test --coverage=member1,member3 +``` + +This cargo invocation will instruct Cargo to run the tests for the default workspace member `member2`, but +will only instrument the crates `member1` and `member3` to collect coverage results for. This flexibility +allows a Rust developer to see just how much a given crate is covered by tests from another crate. + With this feature, existing custom Cargo commands can leverage Cargo to do all of the heavy work to instrument specific crates and generate coverage results in a multitude of formats. This will allow in more flexibilty in generating for custom commands that generate code coverage for crates today. @@ -108,9 +157,9 @@ for custom commands that generate code coverage for crates today. # Reference-level explanation [reference-level-explanation]: #reference-level-explanation -As mentioned earlier this RFC proposes adding multiple flags to the `cargo test` command. The flag `--coverage` +As mentioned earlier, this RFC proposes adding multiple flags to the `cargo test` command. The flag `--coverage` would be responsible for setting the `-C instrument-coverage` flag that Cargo would pass on to the rustc invocation of the -top-level crate. In the previous example, `foo` would be the top level crate and `regex` would be an upstream dependency. +current crate. In the previous example, `foo` would be the crate being instrumented and `regex` would be an upstream dependency. Using the `--coverage` flag, Cargo would only set the `-C instrument-coverage` flag for the crate `foo`. If the `RUSTFLAGS` environment variable had already been set to include the `-C instrument-coverage` flag, then Cargo would still pass that @@ -135,14 +184,21 @@ This would be a requirement to use this new feature since these are the tools ne and generate a code coverage report. If the component is not installed for the current toolchain, an error will occur with a message stating the the `llvm-tools-preview` component is required to generate a code coverage report. -For example, a Rust developer can invoke the following cargo command to generate an HTML coverage report -for the top-level crate or all crates in the workspace: +For example, a Rust developer can invoke the following Cargo command to generate an HTML coverage report +for the current crate or `default-members` in a workspace: ``` cargo test --coverage ``` -This will generate a code coverage report in HTML format in the `target/coverage/` directory. +This will generate a code coverage report in HTML format in the `target/coverage/` directory. If a workspace +does not have `default-members`, then all members would be instrumented. + +Run the following cargo CLI to choose a specific set of crates to instrument and override the defaults: + +``` +cargo test --coverage:crate1,crate3,foo +``` To override the default output format and directory, the `--coverage-format` and `--coverage-output-directory` can be passed to the cargo CLI: @@ -181,14 +237,14 @@ This design provides a simple mechanism to integrate collecting code coverage results for a given crate. Allowing Cargo to be part of the coverage process would reduce the need for setting environment variables manually. Simply running `cargo test --coverage` would automatically run a build setting the `-C instrument-coverage` -Rust flag only for the top level crate and set the `LLVM_PROFILE_FILE` environment +rustc flag only for the crates selected and set the `LLVM_PROFILE_FILE` environment variable to ensure each test run produces a unique `profraw` file. Once the tests have finished running, Cargo would leverage LLVM tooling to analyze profraw files and generate coverage reports for a Rust developer. This design does not break any existing usage of Rust or Cargo. This new feature would be strictly opt-in. A Rust developer would still be able to manually set the -`-C instrument-coverage` Rust flag and instrument all binaries within the dependency +`-C instrument-coverage` rustc flag and instrument all binaries within the dependency graph. Since this is a Cargo specific feature, the Rust compiler will not need any updates. ## Alternatives @@ -204,7 +260,7 @@ the dependency graph, including upstream dependencies. This would also instrumen binaries and report on coverage for functions and branches that are not defined by the current crate with the potential of skewing coverage results. -### Alternative 2: use a new manifest key instead of a cli option +### Alternative 2: use a new manifest key instead of a CLI option Supporting this alternative would mean that changes would need to be made to the existing manifest format to possibly include a new section and/or key. A new `coverage` key could @@ -276,15 +332,6 @@ the previous run. # Future possibilities [future-possibilities]: #future-possibilities -## Specifying multiple crates to instrument for Code Coverage - -This would allow for Rust developers to specify each of the crates they want to instrument in -advance and Cargo would be able to pass on the `-C instrument-coverage` flag for only the -crates specified. This would allow a more targeted approach of getting code coverage results -and not instrumenting the entire dependency graph. This could either be in the form -of a manifest key in the toml file which would take a `,` separated list of crates to include in -the code coverage analysis or by specifying each crate at the command line using `--coverage:crate_name`. - ## A single command to generate coverage results and open in a browser Running `cargo test --coverage --open` would greatly simplify the experience of viewing coverage results. From 98270709dd2dc5b171111897ed2859b0b42b73a8 Mon Sep 17 00:00:00 2001 From: ridwanabdillahi <91507758+ridwanabdillahi@users.noreply.github.com> Date: Fri, 22 Jul 2022 20:44:52 -0700 Subject: [PATCH 4/4] Respond to PR comments. Add collecting coverage data for doctests, tests in binary targets and examples. --- text/0000-cargo-test-coverage.md | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/text/0000-cargo-test-coverage.md b/text/0000-cargo-test-coverage.md index f1062904515..58404dfc970 100644 --- a/text/0000-cargo-test-coverage.md +++ b/text/0000-cargo-test-coverage.md @@ -97,6 +97,12 @@ responsible for merging the generated `profraw` files, and by default would gene format for simple viewing within a browser. The code coverage results produced would be only for the crate `foo` and ignore all code coverage for any function defined outside of this crate. +Doctests, binary tests and example tests would all be part of the coverage report if these tests were included as part of +the test run. The type of tests run can be filtered by using the `--libs`, `--bins`, `--doc`, or `--examples` flag for the +`cargo test` subcommand. By default running `cargo test` runs all tests for lib, bins and test targets unless the +`test` key for the `[lib]`, `[[bin]]` and/or `[[test]]` section was set to false. For more information about +configuring a target, see [the Cargo book](https://doc.rust-lang.org/cargo/reference/cargo-targets.html#configuring-a-target). + LLVM supports exporting code coverage results in a number of formats. `--coverage-format` would be responsible for selecting the code coverage results format and `--coverage-output-directory` would allow a Rust developer to select the output directory for the code coverage report. The default for `--coverage-format` is `html` and the default for @@ -228,6 +234,13 @@ environment variable in order to ensure unique profile data is generated for eac executable. I am not aware of any other Cargo features that set environment variables today so this could potentially create new issues in Cargo. +Another drawback of this feature is that it is specifically geared towards the LLVM codegen +backend. This feature can be expanded upon to support other codegen backends in the future +if/when a code coverage instrumentation mechanism exists for other backends. The libgccjit +AOT codegen for rustc seems to have some support for coverage instrumentation similar to how +LLVM inserts counters at source locations which is promising for adding support for this +feature when targeting this codegen backend in the future. + # Rationale and alternatives [rationale-and-alternatives]: #rationale-and-alternatives @@ -341,3 +354,13 @@ added to the `cargo test` command and would only be valid if the `--coverage` fl would produce a very similar experience to how `cargo doc --open` works today. `cargo doc` compiles all of the documentation for a crate and the `--open` flag automatically opens a browser showing all of the generated documentation. This would be a great feature to have when generating code coverage. + +## Add support for other codegen backends + +Currently this feature is geared towards supporting LLVM's codegen backend but that does not mean +other codegen backends could not be supported in the future. The Cranelift codegen backend does not +support gathering code coverage data via instrumentation. This feature could be added in the future at +which the features proposed by this RFC can be revisited. The libgccjit codegen backend does seem to +have some support for instrumentation and inserting counters at source locations to support collecting +code coverage data. This could be a feature added support using this proposed feature for this backend +in the future.