Subject: [PATCH v2 0/9] 'test_i18ngrep'-related fixes and improvements

From: SZEDER Gábor <[email-masked]>
Date: Thu, 8 Feb 2018 16:56:47 +0100

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/[email-masked]/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
 }