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

PVF host: run coverage and/or miri #2739

Open
mrcnski opened this issue Dec 18, 2023 · 0 comments
Open

PVF host: run coverage and/or miri #2739

mrcnski opened this issue Dec 18, 2023 · 0 comments
Assignees
Labels
T0-node This PR/Issue is related to the topic “node”. T10-tests This PR/Issue is related to tests.

Comments

@mrcnski
Copy link
Contributor

mrcnski commented Dec 18, 2023

It would be useful to run this tooling on the PVF code to check for untested or potentially unsound code. They would only work on the PVF host code (see below for discussion about workers), but it could still be good to find missing tests.

Instructions

Both of these tools choke on wasm, so to run PVF host tests we would have to:

  1. Build the tests and have some code to write the wasm blobs to a file.
  2. Remove the wasm dependencies like rococo-runtime, test-parachain-adder, and test-parachain-halt, or run with SKIP_WASM_BUILD=1.
  3. Include the wasm blobs from the files using include_bytes.
  4. Now run the tests. For coverage I have some instructions here. For miri it's cargo +nightly miri test.

Note: coverage profiling may not work for the process tests since they use rusty-fork, which does some crazy stuff to run tests in separate processes.

Note: miri may not work because FWIU process::Command is not supported. We could stub it but I believe sockets are not supported either. It's probably still worth running it until it crashes. 🤷

Considered but not done: Running on workers

This was considered because all the unsafe code is on the workers, and a strength of miri FWIU is finding undefined behavior in unsafe code.

However, miri chokes on the workers because they make some unsupported syscalls, even with isolation mode disabled.

Meanwhile, coverage for the workers would be tricky to set up and it does not seem very profitable to do so, as the workers do not have a lot of code and it's not very branchy.

@mrcnski mrcnski added T0-node This PR/Issue is related to the topic “node”. T10-tests This PR/Issue is related to tests. labels Dec 18, 2023
@mrcnski mrcnski self-assigned this Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T0-node This PR/Issue is related to the topic “node”. T10-tests This PR/Issue is related to tests.
Projects
Status: Backlog
Development

No branches or pull requests

1 participant