taiki-e / cargo-llvm-cov

Cargo subcommand to easily use LLVM source-based code coverage (-C instrument-coverage).
Apache License 2.0
858 stars 57 forks source link

Add initial support for wasm-pack #338

Open aDogCalledSpot opened 5 months ago

aDogCalledSpot commented 5 months ago

Fixes https://github.com/taiki-e/cargo-llvm-cov/issues/337. Fixes https://github.com/taiki-e/cargo-llvm-cov/issues/221.

Requires https://github.com/rustwasm/wasm-bindgen/pull/3782.

Will also require a follow-up PR in wasm-pack but that will have trivial changes.


The changes made in this PR are still in a very rough state.

Before I turn this into a non-draft I'm mainly looking to get feedback about whether this is something you would be willing to include here.

Usage

Enable the unstable-coverage feature in the wasm-bindgen-test dependency.

cargo llvm-cov test --wasm --chrome --headless

Mixing WASM and non-WASM

Doing the following works:

cargo llvm-cov --no-report
cargo llvm-cov --no-report test --wasm --chrome --headless
cargo llvm-cov report

However, you need to do the following:

pub fn bool_to_str(b: bool) -> &'static str {
    if b {
        "true"
    } else {
        "false"
    }
}

pub fn bar(x: u32) -> u32 {
    if x == 0 {
        1
    } else {
        42
    }
}

fn main() {}

#[cfg(all(test, target_arch = "wasm32"))]
mod test {
    wasm_bindgen_test::wasm_bindgen_test_configure!(run_in_browser);
    use super::*;

    #[wasm_bindgen_test::wasm_bindgen_test]
    async fn test_foo() {
        assert_eq!(bool_to_str(true), "true");
    }
}

#[cfg(all(test, not(target_arch = "wasm32")))]
mod test {
    use super::*;

    #[test]
    fn test_bar() {
        assert_eq!(bar(0), 1);
    }
}

which is unfortunate.

I'll look into a bit and see what I find out.

@njelich do you maybe have any comments on this?

taiki-e commented 5 months ago

Thanks! I haven't reviewed the changes yet, but I'm very excited about this.

Personally, I would prefer to support this as a subcommand (as said in https://github.com/taiki-e/cargo-llvm-cov/issues/221#issuecomment-1250553812), as I find it confusing when the accepted flags change drastically depending on the presence or absence of specific flags. (This is also why we split the functionality of cargo llvm-cov without subcommand into cargo llvm-cov report and cargo llvm-cov test.)

aDogCalledSpot commented 5 months ago

I've updated it to use a subcommand. It's now working as you would expect.

cargo llvm-cov wasm-pack --chrome --headless

You can also do the following:

cargo llvm-cov --no-report
cargo llvm-cov --no-report wasm-pack --chrome --headless
cargo llvm-cov report

wasm-pack runs always return exactly one profraw so I use that to my advantage to figure out which object files I need to evaluate that specific run.

Still need to clean up some unwraps and so on but I'm happy with the logic.

aDogCalledSpot commented 5 months ago

I've documented the safety of the unwraps and think I'm ready here.

wasm-pack seems to be a bit slow on merging at the moment so I guess this will remain stale for a while here.

aDogCalledSpot commented 5 months ago

The tests are failing because I added --sources . as arguments to llvm-cov show. Not sure what exactly the reason is but I guess we can reach a similar result with --exclude.

aDogCalledSpot commented 5 months ago

I guess we can reach a similar result with --exclude.

Hmm doesn't seem I can. @taiki-e do you have any idea why --sources . breaks the tests?