Fuzzer: Use a directory for important fuzz testcases#6297
Fuzzer: Use a directory for important fuzz testcases#6297kripken merged 7 commits intoWebAssembly:mainfrom
Conversation
tlively
left a comment
tlively
left a comment
There was a problem hiding this comment.
This seems reasonable, although one thing to note is that that may make reproducibility slightly worse since the fuzz results will now depend on the contents of the fuzz directory.
scripts/fuzz_opt.py
Outdated
| lit_tests = shared.get_tests(shared.get_test_dir('lit'), test_suffixes, recursive=True) | ||
| all_tests = core_tests + passes_tests + spec_tests + wasm2js_tests + lld_tests + unit_tests + lit_tests | ||
|
|
||
| fuzz_cases = shared.get_tests(shared.get_test_dir('fuzz'), test_suffixes, recursive=True) |
There was a problem hiding this comment.
This won't break if the fuzz directory doesn't exist, will it?
There was a problem hiding this comment.
Good point, I'll add a readme.txt in that directory to force it to exist in git.
|
I'm not strongly opinionated, but wouldn't it be simpler test directories are categorized based on their kinds ( |
I'll clarify in a comment more what I intend here, I think I wasn't clear enough. The new directory is for manually adding new fuzz testcases for the fuzzer - we wouldn't move any existing testcases there, and probably would not even commit anything there. But when fuzzing locally the user can drop a few files in there that get coverage that way. |
|
That makes sense. Thanks for the explanation! |
|
After thinking a little I think maybe it is clearer to make the directory not exist and document the feature as being for local changes, that is, now the docs say "If you want to fuzz some files with high importance, create 'fuzz' and put the files there". That's the intended use case so we might as well describe it that way. |
|
The advantage of checking in the directory with a readme is that it makes this feature and its documentation more discoverable, but either way sgtm. |
|
Makes sense, maybe the important thing is to keep the fuzz dir parallel to the test dir, so it's clear normal testcases do not go there. I did that now and clarified in the readme there. |
|
Also documented in https://github.com/WebAssembly/binaryen/wiki/Fuzzing#helper-scripts |
Users can put files in ./fuzz and they will be fuzzed with high priority. Docs in source and https://github.com/WebAssembly/binaryen/wiki/Fuzzing#helper-scripts
3 participants
This removes the manual list inside the fuzzer with a simple directory that we
scan. The manual list was not well-maintained and nothing there really
mattered enough to keep. Instead, users can place files in
/test/fuzz/that theyconsider important.