t: validate 'test_i18ngrep's parameters
authorSZEDER Gábor <szeder.dev@gmail.com>
Thu, 8 Feb 2018 15:56:55 +0000 (16:56 +0100)
committerJunio C Hamano <gitster@pobox.com>
Thu, 8 Feb 2018 18:54:27 +0000 (10:54 -0800)
Some of the previous patches in this series fixed bogus
'test_i18ngrep' invocations:

- Two invocations where the tested git command's standard output is
directly piped into 'test_i18ngrep'. While convenient, this is an
antipattern, because the pipe hides the git command's exit code,
and the test could continue even if the command exited with error.

- Two invocations that had neither a filename parameter nor anything
piped into their standard input, yet both managed to remain
unnoticed for years. A third similarly bogus invocation is
currently lurking in 'pu' for a couple of weeks now.

Prevent similar mistakes in the future by validating 'test_i18ngrep's
parameters requiring that

- The last parameter names an existing file to be read, effectively
forbidding piping into 'test_i18ngrep'.

Note that this change will also forbid cases where 'test_i18ngrep'
would legitimately read its standard input, e.g. when its standard
input is redirected from a file, or when a git command's standard
output is first written to an intermediate file, which is then
preprocessed by a non-git command before the results are piped
into 'test_i18ngrep'. See two of the previous patches for the
only such cases we had in our test suite. However, reliably
preventing the piping antipattern is arguably more important than
supporting these cases, which can be easily worked around by
opening the file directly or using an intermediate file anyway.

- There are at least two parameters, not including the optional '!'
to negate the pattern. This ought to catch corner cases when
'test_i18ngrep' looks for the name of an existing file on its
standard input; the above check would miss this case becase the
filename as pattern would be the last parameter.

Note that this is not quite perfect, as it doesn't account for any
'grep --options' given as parameters. However, doing so would be
far too complicated, considering that patterns can start with
dashes as well, and in the majority of the cases we don't use any
such options anyway.

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
t/test-lib-functions.sh
index 92ed0293712e8875783037a32cb21252087bc589..64f793e3d73e6f7e0ab2d813487efb0b237489d0 100644 (file)
@@ -719,6 +719,18 @@ test_i18ncmp () {
 # under GETTEXT_POISON this pretends that the command produced expected
 # results.
 test_i18ngrep () {
+       eval "last_arg=\${$#}"
+
+       test -f "$last_arg" ||
+       error "bug in the test script: test_i18ngrep requires a file" \
+             "to read as the last parameter"
+
+       if test $# -lt 2 ||
+          { test "x!" = "x$1" && test $# -lt 3 ; }
+       then
+               error "bug in the test script: too few parameters to test_i18ngrep"
+       fi
+
        if test -n "$GETTEXT_POISON"
        then
            : # pretend success