Thread (18 messages) flat view 18 messages, 4 authors, 2016-06-15

Re: [PATCH 1/2] t8005: avoid grep on non-ASCII data

From: Jeff King <hidden>
Date: 2016-06-15 23:08:20
Subsystem: the rest · Maintainer: Linus Torvalds

Possibly related (same subject, not in this thread)

On Sun, Feb 21, 2016 at 04:01:27PM -0500, Eric Sunshine wrote:
On Sun, Feb 21, 2016 at 12:32 PM, John Keeping [off-list ref] wrote:
quoted
GNU grep 2.23 detects the input used in this test as binary data so it
does not work for extracting lines from a file.  We could add the "-a"
option to force grep to treat the input as text, but not all
implementations support that.  Instead, use sed to extract the desired
lines since it will always treat its input as text.

While touching these lines, modernize the test style to avoid hiding the
exit status of "git blame" and remove a space following a redirection
operator.

Signed-off-by: John Keeping <redacted>
---
diff --git a/t/t8005-blame-i18n.sh b/t/t8005-blame-i18n.sh
@@ -35,8 +35,8 @@ EOF
 test_expect_success !MINGW \
        'blame respects i18n.commitencoding' '
-       git blame --incremental file | \
-               egrep "^(author|summary) " > actual &&
+       git blame --incremental file >output &&
+       sed -ne "/^\(author\|summary\) /p" output >actual &&
These tests all crash and burn with BSD sed (including Mac OS X) since
you're not restricting yourself to BRE (basic regular expressions).
You _could_ request extended regular expressions, which do work on
those platforms, as well as with GNU sed:

    sed -nEe "/^(author|summary) /p" ...
At that point, I think we may as well use grep, because obscure
platforms are probably broken either way.

I'm tempted to just go the perl route. We already depend on at least a
baisc version of perl5 being installed for many of the other tests, so
it's not really introducing a new dependency.

Something like the patch below works for me. I think we could make it
shorter by using $PERLIO to get the raw behavior, but using binmode will
work even on ancient versions of perl.

John, if you agree on the direction, feel free to combine it with your
patch.
diff --git a/t/t8005-blame-i18n.sh b/t/t8005-blame-i18n.sh
index 847d098..f7a02d8 100755
--- a/t/t8005-blame-i18n.sh
+++ b/t/t8005-blame-i18n.sh
@@ -33,10 +33,20 @@ author $SJIS_NAME
 summary $SJIS_MSG
 EOF
 
+filter_blame () {
+	perl -e '
+		binmode STDIN;
+		binmode STDOUT;
+		while (<>) {
+			print if /^(author|summary) /;
+		}
+	'
+}
+
 test_expect_success !MINGW \
 	'blame respects i18n.commitencoding' '
 	git blame --incremental file | \
-		egrep "^(author|summary) " > actual &&
+		filter_blame >actual &&
 	test_cmp actual expected
 '
 
@@ -53,7 +63,7 @@ test_expect_success !MINGW \
 	'blame respects i18n.logoutputencoding' '
 	git config i18n.logoutputencoding eucJP &&
 	git blame --incremental file | \
-		egrep "^(author|summary) " > actual &&
+		filter_blame > actual &&
 	test_cmp actual expected
 '
 
@@ -69,7 +79,7 @@ EOF
 test_expect_success !MINGW \
 	'blame respects --encoding=UTF-8' '
 	git blame --incremental --encoding=UTF-8 file | \
-		egrep "^(author|summary) " > actual &&
+		filter_blame >actual &&
 	test_cmp actual expected
 '
 
@@ -85,7 +95,7 @@ EOF
 test_expect_success !MINGW \
 	'blame respects --encoding=none' '
 	git blame --incremental --encoding=none file | \
-		egrep "^(author|summary) " > actual &&
+		filter_blame >actual &&
 	test_cmp actual expected
 '
 
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help