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.

Sign up for GitHub

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

Jump to bottom

Reenable CI fuzzing of pull requests #1596

Merged
merged 3 commits into from
Sep 20, 2024
Merged

Reenable CI fuzzing of pull requests #1596

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
25893ed
Revert "Looks like fuzzing is failing permanently again"
EliahKagan Aug 23, 2024
aafa8b1
Try manully specifying old serde{,_derive,_json} in `Cargo.toml`
EliahKagan Sep 15, 2024
39cab61
Revert "Try manully specifying old serde{,_derive,_json} in `Cargo.to…
EliahKagan Sep 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .github/workflows/cifuzz.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
name: CIFuzz
on:
pull_request:
branches: [ main ]
workflow_dispatch:
jobs:
Fuzzing:
runs-on: ubuntu-latest
steps:
- name: Build Fuzzers
id: build
uses: google/oss-fuzz/infra/cifuzz/actions/build_fuzzers@master
with:
oss-fuzz-project-name: 'gitoxide'
language: rust
- name: Run Fuzzers
uses: google/oss-fuzz/infra/cifuzz/actions/run_fuzzers@master
with:
oss-fuzz-project-name: 'gitoxide'
language: rust
fuzz-seconds: 600
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
fuzz-seconds: 600
fuzz-seconds: 300

This should bring the Job duration down to ~15 minutes, making it run about as long as the Windows Job, which seems slowest now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The oss-fuzz documentation recommends against decreasing fuzz-seconds below 600:

  1. Set the value of fuzz-seconds. The longest time that the project maintainers are acceptable with should be used. This value should be at minimum 600 seconds and scale with project size.

I don't know that this necessarily means we shouldn't use a smaller value, but I would want to understand why 600 seconds is the recommended minimum before doing so. I am also unclear on whether smaller values are disregarded and treated the same as 600 seconds, or honored but not recommended, possibly due to diminishing value.

There doesn't seem to be a specific explanation for this in the documentation or elsewhere. This was added along with various other material to the documentation in google/oss-fuzz#3572. I can inquire at https://github.com/google/oss-fuzz/discussions about the reason, if decreasing it is something you think is worth pursuing and you want my specific input on it. This might be a way to figure out how best to tune it.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking into this!

Then I think I will… instead not enforce CI-Fuzzing to finish before merging.
If it fails later, I will still get an email.

When working on a parser it's possible to just wait for it to finish just to be sure there is no obvious issue that a fuzzer would catch within a couple of missing.

- name: Upload Crash
uses: actions/upload-artifact@v4
if: failure() && steps.build.outcome == 'success'
with:
name: artifacts
path: ./out/artifacts
Loading