> I assumed the variable reuse was related to the bug.
The patch submitter isn't making that assumption, even after submitting the initial fix:
>> Did you intend to reuse "i" here?
> No idea. ;-)
Saam later found out through playing with the test case that your assumption was correct, the variable reuse was related to the bug. Other weird constructs also questioned, on the other hand, were not related to the bug - so I'd say you got lucky :)
> The thing is, test cases are often ugly, by nature.
All the more reason to keep them only as ugly as they need to be IMO. Being able to understand what's being tested is useful, yes? Seeing e.g. what corner cases and unexpected inputs are already tested helps me identify what corner cases and unexpected inputs aren't already tested, or might be missing.
(This said, I'm generally perfectly okay with not cleaning up fuzz generated tests. They're inherently write-onlyish in nature.)
The patch submitter isn't making that assumption, even after submitting the initial fix:
>> Did you intend to reuse "i" here?
> No idea. ;-)
Saam later found out through playing with the test case that your assumption was correct, the variable reuse was related to the bug. Other weird constructs also questioned, on the other hand, were not related to the bug - so I'd say you got lucky :)
> The thing is, test cases are often ugly, by nature.
All the more reason to keep them only as ugly as they need to be IMO. Being able to understand what's being tested is useful, yes? Seeing e.g. what corner cases and unexpected inputs are already tested helps me identify what corner cases and unexpected inputs aren't already tested, or might be missing.
(This said, I'm generally perfectly okay with not cleaning up fuzz generated tests. They're inherently write-onlyish in nature.)