-
Notifications
You must be signed in to change notification settings - Fork 13k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Doubts about Incorrect Source-Based Test Coverage Results #91661
Comments
@qiangheisenberg I don't test with grcov. It's a layer above the LLVM tools that generate the coverage reports, as I understand it. It's possible you need to tell grcov other things, to get it to process the files properly (for example, to tell it where the test binaries are... here's a similar bug report where the author realized this was an issue: #79456). If you really think there's a bug in coverage, please generate an MCVE, using If you use the documented process on a small example, preferably with a single Rust source file, that's the best way to give me something I can use to debug a coverage problem. Thanks! |
@qiangheisenberg Do you get the same results using the instructions in the Unstable book and llvm-cov as @richkadel suggested? |
@hkratz thank you for your reply. I followed the steps in the unstable book to test. The effect is better, but I still haven't explained my doubts.
There are still undetected codes in the file In summary, the steps and results of my test. From the results, the results of using llvm-cov, grcov, and gcov are:
Their test environment is the same.
Is it because I missed something? If you test |
@qiangheisenberg this appears to be the same underlying issue as the other reports of missed code coverage data and is fixed by #92142 as well. With those changes applied, I see the following from llvm-cov: For Q1:
For Q2:
(Notice that no tests from For Q3: If you still observe this after #92142 is merged, please open an issue with examples so we can take a closer look. |
Oh, my God, I'm so excited. Thank you for your work.It's true that a lot of functions are covered. The ability to test coverage is almost perfect. There's only one thing I'm confused about:
|
This works as intended (WAI), for now. LLVM coverage profiling tracks code regions (starting file position and ending file position). In Rust's implementation of LLVM coverage, comments don't break up the code region, which starts (for example, in your second image) after the open brace and ends before the closing brace. See the Rust coverage test results to get a full understanding of what is currently "expected" coverage results. For example, the following coverage report test result shows fully-covered functions, including comments in the code region: rust/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.async.txt Lines 51 to 68 in 3ece606
|
@qiangheisenberg - I think there is a problem in how you are generating the coverage report. I ran the
|
Thinking about possible causes for your missing coverage, I have another theory. The This is something @wesleywiser already mentioned, and fixed in a pending PR (#92142) to be merged. Please wait until that PR is merged and lands in |
…ioning, r=tmandry [code coverage] Fix missing dead code in modules that are never called The issue here is that the logic used to determine which CGU to put the dead function stubs in doesn't handle cases where a module is never assigned to a CGU (which is what happens when all of the code in the module is dead). The partitioning logic also caused issues in rust-lang#85461 where inline functions were duplicated into multiple CGUs resulting in duplicate symbols. This commit fixes the issue by removing the complex logic used to assign dead code stubs to CGUs and replaces it with a much simpler model: we pick one CGU to hold all the dead code stubs. We pick a CGU which has exported items which increases the likelihood the linker won't throw away our dead functions and we pick the smallest to minimize the impact on compilation times for crates with very large CGUs. Fixes rust-lang#91661 Fixes rust-lang#86177 Fixes rust-lang#85718 Fixes rust-lang#79622 r? `@tmandry` cc `@richkadel` This PR is not urgent so please don't let it interrupt your holidays! 🎄 🎁
…ioning, r=tmandry [code coverage] Fix missing dead code in modules that are never called The issue here is that the logic used to determine which CGU to put the dead function stubs in doesn't handle cases where a module is never assigned to a CGU (which is what happens when all of the code in the module is dead). The partitioning logic also caused issues in rust-lang#85461 where inline functions were duplicated into multiple CGUs resulting in duplicate symbols. This commit fixes the issue by removing the complex logic used to assign dead code stubs to CGUs and replaces it with a much simpler model: we pick one CGU to hold all the dead code stubs. We pick a CGU which has exported items which increases the likelihood the linker won't throw away our dead functions and we pick the smallest to minimize the impact on compilation times for crates with very large CGUs. Fixes rust-lang#91661 Fixes rust-lang#86177 Fixes rust-lang#85718 Fixes rust-lang#79622 r? `@tmandry` cc `@richkadel` This PR is not urgent so please don't let it interrupt your holidays! 🎄 🎁
…ioning, r=tmandry [code coverage] Fix missing dead code in modules that are never called The issue here is that the logic used to determine which CGU to put the dead function stubs in doesn't handle cases where a module is never assigned to a CGU (which is what happens when all of the code in the module is dead). The partitioning logic also caused issues in rust-lang#85461 where inline functions were duplicated into multiple CGUs resulting in duplicate symbols. This commit fixes the issue by removing the complex logic used to assign dead code stubs to CGUs and replaces it with a much simpler model: we pick one CGU to hold all the dead code stubs. We pick a CGU which has exported items which increases the likelihood the linker won't throw away our dead functions and we pick the smallest to minimize the impact on compilation times for crates with very large CGUs. Fixes rust-lang#91661 Fixes rust-lang#86177 Fixes rust-lang#85718 Fixes rust-lang#79622 r? ``@tmandry`` cc ``@richkadel`` This PR is not urgent so please don't let it interrupt your holidays! 🎄 🎁
I used grcov to measure the test coverage of the project, but the results didn't seem very good, both source-based and gcov-based.@richkadel
The source-based approach is shown here.
To test the rand project, perform the following steps:
In the
rand
test result, for example, in the/rand/target/debug/coverage/rand_core/src/le.rs
file:Q1: In this file, two similar functions, one is detected and the other is not found. I don't know why.
Only one function is shown here, and the other function seems odd that grcov did not find it.
Note: In the
/rand/target/debug/coverage/rand_core/src/block.rs
file, a large number of codes are not covered. Judgment here seems to have failed....
Q2:In addition, the test function does not appear to be detected. In other files, the test function is executed.
Do not execute test code:
/rand/target/debug/coverage/rand_core/src/os.rs
execute test code:
/rand/target/debug/coverage/src/seq/mod.rs
Q3:Sometimes structs/enumerations are included in the row coverage, but sometimes they are not. What are the criteria for this?
rustc --version --verbose
:The text was updated successfully, but these errors were encountered: