> Is there a reason this isn't indented properly? Also, the formatting wrt spaces is inconsistent. Did you intend to reuse "i" as the loop variable for the inner loop? Could you add comments as to what the test is doing?
... Let me guess. This is manager input. (I know nothing about your structure, this is just so... manager-flavored. Who the fuck cares about the indentation and spacing for a test case, maybe that's part of the input causing the failure? Or perhaps the forum software ate it. Regardless, dumb.)
> ... Let me guess. This is manager input. (I know nothing about your structure, this is just so... manager-flavored. Who the fuck cares about the indentation and spacing for a test case, maybe that's part of the input causing the failure? Or perhaps the forum software ate it. Regardless, dumb.)
It seems clear Benjamin was originally operating under the erroneous assumption that this was written by a human, which I'd consider an honest and reasonable enough mistake, requiring no managerial boogy men.
If we make the same mistake: While it's a minor quibble given the size of the snippet, there's no less than three scopes with the same indentation level (global, function, for-loop) already - which, if a habit, is one you want to correct before someone tries to check in a few hundred lines of non-test code in the same style.
Fixing the indentation might be trivial enough with any autoformatter (since this isn't python), but cleaning up the variable shadowing is going to get a bit nastier - likely introducing (or fixing) bugs if done manually.
Proactively trying to prevent such problems seems like a reasonable response, not a dumb one, to me.
I assumed the variable reuse was related to the bug.
The thing is, test cases are often ugly, by nature. They are often unexpected inputs, corner-cases, unexpected (but legal) use of variables or indentation or anything really. Bad syntax or bad variable use would potentially fall under that category.
> 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.)
Having a consistent style actually helps a lot in large code bases. But these days, this can be automated with clang-format, so I agree that it's a bit silly that this is checked manually.
The bit about reusing i is a good catch though, that's not just a code-style thing but a real potential bug.
> Is there a reason this isn't indented properly? Also, the formatting wrt spaces is inconsistent. Did you intend to reuse "i" as the loop variable for the inner loop? Could you add comments as to what the test is doing?
... Let me guess. This is manager input. (I know nothing about your structure, this is just so... manager-flavored. Who the fuck cares about the indentation and spacing for a test case, maybe that's part of the input causing the failure? Or perhaps the forum software ate it. Regardless, dumb.)