On Thu, Oct 06 2022, Phillip Wood wrote: > On 06/10/2022 16:56, Ævar Arnfjörð Bjarmason wrote: >> On Thu, Oct 06 2022, Phillip Wood via GitGitGadget wrote: >> >>> From: Phillip Wood <phillip.wood@xxxxxxxxxxxxx> >>> >>> Many failing tests use grep, this commit converts a sample to use >>> test_todo(). As POSIX specifies that a return code greater than one >>> indicates an error rather than a failing match we take care not the >>> hide that. >> Ah, so on the one hand this gives me second thoughts about my stance >> in[1], i.e. if we just allowed any command we wouldn't be forced to add >> these sorts of special-cases. >> Although, we could also allow any command, and just add smartness >> for >> ones we know about, e.g. "grep". >> But I do find doing this to be weirdly inconsistent, i.e. caring >> about >> the difference between these two: >> $ grep blah README.md; echo $? >> 1 >> $ grep blah README.mdx; echo $? >> grep: README.mdx: No such file or directory >> 2 > > The intent was to catch bad options, not missing files (i.e. we don't > want test_todo to hide a failure from "grep --invalid-option"). We > could check the file exists and skip running grep if it does not > (hopefully the test wont be grepping multiple files in a single > command) It returns the same exit code for missing files and bad options, so I don't think this plan will work. I.e. I (in my initial series) wanted to have something where we declared what the behavior was right now, *and* what it should be. But some of our tests are wishy-washing "I wish this worked", so: test_todo git some-new-cmd && # should write "unicorn" to a new foo.txt test_todo grep unicorn foo.txt Won't do what you expect? >> Is basically why I took the approach I did in my [2], i.e. to force us >> to positively assert *what* the bad behavior should be. > > That is what made the end result so hard to use though > > test_todo \ > --want "test_must_fail git" \ > --reset "git reset --hard" \ > --expect git \ > -- \ > rm d/f && > > is not exactly readable. Yes, indeed:) Anyway, my just-sent https://lore.kernel.org/git/221006.86v8owr986.gmgdl@xxxxxxxxxxxxxxxxxxx/ goes into that. I think a "test_todo" should either be a "strictly declare stuff", or a "YOLO this" where we just detect segfaults. But per the above having it be some mix of the two is just confusing, i.e. to extend the example above: test_todo git some-new-cmd && test_todo test_path_exists foo.txt && test_todo grep unicorn foo.txt Won't "work", because the "test_path_exists" isn't strict, but your "grep" is. So I think whatever "test_todo" does it should be picking one or the other.
