Advanced testing for `rustworkx-core`
What is the expected enhancement?
Currently, we test rustworkx with unit tests in Python and we test rustworkx-core with unit tests and doctests in Rust.
Because rustworkx-core is written completely in Rust and has a simpler testing setup (no need for tools that understand two languages), we could run some more advanced tests for it:
- Miri: find undefined behaviour in Rust
-
Mutation Testing: use
cargo-mutantsto find lines of code that are covered by tests but do not cause test failures -
Fuzzing: use
cargo-fuzz, most likely to find panics -
Property-based testing: use
quickcheckto generate randomized tests that are stronger
Also, Miri is known to cause issues to some dependencies we have in Cargo.lock so check https://github.com/Qiskit/qiskit/blob/e4048320db76dcd738a36f5376628855d20e97b3/.github/workflows/miri.yml#L28 to see some patches Qiskit uses.
Hey @IvanIsCoding , can I pick this one up and add the tests?
Congratulations, after #1407 was merged we have our first fuzz target.
Some action items:
- add more fuzzers for popular methods
- add a GithubActions workflows that installed a pinned rust nightly version, C++ toolchain and builds the fuzzer
- Join https://github.com/google/oss-fuzz maybe?
should I add more fuzz targets (and work on the other action items you mentioned) or go ahead with building a basic structure for the other types of tests?
You can focus on adding more targets. I think that is the harder part. I will figure out how to run it
Got it!
Hey @IvanIsCoding, now that basic fuzzing is done, should now focus on adding other kinds of tests?
@Krishn1412 I think the next best thing would be property based tests with quickcheck.
I will try to find some examples later from other packages.
It looks like petgraph already implements some quickcheck utilities we could use: https://github.com/petgraph/petgraph/blob/8439ee24335b8ffd20fb6c44ec3fcd2cc108917c/src/quickcheck.rs#L29
I had a doubt here. If we use petgraph's quickcheck utilities to create arbitrary graphs with random inputs and ensure our properties hold, isn't that similar to fuzzing?
I had a doubt here. If we use petgraph's quickcheck utilities to create arbitrary graphs with random inputs and ensure our properties hold, isn't that similar to fuzzing?
Excellent question. It is indeed similar, but I think this blog summarizes the differences: https://www.tedinski.com/2018/12/11/fuzzing-and-property-testing.html.
In short, some tests are OK with valid input. Eventually, fuzzing could run for hours and find a bug with randomized input.
For property based testing, we'll need to be a little bit more clever. That's why I'd argue that https://github.com/Qiskit/rustworkx/pull/1429 should have been property-based testing (or just a regular unit test to be honest). Meanwhile, fuzzing was valuable for shortest path as we need to assert the triangle property on any input.
Got it! I will start out with property-based checks for special deterministic graph then. Thanks for the detailed explanation.
Hey @IvanIsCoding, I have added many property based quickchecks now. Should we move to something else now?
@Krishn1412 I think your work is complete! We have a good coverage now.
For testing tickets, there’s #1316.
For non-testing tickets, #1466 and #804 can be interesting things to work on. Any open issue works, but those two are interesting feature requests.
Sure thing @IvanIsCoding, I'll pickup and start with 1446. Also could I ping you on LinkedIn for some career queries? Any help would be great, TIA!
@Krishn1412 sounds good to me! Feel free to ping