Re: [PATCH 1/1] Mark messages for translations

[Jeff King <peff@peff.net>]

On Fri, Feb 09, 2018 at 11:15:06AM -0800, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> >> -	if ! grep "Invalid gitfile format" .err
> >> +	if ! test_i18ngrep "invalid gitfile format" .err
> >
> > Shouldn't this rather be like so instead?
> >
> > 	if test_i18ngrep ! "invalid gitfile format" .err
> >
> > Ditto for the other negated use of test_i18ngrep we see in the same
> > file in this patch.
> 
> Sorry, my thinko.  These two ones want to be written in the patch
> as-is.

Yes, I think so, but we may want to avoid this anti-pattern (since
usually "! test_i18ngrep" is a sign of something wrong. It seems like
these tests are doing more manual reporting work than is necessary, and
could just be relying on helpers to report errors.

Something like the patch below, though I'm not sure if we'd want to
leave it as "grep" (if applying on master), or have "test_i18ngrep" in
the preimage (if basing on top of Alexander's patch).

-Peff

---
 t/t0002-gitfile.sh | 54 +++++++----------------------------
 1 file changed, 11 insertions(+), 43 deletions(-)

diff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh
index 9670e8cbe6..74b7307997 100755
--- a/t/t0002-gitfile.sh
+++ b/t/t0002-gitfile.sh
@@ -10,15 +10,6 @@ objpath() {
 	echo "$1" | sed -e 's|\(..\)|\1/|'
 }
 
-objck() {
-	p=$(objpath "$1")
-	if test ! -f "$REAL/objects/$p"
-	then
-		echo "Object not found: $REAL/objects/$p"
-		false
-	fi
-}
-
 test_expect_success 'initial setup' '
 	REAL="$(pwd)/.real" &&
 	mv .git "$REAL"
@@ -26,30 +17,14 @@ test_expect_success 'initial setup' '
 
 test_expect_success 'bad setup: invalid .git file format' '
 	echo "gitdir $REAL" >.git &&
-	if git rev-parse 2>.err
-	then
-		echo "git rev-parse accepted an invalid .git file"
-		false
-	fi &&
-	if ! grep "Invalid gitfile format" .err
-	then
-		echo "git rev-parse returned wrong error"
-		false
-	fi
+	test_must_fail git rev-parse 2>.err &&
+	test_i18ngrep "Invalid gitfile format" .err
 '
 
 test_expect_success 'bad setup: invalid .git file path' '
 	echo "gitdir: $REAL.not" >.git &&
-	if git rev-parse 2>.err
-	then
-		echo "git rev-parse accepted an invalid .git file path"
-		false
-	fi &&
-	if ! grep "Not a git repository" .err
-	then
-		echo "git rev-parse returned wrong error"
-		false
-	fi
+	test_must_fail git rev-parse 2>.err &&
+	test_i18ngrep "Not a git repository" .err
 '
 
 test_expect_success 'final setup + check rev-parse --git-dir' '
@@ -60,7 +35,7 @@ test_expect_success 'final setup + check rev-parse --git-dir' '
 test_expect_success 'check hash-object' '
 	echo "foo" >bar &&
 	SHA=$(cat bar | git hash-object -w --stdin) &&
-	objck $SHA
+	test_path_is_file "$REAL/objects/$(objpath $SHA)"
 '
 
 test_expect_success 'check cat-file' '
@@ -69,29 +44,22 @@ test_expect_success 'check cat-file' '
 '
 
 test_expect_success 'check update-index' '
-	if test -f "$REAL/index"
-	then
-		echo "Hmm, $REAL/index exists?"
-		false
-	fi &&
+	test_path_is_missing "$REAL/index" &&
 	rm -f "$REAL/objects/$(objpath $SHA)" &&
 	git update-index --add bar &&
-	if ! test -f "$REAL/index"
-	then
-		echo "$REAL/index not found"
-		false
-	fi &&
-	objck $SHA
+	test_path_is_file "$REAL/index" &&
+	test_path_is_file "$REAL/objects/$(objpath $SHA)" &&
+	false
 '
 
 test_expect_success 'check write-tree' '
 	SHA=$(git write-tree) &&
-	objck $SHA
+	test_path_is_file "$REAL/objects/$(objpath $SHA)"
 '
 
 test_expect_success 'check commit-tree' '
 	SHA=$(echo "commit bar" | git commit-tree $SHA) &&
-	objck $SHA
+	test_path_is_file "$REAL/objects/$(objpath $SHA)"
 '
 
 test_expect_success 'check rev-list' '