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 round-trip parser fuzzers #57

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

addisoncrump
Copy link

Partially addresses #14. More specialized fuzzing for e.g. correctness is necessary.

@addisoncrump addisoncrump marked this pull request as draft July 29, 2024 13:53
@addisoncrump
Copy link
Author

Realized we get some round-trip tests for free here. Quickly changing these over.

@addisoncrump
Copy link
Author

@BurntSushi This is starting to find some things, mostly inconsistencies with printing and parsing. Do you want me to investigate these and raise issues, or let you experiment with this? I need to leave this for today but can come back to it later this week (Thursday and beyond).

@addisoncrump addisoncrump marked this pull request as ready for review July 29, 2024 14:05
@BurntSushi
Copy link
Owner

If by inconsistency you mean "printing a parsed string does not return the same string back," then yeah, that's very much intended. If it's not that, then yeah, could be worth some issues. Maybe start with just one or two if there's a way to capture the general "shape" of the failures if that makes sense.

@BurntSushi
Copy link
Owner

I can also take a look as well.

@addisoncrump
Copy link
Author

addisoncrump commented Jul 29, 2024

By inconsistency I mean, "We have some existing date structure and print it. Reparsing it back to date structure and comparing the date structures shows not equal."

For strtime it's a special case, I compare (parse, unparse) with (parse, unparse, parse, unparse) results since BrokenDownTime doesn't impl PartialEq.

@addisoncrump
Copy link
Author

addisoncrump commented Jul 29, 2024

Maybe start with just one or two if there's a way to capture the general "shape" of the failures if that makes sense.

The example that gets spit out for temporal is: Pt843517081,1H

It is parsed, the unparsed, but then the test panics while parsing the first unparsed result because we expect that all unparsed items are parseable. The intermediate unparsed value is: PT175307616h10518456960m1774446656760s.

@addisoncrump addisoncrump changed the title Add simple parser fuzzers Add round-trip parser fuzzers Jul 29, 2024
@BurntSushi
Copy link
Owner

By inconsistency I mean, "We have some existing date structure and print it. Reparsing it back to date structure and comparing the date structures shows not equal."

Yes, that sounds like it should be investigated!

And any panics in the parsing should definitely be considered bugs.

Probably BrokenDownTime should implement Eq, yeah.

When you get a chance, and if it's easier, filing just one issue is fine. And feel free to take your time. If there's a timeout, I'll pickup the fuzzing work from this great start. :)

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.

2 participants