Re: [GSoC][PATCH v6] parse-options: do not show usage upon invalid option value
From: Junio C Hamano <hidden>
Date: 2018-03-21 18:22:34
Paul-Sebastian Ungureanu [off-list ref] writes:
quoted hunk
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.