This is the second version of 'sg/test-i18ngrep'. To recap, this patch series fixes a couple of bogus 'test_i18ngrep' invocations (patches 1-4), tries to prevent similar bugs in the future (patch 8), teaches 'test_i18ngrep' to be more informative on failure (patch 9), and a bit of cleanups in between (patches 5-7). Changes since the previous version [1]: - Use Junio's "last parameter must be file" suggestion instead of trying to read stdin in patch 8. - Squashed together the patches validating 'test_i18ngrep's parameters (patches 8 and 9), in the hope that this way I can better explain that the two checks are not redundant but complement each other. - Followed Simon's suggestion and dropped the now unnecessary curly brackets in patch 2. - Dropped a subshell in the last patch. I initially used it to prevent the variable $f from leaking into the tests, since we can't use the 'local' keyword (yet), but other test helper function don't seem to care. - Fixed the placements of single quotes and '!' in error messages and redirected one more error message to stderr in the last patch. - Fixed a couple of typos in commit messages (the one Eric pointed out, but later noticed maybe 2-3 more). [1] - https://public-inbox.org/git/20180126123708.21722-1-szeder.dev@xxxxxxxxx/T/ SZEDER Gábor (9): t5541: add 'test_i18ngrep's missing filename parameter t5812: add 'test_i18ngrep's missing filename parameter t6022: don't run 'git merge' upstream of a pipe t4001: don't run 'git status' upstream of a pipe t5510: consolidate 'grep' and 'test_i18ngrep' patterns t5536: let 'test_i18ngrep' read the file without redirection t: move 'test_i18ncmp' and 'test_i18ngrep' to 'test-lib-functions.sh' t: validate 'test_i18ngrep's parameters t: make 'test_i18ngrep' more informative on failure t/t4001-diff-rename.sh | 11 ++++++--- t/t5510-fetch.sh | 9 +++----- t/t5536-fetch-conflicts.sh | 2 +- t/t5541-http-push-smart.sh | 2 +- t/t5812-proto-disable-http.sh | 5 +--- t/t6022-merge-rename.sh | 6 +++-- t/test-lib-functions.sh | 54 +++++++++++++++++++++++++++++++++++++++++++ t/test-lib.sh | 26 --------------------- 8 files changed, 72 insertions(+), 43 deletions(-) -- 2.16.1.158.ge6451079d diff --git a/t/t5812-proto-disable-http.sh b/t/t5812-proto-disable-http.sh index 226a4920cd..872788ac8c 100755 --- a/t/t5812-proto-disable-http.sh +++ b/t/t5812-proto-disable-http.sh @@ -20,9 +20,7 @@ test_expect_success 'curl redirects respect whitelist' ' test_must_fail env GIT_ALLOW_PROTOCOL=http:https \ GIT_SMART_HTTP=0 \ git clone "$HTTPD_URL/ftp-redir/repo.git" 2>stderr && - { - test_i18ngrep -E "(ftp.*disabled|your curl version is too old)" stderr - } + test_i18ngrep -E "(ftp.*disabled|your curl version is too old)" stderr ' test_expect_success 'curl limits redirects' ' diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 1f1d89d7ad..d936ecc0a5 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -719,9 +719,11 @@ test_i18ncmp () { # under GETTEXT_POISON this pretends that the command produced expected # results. test_i18ngrep () { - ( read line ) && - error "bug in the test script: data on test_i18ngrep's stdin;" \ - "perhaps a git command's output is piped into it?" + 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 ; } @@ -740,21 +742,20 @@ test_i18ngrep () { shift ! grep "$@" && return 0 - echo >&2 "error: grep '! $@' did find a match in:" + echo >&2 "error: '! grep $@' did find a match in:" else grep "$@" && return 0 - echo >&2 "error: grep '$@' didn't find a match in:" + echo >&2 "error: 'grep $@' didn't find a match in:" fi - ( - eval "f=\"\${$#}\"" - if test -s "$f" - then - cat >&2 "$f" - else - echo "<File '$f' is empty>" - fi - ) + + if test -s "$last_arg" + then + cat >&2 "$last_arg" + else + echo >&2 "<File '$last_arg' is empty>" + fi + return 1 }