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

Code Coverage in wasm-bindgen-test? #2276

Open
anp opened this issue Aug 8, 2020 · 12 comments
Open

Code Coverage in wasm-bindgen-test? #2276

anp opened this issue Aug 8, 2020 · 12 comments

Comments

@anp
Copy link

anp commented Aug 8, 2020

Motivation

I'd like to be able to use -Zprofile when building my wasm-bindgen tests with wasm-pack and have those tests produce coverage files. I currently run my tests in a headless browser with wasm-pack and an ideal solution would have a clear integration story there with a good out of the box experience.

Proposed Solution

The instrumentation emitted by default writes coverage profiling data to disk in a location relative to the binary being run. If there's a nice way to override those callbacks for a single binary/target then wasm-bindgen could presumably collect the coverage buffers in its JS test harness and forward them to the CLI for persisting to disk.

Alternatives

I'm still looking at LLVM's coverage docs and I'm not sure if it's straightforward to override the instrumentation collection buffers like that. If it's not then I'd be willing to compile in a larger (wasi?) runtime into my tests to provide the needed filesystem I/O. I think the LLVM instrumentation at least offers environment variables to control the paths where data is written.

Additional Context

I looked for related issues and couldn't find any. I think that "test artifact extraction" from wasm tests might also be useful for supporting snapshot testing but I'm not familiar enough with relevant internals to say for sure.

@anp anp added the enhancement label Aug 8, 2020
@alexcrichton
Copy link
Contributor

To some degree WebAssembly instrumentation and coverage should be easier in theory than native binaries because it's easy to change/inject instructions into a binary (e.g. at the top/end of all blocks). That would require work investing into some sort of wasm transform, however, which would require a side table (like dwarf info) about what instruction corresponds to what.

Otherwise LLVM's infrastructure may work with tweaks, but AFAIK no work has been done towards that. I would suspect you'd probably want to start out with wasm32-wasi since it'll be easier as many things already "just work", but if something works there seems reasonable to try to extend it to wasm-bindgen!

@anp
Copy link
Author

anp commented Aug 16, 2020

Makes sense and thanks for the advice! I am skimming https://motley-coder.com/2019/03/31/webassembly-coverage/ and it looks like compiler-rt support for the target is the main thing.

@Amanieu
Copy link

Amanieu commented Feb 8, 2021

You could try using minicov which embeds a stripped down version of the LLVM profiling runtime. It has no dependencies on libc or any external code so it should compile on WASM. You'll need to write out the profraw file yourself though (see the README).

@bbarwik
Copy link

bbarwik commented Oct 19, 2023

Hey. I've created a tutorial how to generate code coverage for webassembly projects. This one is dedicated for blockchains using WASM VM, however I think it may be useful to you - https://github.com/hknio/code-coverage-for-webassembly

@fzyzcjy
Copy link

fzyzcjy commented Dec 19, 2023

Hi, is there any updates? Thanks!

@aDogCalledSpot
Copy link

aDogCalledSpot commented Jan 9, 2024

I looked into this a bit and tried to simply compile a test using minicov.

wasm_bindgen_test::wasm_bindgen_test_configure!(run_in_browser);

#[cfg(test)]
mod test {
    #[wasm_bindgen_test::wasm_bindgen_test]
    async fn test_foo() {
        assert!(true);
    }
}

then run RUSTFLAGS="-Cprofile-generate -Zno-profiler-runtime" wasm-pack test --firefox --headless.

Output:

[2024-01-09T12:58:06Z DEBUG wasm_bindgen_wasm_interpreter] starting a call of Id { idx: 12653 } Some("__wbindgen_describe___wbg_window_57e9bd3b31ab8737.command_export")
[2024-01-09T12:58:06Z DEBUG wasm_bindgen_wasm_interpreter] arguments []
[2024-01-09T12:58:06Z DEBUG wasm_bindgen_wasm_interpreter] starting a call of Id { idx: 3133 } Some("__wasm_call_ctors")
[2024-01-09T12:58:06Z DEBUG wasm_bindgen_wasm_interpreter] arguments []
[2024-01-09T12:58:06Z DEBUG wasm_bindgen_wasm_interpreter] starting a call of Id { idx: 3138 } Some("__llvm_profile_init")
[2024-01-09T12:58:06Z DEBUG wasm_bindgen_wasm_interpreter] arguments []
[2024-01-09T12:58:06Z DEBUG wasm_bindgen_wasm_interpreter] starting a call of Id { idx: 3137 } Some("__llvm_profile_register_functions")
[2024-01-09T12:58:06Z DEBUG wasm_bindgen_wasm_interpreter] arguments []
[2024-01-09T12:58:06Z DEBUG wasm_bindgen_wasm_interpreter] starting a call of Id { idx: 0 } Some("__llvm_profile_register_function")
[2024-01-09T12:58:06Z DEBUG wasm_bindgen_wasm_interpreter] arguments [1205344]
thread 'main' panicked at crates/wasm-interpreter/src/lib.rs:222:18:
can only call locally defined functions

The last function to be called __llvm_profile_register_function is the first function from the shown stack that is part of minicov.

So it seems as if the issue is that we would have to be able to call functions through FFI but the wasm-interpreter doesn't support this since it's really only supposed to be used for much more simple things.

We need a way to skip these functions since they are irrelevant to the unit under test. First approaches are having complications with keeping the stack properly in order.

If we can skip through these functions and make it to the point where the generated wasm is actually mounted by the node/chrome/firefox, then we can see how to retrieve the coverage from minicov.

@aDogCalledSpot
Copy link

aDogCalledSpot commented Jan 10, 2024

I believe that the issue is that we are trying to measure coverage of the describe function created by #[wasm_bindgen]. The profiler doesn't know nor care that this macro expanded code so it just gets treated the same way as every other code. A simple solution could be to add #[coverage(off)] to describe but it's an unstable feature so you would also have to add #![feature(coverage_attribute)] to the appropriate path and that's what I'm currently struggling with.

@aDogCalledSpot
Copy link

I compiled rustc locally and removed the feature gate of #[coverage(off)]. Unfortunately, I wasn't able to get the desired results. For now, what works is simply ignoring any i64 related instructions and saying that these only exist because of coverage.

I can now at least run my code with coverage profiles. Will now look into getting the coverage information back out of the tests.

@aDogCalledSpot
Copy link

aDogCalledSpot commented Jan 11, 2024

I've been able to create a coverage report for a test (yay). However I simply printed the profdata to stdout and parsed it afterwards from there.

The remaining crucial question is how to best save the profdata during the tests.

We need to call minicov::capture_coverage after every test. This will generate profraw data which will need to be somehow written to a file, either immediately so there is a single profraw for each test, or if the data can be saved somewhere then we could generate a single profraw after all tests have run.

The #[wasm_bindgen_test] macro can easily be expanded to call something after a test, the question is what.

@alexcrichton are you aware of any ways to do this? We can't write to a file from a browser. Is it maybe possible to inject a hook to a function on the host which can write to a file? Otherwise we would probably have to set up a local server which receives the data and writes it to a file.

Otherwise the workflow is very close to what's specified in https://github.com/hknio/code-coverage-for-webassembly

# Build for use with minicov
RUSTFLAGS="-Cinstrument-coverage -Zno-profiler-runtime --emit=llvm-ir" wasm-pack test --chrome --headless
# Generate a {file_name}.o which contains the mapping of wasm instructions
# to source code lines. This should probably be done by us before we run the tests since
# otherwise there's a warning that the .o file is newer than the coverage data in the 
# profraws
clang target/wasm32-unknown-unknown/debug/deps/{file_name}.ll -Wno-override-module -c -o {tmpdir}/{file_name}.o
#??? Obtain profraws somehow
# Merge all profraws into a profdata
llvm-profdata merge --sparse {tmpdir}/*.profraw -o {tmpdir}/coverage.profdata
# Build HTML output from the profdata
llvm-cov show --instr-profile={tmpdir}/coverage.profdata {tmpdir}/{file_name}.o --format=html --output-dir=coverage/ --sources .

@daxpedda
Copy link
Collaborator

daxpedda commented Jan 11, 2024

@alexcrichton are you aware of any ways to do this? We can't write to a file from a browser. Is it maybe possible to inject a hook to a function on the host which can write to a file? Otherwise we would probably have to set up a local server which receives the data and writes it to a file.

We could built that into wasm-bindgen-test-runner, which hosts a server where we can add an endpoint that saves these files and does something with them afterwards.

I'm happy to review a proposal and a subsequent PR.

EDIT: The #[coverage(off)] problem could be solved by just requiring nightly to support this feature. I don't know what it would take to solve this for stable, but maybe some post processor could remove all describe() functions from the missing coverage?

@aDogCalledSpot
Copy link

aDogCalledSpot commented Jan 11, 2024

We could built that into wasm-bindgen-test-runner, which hosts a server

Oh, that's convenient. I'll add two endpoints then. One that is run after every test that appends the coverage data to a Vec and one that is run when all tests are done that dumps the Vec into a file. Then we only have to generate a single file which should improve performance significantly compared to having a file for each test - not only when running the tests but also when merging the profraws into a profdata.

The #[coverage(off)] problem could be solved by just requiring nightly to support this feature. I don't know what it would take to solve this for stable, but maybe some post processor could remove all describe() functions from the missing coverage?

I think stabilizing the feature wouldn't be much of an issue. Someone just has to go to the effort of creating the stabilization PR and starting the Final Comment Period. However, I wasn't able to get the results I wanted with this approach. Maybe I was still missing the annotation in some places. It's a small implementation detail we can discuss in the PR when we get there.

@daxpedda
Copy link
Collaborator

Great to hear!

Before making a PR it would be great if you put together a more detailed plan so we can get this approved before putting in any significant coding work.

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

Successfully merging a pull request may close this issue.

7 participants