]> granicus.if.org Git - git/commitdiff
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