Paul-Sebastian Ungureanu <ungureanupaulsebastian@xxxxxxxxx> writes: > diff --git a/t/t0041-usage.sh b/t/t0041-usage.sh > new file mode 100755 > index 000000000..ac96bc3b9 > --- /dev/null > +++ b/t/t0041-usage.sh > @@ -0,0 +1,107 @@ > +#!/bin/sh > + > +test_description='Test commands behavior when given invalid argument value' > + > +. ./test-lib.sh > + > +test_expect_success 'setup ' ' > + test_commit "v1.0" > +' > + > +test_expect_success 'tag --contains <existent_tag>' ' > + git tag --contains "v1.0" 1>actual 2>actual.err && It is not wrong per-se, but >redirection without a number redirects fd#1, so "1>actual" is an unusual way to spell ">actual". > + grep "v1.0" actual && > + test_line_count = 0 actual.err > +' > + > +test_expect_success 'tag --contains <inexistent_tag>' ' > + test_must_fail git tag --contains "notag" 1>actual 2>actual.err && > + test_line_count = 0 actual && > + test_i18ngrep "error" actual.err && > + test_must_fail test_i18ngrep "usage" actual.err test_must_fail mustn't be used like that for two reasons - It is to be used for "git" stuff, because it knows failing with segfault and other failure modes are _wrong_ and should not be happy even if the command "fail"ed. We however do not expect commands that are not git (e.g. test_i18ngrep) to require special casing of the failure modes. - test_i18ngrep pretends to always "have found match" when running under GETTEXT_POISON build, so it will pretend that usage exists in actual.error. test_must_fail will then say "oops, the string 'usage' shouldn't appear in the output but it did", which is not what you want. Perhaps test_i18ngrep ! "usage" actual.err is what you want to say here instead. In any case, the tests got a lot cleaner compared to the previous round, and I feel that this patch is "getting there" ;-) Thanks for working on it.
