Skip to content
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

No coverage information generated #20

Closed
zaneduffield opened this issue Feb 26, 2024 · 15 comments
Closed

No coverage information generated #20

zaneduffield opened this issue Feb 26, 2024 · 15 comments

Comments

@zaneduffield
Copy link
Contributor

I'm not sure if this is just some kind of user error, but I can't see to get coverage information generated for my datatests.

Coverage information is being generated, but it only seems to contain the hits from the non-datatest tests. When I clear the '*.profraw' files and run just the datatest tests, there is 0% coverage.

Initially I tried running cargo llvm-cov --html, then I tried following the steps for external tests but that didn't work either.

What might make my use-case a bit unusual is the fact that the datatest tests are in their own crate in the workspace, and I'm expecting coverage information to be generated for another crate in the workspace.
I thought this structure could be part of the problem, so I tried replacing the datatest_stable::harness! macro with a main function that directly calls a function from the other crate, and then coverage information was successfully generated.

Any help on this issue would be greatly appreciated.

@sunshowers
Copy link
Member

Hi, thanks for the report -- completely missed this earlier.

This is very strange. I got around to adding coverage for datatest-stable itself, and it seems to work fine: https://app.codecov.io/github/nextest-rs/datatest-stable/commit/ff0bbaf8923cdcb8621bafa459ad11589085845a/tree/src. (But this is within the same crate.)

datatest_stable::harness is just a main function that calls into your functions through a couple levels of indirection, so there really isn't anything that should be different.

Do you have a minimal reproduction example?

@zaneduffield
Copy link
Contributor Author

@sunshowers I created a minimal example repo here: https://github.com/zaneduffield/datatest_coverage_issue

Please let me know if you can reproduce the issue with this.

@sunshowers
Copy link
Member

Thanks for the example! Sadly I can't repro this at all. I get identical coverage results in both cases.

image

image

The results with your main and with the datatest-stable harness are completely identical.

This is with Rust 1.77.1 on Linux x86_64.

@sunshowers
Copy link
Member

I realized that my cargo-llvm-cov was out of date (0.5.3). I updated it to the latest version, 0.6.9, and still see the same results.

@zaneduffield
Copy link
Contributor Author

Sadly I can't repro this at all.

I thought that might be the case 😢.
I just upgraded cargo-llvm-cov and rust to match your versions and I still have the issue.
I am running on Windows, which would be the main remaining difference between our systems.

I'll close this here for now since it doesn't seem like the fault of datatest-stable. It's more likely to be an upstream issue with libtest-mimic (or some weird configuration issue on my system).

@zaneduffield zaneduffield closed this as not planned Won't fix, can't repro, duplicate, stale Apr 9, 2024
@sunshowers
Copy link
Member

Ahh -- Windows is interesting! There might definitely be something weird going on there. Would definitely be curious to learn if you find out more.

@zaneduffield
Copy link
Contributor Author

I think I worked out the problem.
Right here, libtest_mimic::Conclusion::exit is called on the return value of libtest_mimic::run

libtest_mimic::run(&args, tests).exit()

Internally, this function calls process::exit
https://github.com/LukasKalbertodt/libtest-mimic/blob/d68ce73c7066a769e303b589cf79676c33c09495/src/lib.rs#L339C1-L350C6

From the documentation of process::exit

Note that because this function never returns, and that it terminates the process, no destructors on the current stack or any other thread's stack will be run.

I have a feeling that (maybe just on Windows), something is depending on destructors being terminated on some thread.

When I return ExitCode from the main function, instead of calling libtest_mimic::Conclusion::exit, coverage works perfectly.

For example, if I use this as my main function I get no coverage

use std::{path::PathBuf, process::ExitCode};
use libtest_mimic::{Arguments, Trial};

fn main() {
    let args = Arguments::from_args();

    libtest_mimic::run(
        &args,
        vec![Trial::test("foo", || {
            call_fn_to_cover(&PathBuf::new()).unwrap();
            Ok(())
        })],
    ).exit();
}

but if I change it to use ExitCode, then coverage is generated properly:

use std::{path::PathBuf, process::ExitCode};
use libtest_mimic::{Arguments, Trial};

fn main() -> ExitCode {
    let args = Arguments::from_args();

    let result = libtest_mimic::run(
        &args,
        vec![Trial::test("foo", || {
            call_fn_to_cover(&PathBuf::new()).unwrap();
            Ok(())
        })],
    );

    if result.has_failed() {
        ExitCode::from(101)
    } else {
        ExitCode::SUCCESS
    }
}

@zaneduffield zaneduffield reopened this Apr 9, 2024
@sunshowers
Copy link
Member

Aha! Yes that would explain it, wouldn't it. Worth filing in libtest-mimic, and honestly might be a bug in Rust itself.

@sunshowers
Copy link
Member

I've fixed this in 33d4725, planning to get a release out very soon.

@sunshowers
Copy link
Member

Meanwhile could you try patching that in locally and ensuring it works?

@zaneduffield
Copy link
Contributor Author

Meanwhile could you try patching that in locally and ensuring it works?

Yep, with this patch coverage is generated successfully.

[patch.crates-io]
datatest-stable = { git = "https://github.com/nextest-rs/datatest-stable/", rev = "33d4725463e839060c70254b0c879ad5e27a3cfb" }

Thanks for fixing this 😄.

Do you still think I should raise this with libtest-mimic? I guess the only thing they could be doing better is have a function that returns an ExitCode instance, but it would be a breaking change to have the existing Conclusion::exit function do that.

I wouldn't really know if this is worth raising with rust (or llvm).

@sunshowers
Copy link
Member

sunshowers commented Apr 9, 2024

Awesome, datatest-stable 0.2.5 should be out in a few.

I think it's absolutely worth raising with libtest-mimic, yeah. Other custom harnesses are going to run into this. Personally I'd consider deprecating Conclusion::exit, and adding a new exit_code method to Conclusion.

The MSRV for ExitCode is Rust 1.61 so it's been long enough.

As far as Rust or llvm goes, probably not, but I think it's worth getting this documented in llvm-cov. Windows is a relatively rare OS for Rust dev and std::process::exit seems to work fine on Linux, so this will be surprising to a lot of people I think.

Thanks again and great job on the investigation!

@zaneduffield
Copy link
Contributor Author

FYI @sunshowers libtest-mimic 0.7.1 just added Conclusion::exit_code

@sunshowers
Copy link
Member

FYI @sunshowers libtest-mimic 0.7.1 just added Conclusion::exit_code

Thanks. Needs LukasKalbertodt/libtest-mimic#40 to be fixed to use it.

@sunshowers
Copy link
Member

@zaneduffield released datatest-stable 0.2.6 with libtest-mimic 0.7.2. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants