tests: add fuzzing to our test suite#52
Conversation
|
can the fuzzer be used for both C++ and Rust impl? |
|
not one fuzzer, but I think having two is fair |
|
the cpp fuzzer is not as fun though, because it does not immediately crash and burn.. The rust one is (for a fuzzer) basically made of very good firewood soaked in gasoline. |
|
hehe. I do think we need the same api for both ;) |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| steps: | ||
| - uses: actions/checkout@v6 | ||
| with: {persist-credentials: false, submodules: recursive} |
There was a problem hiding this comment.
The fuzz job is missing Rust caching that is present in other CI jobs. Adding Swatinem/rust-cache@v2 (like in the test, test-nightly, and coverage jobs) would speed up builds by caching dependencies and compilation artifacts. This is especially important for fuzzing which may run frequently.
| with: {persist-credentials: false, submodules: recursive} | |
| with: {persist-credentials: false, submodules: recursive} | |
| - if: github.event_name != 'release' && github.event_name != 'workflow_dispatch' | |
| uses: Swatinem/rust-cache@v2 |
| use std::io::Cursor; | ||
| use std::num::NonZeroU32; | ||
|
|
||
| use fastpfor::rust::{DEFAULT_PAGE_SIZE, FastPFOR, Integer}; |
There was a problem hiding this comment.
The Integer trait is imported but never used in this fuzz target. Remove it from the import statement to clean up unused imports.
| use fastpfor::rust::{DEFAULT_PAGE_SIZE, FastPFOR, Integer}; | |
| use fastpfor::rust::{DEFAULT_PAGE_SIZE, FastPFOR}; |
| - run: cargo fuzz build --target x86_64-unknown-linux-gnu ${{ matrix.fuzz_target }} | ||
| - run: cargo fuzz run --target x86_64-unknown-linux-gnu ${{ matrix.fuzz_target }} -- -max_total_time=${{ env.FUZZ_TIME }} |
There was a problem hiding this comment.
The cargo fuzz commands are being run from the repository root, but they should be run from the fuzz directory. cargo-fuzz expects to be run from the directory containing the fuzz Cargo.toml. Add working-directory: fuzz to these steps or use cd fuzz && cargo fuzz ... to ensure the commands run in the correct directory.
| - run: cargo fuzz build --target x86_64-unknown-linux-gnu ${{ matrix.fuzz_target }} | |
| - run: cargo fuzz run --target x86_64-unknown-linux-gnu ${{ matrix.fuzz_target }} -- -max_total_time=${{ env.FUZZ_TIME }} | |
| - run: cargo fuzz build --target x86_64-unknown-linux-gnu ${{ matrix.fuzz_target }} | |
| working-directory: fuzz | |
| - run: cargo fuzz run --target x86_64-unknown-linux-gnu ${{ matrix.fuzz_target }} -- -max_total_time=${{ env.FUZZ_TIME }} | |
| working-directory: fuzz |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This currently fails fairly fast, but that is expected.
Here is the minimized reproduciton: