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

Add fuzz tests #137

Merged
merged 34 commits into from
Dec 19, 2024
Merged

Add fuzz tests #137

merged 34 commits into from
Dec 19, 2024

Conversation

anupsv
Copy link
Contributor

@anupsv anupsv commented Sep 20, 2024

Fixes Issue

Fixes #

Changes proposed

Screenshots (Optional)

Note to reviewers

@anupsv anupsv requested review from bxue-l2 and samlaf September 24, 2024 19:47
@anupsv anupsv marked this pull request as ready for review September 24, 2024 19:53
@anupsv anupsv requested a review from epociask September 24, 2024 20:08
e2e/server_fuzz_test.go Outdated Show resolved Hide resolved
@samlaf
Copy link
Collaborator

samlaf commented Dec 18, 2024

@anupsv why did we never merge this? Do you want to rebase so we can merge? Hopefully its not too gnarly

@anupsv
Copy link
Contributor Author

anupsv commented Dec 19, 2024

@anupsv why did we never merge this? Do you want to rebase so we can merge? Hopefully its not too gnarly

That's a good question. Idk why we didn't merge it. I'll rebase and merge.

Makefile Outdated Show resolved Hide resolved
e2e/main_test.go Outdated Show resolved Hide resolved
e2e/server_fuzz_test.go Show resolved Hide resolved
e2e/server_test.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@samlaf samlaf left a comment

Choose a reason for hiding this comment

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

LGTM. Haven't actually read through the tests but let's just merge and if they are flaky we can deal with them later. Better to have more protection than less.

Things to look for in the future:

  • don't think we need the FUZZ=true env var, go fuzzer (https://pkg.go.dev/testing) seems to only run fuzz testing when you add the -fuzz
  • 15m seems a bit long if our other tests run in <10 mins. We might want to bring this down.

@anupsv anupsv merged commit d89658b into main Dec 19, 2024
9 checks passed
@anupsv anupsv deleted the add-fuzz-tests branch December 19, 2024 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants