t: add lint-style.pl and convert grep to test_grep

t/Makefile

@@ -139,7 +139,7 @@ check-meson:
 test-lint: test-lint-duplicates test-lint-executable \
 	test-lint-filenames
 ifneq ($(PERL_PATH),)
-test-lint: test-lint-shell-syntax
+test-lint: test-lint-shell-syntax test-lint-grep
 else
 GIT_TEST_CHAIN_LINT = 0
 endif
@@ -157,6 +157,9 @@ test-lint-executable:
 		test -z "$$bad" || { \
 		echo >&2 "non-executable tests:" $$bad; exit 1; }
 
+test-lint-grep:
+	@'$(PERL_PATH_SQ)' check-grep.pl $(T) $(TPERF)
+
 test-lint-shell-syntax:
 	@'$(PERL_PATH_SQ)' check-non-portable-shell.pl $(T) $(THELPERS) $(TPERF)
 

t/README

@@ -1039,6 +1039,27 @@ see test-lib-functions.sh for the full list and their options.
 
    Check whether a file has the length it is expected to.
 
+ - test_grep [!] [<grep-options>] <pattern> <file>
+
+   Check whether <file> contains a line matching <pattern>, or
+   with '!' that no line matches.  Use this instead of bare
+   'grep <pattern> <file>' in test assertions.  On failure,
+   test_grep prints the contents of <file> for easier debugging,
+   whereas a bare 'grep' would fail silently.
+
+   For negation, pass '!' as the first argument:
+
+	test_grep ! "^diff --git" actual
+
+   Do not negate by writing '! test_grep', as that suppresses the
+   diagnostic output.
+
+   test_grep should only be used as a test assertion.  When grep
+   is used as a data filter (e.g. 'grep -v "^index" actual >filtered')
+   or inside a command substitution (e.g. '$(grep -c ...)'), plain
+   'grep' is the right choice because the exit code is not the
+   assertion itself.
+
  - test_path_is_file <path>
    test_path_is_dir <path>
    test_path_is_missing <path>

t/check-grep.pl

@@ -0,0 +1,51 @@
+#!/usr/bin/perl
+
+# Detect bare 'grep' used as a test assertion where 'test_grep'
+# should be used, and '! test_grep' where 'test_grep !' should
+# be used.
+
+use strict;
+use warnings;
+
+my $exit_code = 0;
+
+sub err {
+	my $msg = shift;
+	s/^\s+//;
+	s/\s+$//;
+	s/\s+/ /g;
+	print "$ARGV:$.: error: $msg: $_\n";
+	$exit_code = 1;
+}
+
+my $line = '';
+my $prev_pipe = 0;
+while (<>) {
+	chomp;
+	$line .= $_;
+	# stitch together incomplete lines (those ending with "\")
+	next if $line =~ s/\\$/ /;
+
+	$_ = $line;
+	/!\s*test_grep\b/ and !/^\s*#/ and !/#.*\blint-ok\b/ and
+		err 'use "test_grep !" instead of "! test_grep"';
+	/(?:^|\s)grep(?=\s)/ and         # has grep as a command word
+	    !$prev_pipe and              # not preceded by pipe
+	    !/^\s*#/ and !/#.*\blint-ok\b/ and
+	    !/\btest_grep\b/ and         # not already converted
+	    !/\btest_(?:expect_\w+|perf)\b/ and  # not in test title
+	    !/\bgit\b[^;&|]*\bgrep\b/ and       # not git grep
+	    !/\|\s*(?:!\s*)?\bgrep\b/ and       # not piped input
+	    !/\bgrep\b.*\s\|\s/ and             # not piped output (same line)
+	    !(/\|\s*$/ and !/\|\|\s*$/) and     # not piped output (trailing |)
+	    !/\bgrep\b.*\s[<>]/ and             # not redirected I/O
+	    !/(?:\$\(|`).*\bgrep\b/ and         # not in $() or backticks
+	    !/\b(?:if|elif|while|until)\b.*\bgrep\b/ and  # not in conditional
+	    !/\bgrep\s+-\w*[clLrR]/ and         # not counting/listing/recursive
+		err 'bare grep outside pipeline (use test_grep)';
+	$prev_pipe = /\|\s*$/ && !/\|\|\s*$/;
+	$line = '';
+	# this resets our $. for each file
+	close ARGV if eof;
+}
+exit $exit_code;

t/perf/p7519-fsmonitor.sh

@@ -157,7 +157,7 @@ setup_for_fsmonitor_hook () {
 	then
 		test_must_be_empty error  # ensure no silent error
 	else
-		grep "Empty last update token" error
+		test_grep "Empty last update token" error
 	fi
 }
 

t/t0000-basic.sh

@@ -743,7 +743,7 @@ test_expect_success 'subtest: lazy prereqs do not turn off tracing' '
 	test_done
 	EOF
 
-	grep "echo trace" lazy-prereq-and-tracing/err
+	test_grep "echo trace" lazy-prereq-and-tracing/err
 '
 
 test_expect_success 'subtest: tests clean up after themselves' '
@@ -815,7 +815,7 @@ test_expect_success 'subtest: test_atexit is run' '
 
 test_expect_success 'test_oid provides sane info by default' '
 	test_oid zero >actual &&
-	grep "^00*\$" actual &&
+	test_grep "^00*\$" actual &&
 	rawsz="$(test_oid rawsz)" &&
 	hexsz="$(test_oid hexsz)" &&
 	# +1 accounts for the trailing newline
@@ -827,7 +827,7 @@ test_expect_success 'test_oid can look up data for SHA-1' '
 	test_when_finished "test_detect_hash" &&
 	test_set_hash sha1 &&
 	test_oid zero >actual &&
-	grep "^00*\$" actual &&
+	test_grep "^00*\$" actual &&
 	rawsz="$(test_oid rawsz)" &&
 	hexsz="$(test_oid hexsz)" &&
 	test $(wc -c <actual) -eq 41 &&
@@ -839,7 +839,7 @@ test_expect_success 'test_oid can look up data for SHA-256' '
 	test_when_finished "test_detect_hash" &&
 	test_set_hash sha256 &&
 	test_oid zero >actual &&
-	grep "^00*\$" actual &&
+	test_grep "^00*\$" actual &&
 	rawsz="$(test_oid rawsz)" &&
 	hexsz="$(test_oid hexsz)" &&
 	test $(wc -c <actual) -eq 65 &&
@@ -884,11 +884,11 @@ test_expect_success 'test_bool_env' '
 		# test script, hence the redirection of fd 7, and aborts
 		# with "exit 1", hence the subshell.
 		! ( test_bool_env envvar true ) 7>err &&
-		grep "error: test_bool_env requires bool values" err &&
+		test_grep "error: test_bool_env requires bool values" err &&
 
 		envvar=true &&
 		! ( test_bool_env envvar invalid ) 7>err &&
-		grep "error: test_bool_env requires bool values" err
+		test_grep "error: test_bool_env requires bool values" err
 	)
 '
 
@@ -1241,13 +1241,13 @@ test_expect_success 'test_must_fail on a failing git command with env' '
 '
 
 test_expect_success 'test_must_fail rejects a non-git command' '
-	! test_must_fail grep ^$ notafile 2>err &&
-	grep -F "test_must_fail: only '"'"'git'"'"' is allowed" err
+	! test_must_fail grep ^$ notafile 2>err && # lint-ok
+	test_grep -F "test_must_fail: only '"'"'git'"'"' is allowed" err
 '
 
 test_expect_success 'test_must_fail rejects a non-git command with env' '
-	! test_must_fail env var1=a var2=b grep ^$ notafile 2>err &&
-	grep -F "test_must_fail: only '"'"'git'"'"' is allowed" err
+	! test_must_fail env var1=a var2=b grep ^$ notafile 2>err && # lint-ok
+	test_grep -F "test_must_fail: only '"'"'git'"'"' is allowed" err
 '
 
 test_done

t/t0001-init.sh

@@ -278,9 +278,9 @@ test_expect_success POSIXPERM 'init creates a new deep directory (umask vs. shar
 		git init --bare --shared=0660 newdir/a/b/c &&
 		test_path_is_dir newdir/a/b/c/refs &&
 		ls -ld newdir/a newdir/a/b > lsab.out &&
-		! grep -v "^drwxrw[sx]r-x" lsab.out &&
+		test_grep ! -v "^drwxrw[sx]r-x" lsab.out &&
 		ls -ld newdir/a/b/c > lsc.out &&
-		! grep -v "^drwxrw[sx]---" lsc.out
+		test_grep ! -v "^drwxrw[sx]---" lsc.out
 	)
 '
 
@@ -619,7 +619,7 @@ test_expect_success DEFAULT_REPO_FORMAT 'extensions.refStorage is not allowed wi
 	git init refstorage &&
 	git -C refstorage config extensions.refStorage files &&
 	test_must_fail git -C refstorage rev-parse 2>err &&
-	grep "repo version is 0, but v1-only extension found" err
+	test_grep "repo version is 0, but v1-only extension found" err
 '
 
 test_expect_success DEFAULT_REPO_FORMAT 'extensions.refStorage with files backend' '
@@ -637,7 +637,7 @@ test_expect_success DEFAULT_REPO_FORMAT 'extensions.refStorage with unknown back
 	git -C refstorage config core.repositoryformatversion 1 &&
 	git -C refstorage config extensions.refStorage garbage &&
 	test_must_fail git -C refstorage rev-parse 2>err &&
-	grep "invalid value for ${SQ}extensions.refstorage${SQ}: ${SQ}garbage${SQ}" err
+	test_grep "invalid value for ${SQ}extensions.refstorage${SQ}: ${SQ}garbage${SQ}" err
 '
 
 test_expect_success 'init with GIT_DEFAULT_REF_FORMAT=garbage' '
@@ -848,8 +848,8 @@ test_expect_success MINGW 'redirect std handles' '
 		GIT_REDIRECT_STDOUT=output.txt \
 		GIT_REDIRECT_STDERR="2>&1" \
 		git rev-parse --git-dir --verify refs/invalid &&
-	grep "^\\.git\$" output.txt &&
-	grep "Needed a single revision" output.txt
+	test_grep "^\\.git\$" output.txt &&
+	test_grep "Needed a single revision" output.txt
 '
 
 test_expect_success '--initial-branch' '
@@ -862,14 +862,14 @@ test_expect_success '--initial-branch' '
 	git init --initial-branch=ignore initial-branch-option 2>err &&
 	test_grep "ignored --initial-branch" err &&
 	git -C initial-branch-option symbolic-ref HEAD >actual &&
-	grep hello actual
+	test_grep hello actual
 '
 
 test_expect_success 'overridden default initial branch name (config)' '
 	test_config_global init.defaultBranch nmb &&
 	GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME= git init initial-branch-config &&
 	git -C initial-branch-config symbolic-ref HEAD >actual &&
-	grep nmb actual
+	test_grep nmb actual
 '
 
 test_expect_success 'advice on unconfigured init.defaultBranch' '
@@ -907,7 +907,7 @@ test_expect_success 'overridden default main branch name (env)' '
 	test_config_global init.defaultBranch nmb &&
 	GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=env git init main-branch-env &&
 	git -C main-branch-env symbolic-ref HEAD >actual &&
-	grep env actual
+	test_grep env actual
 '
 
 test_expect_success 'invalid default branch name' '

t/t0008-ignores.sh

@@ -790,8 +790,8 @@ test_expect_success 'existing file and directory' '
 	>one &&
 	mkdir top-level-dir &&
 	git check-ignore one top-level-dir >actual &&
-	grep one actual &&
-	grep top-level-dir actual
+	test_grep one actual &&
+	test_grep top-level-dir actual
 '
 
 test_expect_success 'existing directory and file' '
@@ -800,8 +800,8 @@ test_expect_success 'existing directory and file' '
 	>one &&
 	mkdir top-level-dir &&
 	git check-ignore top-level-dir one >actual &&
-	grep one actual &&
-	grep top-level-dir actual
+	test_grep one actual &&
+	test_grep top-level-dir actual
 '
 
 test_expect_success 'exact prefix matching (with root)' '

t/t0009-git-dir-validation.sh

@@ -35,7 +35,7 @@ test_expect_success PIPE 'setup: .git as a FIFO (named pipe) is rejected' '
 		cd parent/fifo-trap &&
 		mkfifo .git &&
 		test_must_fail git rev-parse --git-dir 2>stderr &&
-		grep "not a regular file" stderr
+		test_grep "not a regular file" stderr
 	)
 '
 
@@ -47,7 +47,7 @@ test_expect_success SYMLINKS,PIPE 'setup: .git as a symlink to a FIFO is rejecte
 		mkfifo target-fifo &&
 		ln -s target-fifo .git &&
 		test_must_fail git rev-parse --git-dir 2>stderr &&
-		grep "not a regular file" stderr
+		test_grep "not a regular file" stderr
 	)
 '
 
@@ -58,7 +58,7 @@ test_expect_success 'setup: .git with garbage content is rejected' '
 		cd parent/garbage-trap &&
 		echo "garbage" >.git &&
 		test_must_fail git rev-parse --git-dir 2>stderr &&
-		grep "invalid gitfile format" stderr
+		test_grep "invalid gitfile format" stderr
 	)
 '
 

t/t0012-help.sh

@@ -131,8 +131,8 @@ test_expect_success 'git help succeeds without git.html' '
 
 test_expect_success 'git help --user-interfaces' '
 	git help --user-interfaces >help.output &&
-	grep "^   attributes   " help.output &&
-	grep "^   mailmap   " help.output
+	test_grep "^   attributes   " help.output &&
+	test_grep "^   mailmap   " help.output
 '
 
 test_expect_success 'git help -c' '

t/t0013-sha1dc.sh

@@ -16,7 +16,7 @@ fi
 test_expect_success 'test-sha1 detects shattered pdf' '
 	test_must_fail test-tool sha1 <"$TEST_DATA/shattered-1.pdf" 2>err &&
 	test_grep collision err &&
-	grep 38762cf7f55934b34d179ae6a4c80cadccbb7f0a err
+	test_grep 38762cf7f55934b34d179ae6a4c80cadccbb7f0a err
 '
 
 test_done

t/t0014-alias.sh

@@ -105,7 +105,7 @@ cannot_alias_regular_builtin () {
 }
 
 test_expect_success 'cannot alias-shadow a sample of regular builtins' '
-	for cmd in grep check-ref-format interpret-trailers \
+	for cmd in test_grep check-ref-format interpret-trailers \
 		checkout-index fast-import diagnose rev-list prune
 	do
 		cannot_alias_regular_builtin "$cmd" || return 1

t/t0017-env-helper.sh

@@ -88,7 +88,7 @@ test_expect_success 'test-tool env-helper reads config thanks to trace2' '
 	test_must_fail \
 		env HOME="$(pwd)/home" \
 		git config -l 2>err &&
-	grep "exceeded maximum include depth" err &&
+	test_grep "exceeded maximum include depth" err &&
 
 	# This validates that the assumption that we attempt to
 	# read the configuration and fail very early in the start-up
@@ -100,7 +100,7 @@ test_expect_success 'test-tool env-helper reads config thanks to trace2' '
 		test-tool -C no-such-directory \
 		env-helper --type=bool --default=0 \
 		--exit-code GIT_TEST_ENV_HELPER 2>err &&
-	grep "exceeded maximum include depth" err
+	test_grep "exceeded maximum include depth" err
 '
 
 test_done