On Thu, Oct 06 2022, Phillip Wood via GitGitGadget wrote: > From: Phillip Wood <phillip.wood@xxxxxxxxxxxxx> > > For simplicity test_todo() allows verbose to precede any valid > command. As POSIX specifies that a return code greater than one is an > error rather than a failed test we take care not to hide that. > > I'm in two minds about this patch. Generally it is better to use one > of our test helpers such as test_cmp() rather than calling test > directly. There are so few instances of test being used within > test_expect_failure() (the conversions here are not exhaustive but > there are not many more) that it would probably be better to convert > the tests by using the appropriate helper rather than supporting > calling test as the command to test_todo(). I think that we might want to salvage parts of this, but we really shouldn't be spending review time on carrying forward a bad pattern that hides segfaults. I.e. whatever we do about "test_todo"'s interaction with "test" let's first change things like... > -test_expect_failure CASE_INSENSITIVE_FS 'add (with different case)' ' > +test_expect_success CASE_INSENSITIVE_FS 'add (with different case)' ' > git reset --hard initial && > rm camelcase && > echo 1 >CamelCase && > @@ -108,7 +108,7 @@ test_expect_failure CASE_INSENSITIVE_FS 'add (with different case)' ' > git ls-files >tmp && > camel=$(grep -i camelcase tmp) && > test $(echo "$camel" | wc -l) = 1 && > - test "z$(git cat-file blob :$camel)" = z1 > + test_todo test "z$(git cat-file blob :$camel)" = z1 ...this to e.g.: echo z1 >expect && git cat-file blob :$camel >actual && test_cmp expect actual Or whatever, then let's see if migrating "verbose" is worthwhile, in the post-image you end up with no real users of it, only your tests. I've wanted to just remove it for a while, all its users seem to be either bad uses like that, or we'd get much better bang for the buck out of it by having a t/verbose-bin/ or whatever, which would just wrap arbitrary commands like "grep" and the like (i.e. ones where we could provide useful context).
