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

Update fuzz targets #697

Merged
merged 6 commits into from
Feb 1, 2023
Merged

Update fuzz targets #697

merged 6 commits into from
Feb 1, 2023

Conversation

ericcornelissen
Copy link
Owner

Relates to #361, #688

Summary

Update fuzz targets in various ways:

  • Refactor for performance. (169c9c6)
  • Refactor for improved readability. (343c9db)
  • Handle errors explicitly. (eb57fe4)
  • Test non-sync versions of execFileSync, execSync, and spawnSync. (ccdc4b6)
  • Don't test multiple args. (5ab2740)

Should make for easier overall reading and also aligns all fuzz targets
with the style of `exec.test.cjs`.
Since, per [1], it's not actually guaranteed these work the same.

--
1. nodejs/node#43333
I do not believe these currently provide significant value. They don't
seem to test anything special about multiple arguments. Plus the method
by which these convert the buffer into multiple args leaves a lot to be
desired. An upside is that fuzzing should become more efficient as less
cycles are wasted on testing something with little to no value.
@ericcornelissen ericcornelissen added the test Relates to testing label Jan 31, 2023
@ericcornelissen ericcornelissen merged commit 15c805b into main Feb 1, 2023
@ericcornelissen ericcornelissen deleted the fuzz-target-improvements branch February 1, 2023 00:02
@ericcornelissen ericcornelissen added fuzz Relates to fuzzing and removed test Relates to testing labels Jul 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fuzz Relates to fuzzing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant