From 472d05111a38276192e30f454f42aa39df51d604 Mon Sep 17 00:00:00 2001
From: Phillip Wood <phillip.wood@dunelm.org.uk>
Date: Sun, 2 Oct 2022 16:51:02 +0100
Subject: [PATCH 1/3] [RFC] tests: add test_todo() to mark known breakages

test_todo() is intended as a fine grained replacement for
test_expect_failure(). Rather than marking the whole test as failing
test_todo() is used to mark individual failing commands within a
test. This approach to writing failing tests allows us to detect
unexpected failures that are hidden by test_expect_failure().

Failing commands are reported by the test harness in the same way as
test_expect_failure() so there is no change in output when migrating
from test_expect_failure() to test_todo(). If a command marked with
test_todo() succeeds then the test will fail. This is designed to make
it easier to see when a command starts succeeding in our CI compared
to using test_expect_failure() where it is easy to fix a failing test
case and not realize it.

test_todo() is built upon test_expect_failure() but accepts commands
starting with test_* in addition to git. As our test_* assertions use
BUG() to signal usage errors any such error will not be hidden by
test_todo().

This commit coverts a few tests to show the intended use of
test_todo().  A limitation of test_todo() as it is currently
implemented is that it cannot be used in a subshell.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 t/README                        |  12 +++
 t/t0000-basic.sh                |  64 ++++++++++++++
 t/t3401-rebase-and-am-rename.sh |  12 +--
 t/t3424-rebase-empty.sh         |   6 +-
 t/t3600-rm.sh                   |   8 +-
 t/test-lib-functions.sh         | 147 +++++++++++++++++++++++---------
 6 files changed, 194 insertions(+), 55 deletions(-)

diff --git a/t/README b/t/README
index 979b2d4833d983..642aeab80b4913 100644
--- a/t/README
+++ b/t/README
@@ -892,6 +892,10 @@ see test-lib-functions.sh for the full list and their options.
 
  - test_expect_failure [<prereq>] <message> <script>
 
+   Where possible new tests should use test_expect_success and mark
+   the individual failing commands with test_todo (see below) rather
+   than using test_expect_failure.
+
    This is NOT the opposite of test_expect_success, but is used
    to mark a test that demonstrates a known breakage.  Unlike
    the usual test_expect_success tests, which say "ok" on
@@ -902,6 +906,14 @@ see test-lib-functions.sh for the full list and their options.
    Like test_expect_success this function can optionally use a three
    argument invocation with a prerequisite as the first argument.
 
+ - test_todo <command>
+
+   This is used to mark commands that should succeed but do not due to
+   a known issue. The test will be reported as a known failure if the
+   issue still exists and won’t cause -i (immediate) to stop. If the
+   command unexpectedly succeeds then the test will be reported as a
+   failing. test_todo cannot be used in a subshell.
+
  - test_debug <script>
 
    This takes a single argument, <script>, and evaluates it only
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 502b4bcf9ea0ad..52362ad3dd3c83 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -141,6 +141,70 @@ test_expect_success 'subtest: a passing TODO test' '
 	EOF
 '
 
+test_expect_success 'subtest: a failing test_todo' '
+	write_and_run_sub_test_lib_test failing-test-todo <<-\EOF &&
+	test_false () {
+		false
+	}
+	test_expect_success "passing test" "true"
+	test_expect_success "known todo" "test_todo test_false"
+	test_done
+	EOF
+	check_sub_test_lib_test failing-test-todo <<-\EOF
+	> ok 1 - passing test
+	> not ok 2 - known todo # TODO known breakage
+	> # still have 1 known breakage(s)
+	> # passed all remaining 1 test(s)
+	> 1..2
+	EOF
+'
+
+test_expect_success 'subtest: a passing test_todo' '
+	write_and_run_sub_test_lib_test_err passing-test-todo <<-\EOF &&
+	test_true () {
+		true
+	}
+	test_expect_success "pretend we have fixed a test_todo breakage" \
+		"test_todo test_true"
+	test_done
+	EOF
+	check_sub_test_lib_test passing-test-todo <<-\EOF
+	> not ok 1 - pretend we have fixed a test_todo breakage
+	> #	test_todo test_true
+	> # failed 1 among 1 test(s)
+	> 1..1
+	EOF
+'
+
+test_expect_success 'subtest: test_todo allowed arguments' '
+	write_and_run_sub_test_lib_test_err acceptable-test-todo <<-\EOF &&
+	# This an acceptable command for test_todo but not test_must_fail
+	test_true () {
+		  return 0
+	}
+	test_expect_success "test_todo skips env and accepts good command" \
+		"test_todo env Var=Value git --invalid-option"
+	test_expect_success "test_todo skips env and rejects bad command" \
+		"test_todo env Var=Value false"
+	test_expect_success "test_todo test_must_fail accepts good command" \
+		"test_todo test_must_fail git --version"
+	test_expect_success "test_todo test_must_fail rejects bad command" \
+		"test_todo test_must_fail test_true"
+	test_done
+	EOF
+	check_sub_test_lib_test acceptable-test-todo <<-\EOF
+	> not ok 1 - test_todo skips env and accepts good command # TODO known breakage
+	> not ok 2 - test_todo skips env and rejects bad command
+	> #	test_todo env Var=Value false
+	> not ok 3 - test_todo test_must_fail accepts good command # TODO known breakage
+	> not ok 4 - test_todo test_must_fail rejects bad command
+	> #	test_todo test_must_fail test_true
+	> # still have 2 known breakage(s)
+	> # failed 2 among remaining 2 test(s)
+	> 1..4
+	EOF
+'
+
 test_expect_success 'subtest: 2 TODO tests, one passin' '
 	write_and_run_sub_test_lib_test partially-passing-todos <<-\EOF &&
 	test_expect_failure "pretend we have a known breakage" "false"
diff --git a/t/t3401-rebase-and-am-rename.sh b/t/t3401-rebase-and-am-rename.sh
index f18bae94507587..cc5da9f5afe9ed 100755
--- a/t/t3401-rebase-and-am-rename.sh
+++ b/t/t3401-rebase-and-am-rename.sh
@@ -52,7 +52,7 @@ test_expect_success 'rebase --interactive: directory rename detected' '
 	)
 '
 
-test_expect_failure 'rebase --apply: directory rename detected' '
+test_expect_success 'rebase --apply: directory rename detected' '
 	(
 		cd dir-rename &&
 
@@ -63,8 +63,8 @@ test_expect_failure 'rebase --apply: directory rename detected' '
 		git ls-files -s >out &&
 		test_line_count = 5 out &&
 
-		test_path_is_file y/d &&
-		test_path_is_missing x/d
+		test_todo test_path_is_file y/d &&
+		test_todo test_path_is_missing x/d
 	)
 '
 
@@ -84,7 +84,7 @@ test_expect_success 'rebase --merge: directory rename detected' '
 	)
 '
 
-test_expect_failure 'am: directory rename detected' '
+test_expect_success 'am: directory rename detected' '
 	(
 		cd dir-rename &&
 
@@ -97,8 +97,8 @@ test_expect_failure 'am: directory rename detected' '
 		git ls-files -s >out &&
 		test_line_count = 5 out &&
 
-		test_path_is_file y/d &&
-		test_path_is_missing x/d
+		test_todo test_path_is_file y/d &&
+		test_todo test_path_is_missing x/d
 	)
 '
 
diff --git a/t/t3424-rebase-empty.sh b/t/t3424-rebase-empty.sh
index 5e1045a0afc9ec..b7cae260759da6 100755
--- a/t/t3424-rebase-empty.sh
+++ b/t/t3424-rebase-empty.sh
@@ -34,15 +34,15 @@ test_expect_success 'setup test repository' '
 	git commit -m "Five letters ought to be enough for anybody"
 '
 
-test_expect_failure 'rebase (apply-backend)' '
-	test_when_finished "git rebase --abort" &&
+test_expect_success 'rebase (apply-backend)' '
+	test_when_finished "test_might_fail git rebase --abort" &&
 	git checkout -B testing localmods &&
 	# rebase (--apply) should not drop commits that start empty
 	git rebase --apply upstream &&
 
 	test_write_lines D C B A >expect &&
 	git log --format=%s >actual &&
-	test_cmp expect actual
+	test_todo test_cmp expect actual
 '
 
 test_expect_success 'rebase --merge --empty=drop' '
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index e74a318ac33ac0..fa7831c0674fac 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -790,7 +790,7 @@ test_expect_success SYMLINKS 'rm across a symlinked leading path (no index)' '
 	test_path_is_file e/f
 '
 
-test_expect_failure SYMLINKS 'rm across a symlinked leading path (w/ index)' '
+test_expect_success SYMLINKS 'rm across a symlinked leading path (w/ index)' '
 	rm -rf d e &&
 	mkdir d &&
 	echo content >d/f &&
@@ -798,10 +798,10 @@ test_expect_failure SYMLINKS 'rm across a symlinked leading path (w/ index)' '
 	git commit -m "d/f exists" &&
 	mv d e &&
 	ln -s e d &&
-	test_must_fail git rm d/f &&
-	git rev-parse --verify :d/f &&
+	test_todo test_must_fail git rm d/f &&
+	test_todo git rev-parse --verify :d/f &&
 	test -h d &&
-	test_path_is_file e/f
+	test_todo test_path_is_file e/f
 '
 
 test_expect_success 'setup for testing rm messages' '
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index c6479f24eb5ac2..8978709b231798 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -802,6 +802,7 @@ test_expect_failure () {
 	export test_prereq
 	if ! test_skip "$@"
 	then
+		test_todo_=test_expect_failure
 		test -n "$test_skip_test_preamble" ||
 		say >&3 "checking known breakage of $TEST_NUMBER.$test_count '$1': $2"
 		if test_run_ "$2" expecting_failure
@@ -825,9 +826,15 @@ test_expect_success () {
 	then
 		test -n "$test_skip_test_preamble" ||
 		say >&3 "expecting success of $TEST_NUMBER.$test_count '$1': $2"
+		test_todo_=test_expect_success
 		if test_run_ "$2"
 		then
-			test_ok_ "$1"
+			if test "$test_todo_" = "todo"
+			then
+				test_known_broken_failure_ "$1"
+			else
+				test_ok_ "$1"
+			fi
 		else
 			test_failure_ "$@"
 		fi
@@ -999,10 +1006,18 @@ list_contains () {
 }
 
 # Returns success if the arguments indicate that a command should be
-# accepted by test_must_fail(). If the command is run with env, the env
-# and its corresponding variable settings will be stripped before we
-# test the command being run.
+# accepted by test_must_fail() or test_todo(). If the command is run
+# with env, the env and its corresponding variable settings will be
+# stripped before we we test the command being run.
+#
+# test_todo() allows any of the assertions beginning test_ such as
+# test_cmp in addition the commands allowed by test_must_fail().
+
 test_must_fail_acceptable () {
+	local name
+	name="$1"
+	shift
+
 	if test "$1" = "env"
 	then
 		shift
@@ -1023,12 +1038,96 @@ test_must_fail_acceptable () {
 	git|__git*|test-tool|test_terminal)
 		return 0
 		;;
+	test_might_fail|test_todo|test_when_finished)
+		return 1
+		;;
+	test_must_fail)
+		if test "$name" = "todo"
+		then
+			shift
+			test_must_fail_acceptable must_fail "$@"
+			return $?
+		fi
+		return 1
+		;;
+	test_*)
+		test "$name" = "todo"
+		return $?
+		;;
 	*)
 		return 1
 		;;
 	esac
 }
 
+test_must_fail_helper () {
+	test_must_fail_name_="$1"
+	shift
+	case "$1" in
+	ok=*)
+		_test_ok=${1#ok=}
+		shift
+		;;
+	*)
+		_test_ok=
+		;;
+	esac
+	if ! test_must_fail_acceptable $test_must_fail_name_ "$@"
+	then
+		echo >&7 "test_$test_must_fail_name_: only 'git' is allowed: $*"
+		return 1
+	fi
+	"$@" 2>&7
+	exit_code=$?
+	if test $exit_code -eq 0 && ! list_contains "$_test_ok" success
+	then
+		echo >&4 "test_$test_must_fail_name_: command succeeded: $*"
+		return 1
+	elif test_match_signal 13 $exit_code && list_contains "$_test_ok" sigpipe
+	then
+		return 0
+	elif test $exit_code -gt 129 && test $exit_code -le 192
+	then
+		echo >&4 "test_$test_must_fail_name_: died by signal $(($exit_code - 128)): $*"
+		return 1
+	elif test $exit_code -eq 127
+	then
+		echo >&4 "test_$test_must_fail_name_: command not found: $*"
+		return 1
+	elif test $exit_code -eq 126
+	then
+		echo >&4 "test_$test_must_fail_name_: valgrind error: $*"
+		return 1
+	fi
+
+	return 0
+} 7>&2 2>&4
+
+# This is used to mark commands that should succeed but do not due to
+# a known issue. Marking the individual commands that fail rather than
+# using test_expect_failure allows us to detect any unexpected
+# failures. As with test_must_fail if the command is killed by a
+# signal the test will fail. If the command unexpectedly succeeds then
+# the test will also fail. For example:
+#
+#	test_expect_success 'test a known failure' '
+#		git foo 2>err &&
+#		test_todo test_must_be_empty err
+#	'
+#
+# This test will fail if "git foo" fails or err is unexpectedly empty.
+# test_todo can be used with "git" or any of the "test_*" assertions
+# such as test_cmp().
+
+test_todo () {
+	if test "$test_todo_" = "test_expect_failure"
+	then
+		BUG "test_todo_ cannot be used inside test_expect_failure"
+	fi
+	test_todo_=todo
+	test_must_fail_helper todo "$@" 2>&7
+} 7>&2 2>&4
+
 # This is not among top-level (test_expect_success | test_expect_failure)
 # but is a prefix that can be used in the test script, like:
 #
@@ -1061,43 +1160,7 @@ test_must_fail_acceptable () {
 #    ! grep pattern output
 
 test_must_fail () {
-	case "$1" in
-	ok=*)
-		_test_ok=${1#ok=}
-		shift
-		;;
-	*)
-		_test_ok=
-		;;
-	esac
-	if ! test_must_fail_acceptable "$@"
-	then
-		echo >&7 "test_must_fail: only 'git' is allowed: $*"
-		return 1
-	fi
-	"$@" 2>&7
-	exit_code=$?
-	if test $exit_code -eq 0 && ! list_contains "$_test_ok" success
-	then
-		echo >&4 "test_must_fail: command succeeded: $*"
-		return 1
-	elif test_match_signal 13 $exit_code && list_contains "$_test_ok" sigpipe
-	then
-		return 0
-	elif test $exit_code -gt 129 && test $exit_code -le 192
-	then
-		echo >&4 "test_must_fail: died by signal $(($exit_code - 128)): $*"
-		return 1
-	elif test $exit_code -eq 127
-	then
-		echo >&4 "test_must_fail: command not found: $*"
-		return 1
-	elif test $exit_code -eq 126
-	then
-		echo >&4 "test_must_fail: valgrind error: $*"
-		return 1
-	fi
-	return 0
+	test_must_fail_helper must_fail "$@" 2>&7
 } 7>&2 2>&4
 
 # Similar to test_must_fail, but tolerates success, too.  This is
@@ -1114,7 +1177,7 @@ test_must_fail () {
 # Accepts the same options as test_must_fail.
 
 test_might_fail () {
-	test_must_fail ok=success "$@" 2>&7
+	test_must_fail_helper might_fail ok=success "$@" 2>&7
 } 7>&2 2>&4
 
 # Similar to test_must_fail and test_might_fail, but check that a

From 645fa2990f79bdb7ee00ff3fd34122676469a783 Mon Sep 17 00:00:00 2001
From: Phillip Wood <phillip.wood@dunelm.org.uk>
Date: Sun, 2 Oct 2022 16:55:57 +0100
Subject: [PATCH 2/3] [RFC] test_todo: allow [!] grep as the command

Many failing tests use grep, this commit converts a sample to use
test_todo(). As POSIX specifies that a return code greater than one
indicates an error rather than a failing match we take care not the
hide that.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 t/t0000-basic.sh                | 20 ++++++++++--
 t/t3510-cherry-pick-sequence.sh | 12 +++----
 t/t4014-format-patch.sh         | 20 ++++++------
 t/test-lib-functions.sh         | 56 ++++++++++++++++++++++++++++-----
 4 files changed, 82 insertions(+), 26 deletions(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 52362ad3dd3c83..db5c2059eb5893 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -190,6 +190,14 @@ test_expect_success 'subtest: test_todo allowed arguments' '
 		"test_todo test_must_fail git --version"
 	test_expect_success "test_todo test_must_fail rejects bad command" \
 		"test_todo test_must_fail test_true"
+	test_expect_success "test_todo accepts grep" \
+		"echo a >a && test_todo grep b <a"
+	test_expect_success "test_todo accepts ! grep" \
+		"echo a >a && test_todo ! grep -v b <a"
+	test_expect_success "test_todo detects grep errors" \
+		"echo a >a && test_todo grep --invalid-option b <a"
+	test_expect_success "test_todo detects ! grep errors" \
+		"echo a >a && test_todo ! grep --invalid-option -v b <a"
 	test_done
 	EOF
 	check_sub_test_lib_test acceptable-test-todo <<-\EOF
@@ -199,9 +207,15 @@ test_expect_success 'subtest: test_todo allowed arguments' '
 	> not ok 3 - test_todo test_must_fail accepts good command # TODO known breakage
 	> not ok 4 - test_todo test_must_fail rejects bad command
 	> #	test_todo test_must_fail test_true
-	> # still have 2 known breakage(s)
-	> # failed 2 among remaining 2 test(s)
-	> 1..4
+	> not ok 5 - test_todo accepts grep # TODO known breakage
+	> not ok 6 - test_todo accepts ! grep # TODO known breakage
+	> not ok 7 - test_todo detects grep errors
+	> #	echo a >a && test_todo grep --invalid-option b <a
+	> not ok 8 - test_todo detects ! grep errors
+	> #	echo a >a && test_todo ! grep --invalid-option -v b <a
+	> # still have 4 known breakage(s)
+	> # failed 4 among remaining 4 test(s)
+	> 1..8
 	EOF
 '
 
diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
index 3b0fa66c33da58..cd869255a2c90d 100755
--- a/t/t3510-cherry-pick-sequence.sh
+++ b/t/t3510-cherry-pick-sequence.sh
@@ -577,7 +577,7 @@ test_expect_success '--continue respects -x in first commit in multi-pick' '
 	grep "cherry picked from.*$picked" msg
 '
 
-test_expect_failure '--signoff is automatically propagated to resolved conflict' '
+test_expect_success '--signoff is automatically propagated to resolved conflict' '
 	pristine_detach initial &&
 	test_expect_code 1 git cherry-pick --signoff base..anotherpick &&
 	echo "c" >foo &&
@@ -591,11 +591,11 @@ test_expect_failure '--signoff is automatically propagated to resolved conflict'
 	git cat-file commit HEAD~3 >initial_msg &&
 	! grep "Signed-off-by:" initial_msg &&
 	grep "Signed-off-by:" unrelatedpick_msg &&
-	! grep "Signed-off-by:" picked_msg &&
+	test_todo ! grep "Signed-off-by:" picked_msg &&
 	grep "Signed-off-by:" anotherpick_msg
 '
 
-test_expect_failure '--signoff dropped for implicit commit of resolution, multi-pick case' '
+test_expect_success '--signoff dropped for implicit commit of resolution, multi-pick case' '
 	pristine_detach initial &&
 	test_must_fail git cherry-pick -s picked anotherpick &&
 	echo c >foo &&
@@ -605,10 +605,10 @@ test_expect_failure '--signoff dropped for implicit commit of resolution, multi-
 	git diff --exit-code HEAD &&
 	test_cmp_rev initial HEAD^^ &&
 	git cat-file commit HEAD^ >msg &&
-	! grep Signed-off-by: msg
+	test_todo ! grep Signed-off-by: msg
 '
 
-test_expect_failure 'sign-off needs to be reaffirmed after conflict resolution, single-pick case' '
+test_expect_success 'sign-off needs to be reaffirmed after conflict resolution, single-pick case' '
 	pristine_detach initial &&
 	test_must_fail git cherry-pick -s picked &&
 	echo c >foo &&
@@ -618,7 +618,7 @@ test_expect_failure 'sign-off needs to be reaffirmed after conflict resolution,
 	git diff --exit-code HEAD &&
 	test_cmp_rev initial HEAD^ &&
 	git cat-file commit HEAD >msg &&
-	! grep Signed-off-by: msg
+	test_todo ! grep Signed-off-by: msg
 '
 
 test_expect_success 'malformed instruction sheet 1' '
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index ad5c02927943ab..ab649bf27e50c6 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -165,12 +165,12 @@ test_expect_success 'additional command line cc (ascii)' '
 	grep "^ *S E Cipient <scipient@example.com>\$" hdrs5
 '
 
-test_expect_failure 'additional command line cc (rfc822)' '
+test_expect_success 'additional command line cc (rfc822)' '
 	git config --replace-all format.headers "Cc: R E Cipient <rcipient@example.com>" &&
 	git format-patch --cc="S. E. Cipient <scipient@example.com>" --stdout main..side >patch5 &&
 	sed -e "/^\$/q" patch5 >hdrs5 &&
 	grep "^Cc: R E Cipient <rcipient@example.com>,\$" hdrs5 &&
-	grep "^ *\"S. E. Cipient\" <scipient@example.com>\$" hdrs5
+	test_todo grep "^ *\"S. E. Cipient\" <scipient@example.com>\$" hdrs5
 '
 
 test_expect_success 'command line headers' '
@@ -195,16 +195,16 @@ test_expect_success 'command line To: header (ascii)' '
 	grep "^To: R E Cipient <rcipient@example.com>\$" hdrs8
 '
 
-test_expect_failure 'command line To: header (rfc822)' '
+test_expect_success 'command line To: header (rfc822)' '
 	git format-patch --to="R. E. Cipient <rcipient@example.com>" --stdout main..side >patch8 &&
 	sed -e "/^\$/q" patch8 >hdrs8 &&
-	grep "^To: \"R. E. Cipient\" <rcipient@example.com>\$" hdrs8
+	test_todo grep "^To: \"R. E. Cipient\" <rcipient@example.com>\$" hdrs8
 '
 
-test_expect_failure 'command line To: header (rfc2047)' '
+test_expect_success 'command line To: header (rfc2047)' '
 	git format-patch --to="R Ä Cipient <rcipient@example.com>" --stdout main..side >patch8 &&
 	sed -e "/^\$/q" patch8 >hdrs8 &&
-	grep "^To: =?UTF-8?q?R=20=C3=84=20Cipient?= <rcipient@example.com>\$" hdrs8
+	test_todo grep "^To: =?UTF-8?q?R=20=C3=84=20Cipient?= <rcipient@example.com>\$" hdrs8
 '
 
 test_expect_success 'configuration To: header (ascii)' '
@@ -214,18 +214,18 @@ test_expect_success 'configuration To: header (ascii)' '
 	grep "^To: R E Cipient <rcipient@example.com>\$" hdrs9
 '
 
-test_expect_failure 'configuration To: header (rfc822)' '
+test_expect_success 'configuration To: header (rfc822)' '
 	git config format.to "R. E. Cipient <rcipient@example.com>" &&
 	git format-patch --stdout main..side >patch9 &&
 	sed -e "/^\$/q" patch9 >hdrs9 &&
-	grep "^To: \"R. E. Cipient\" <rcipient@example.com>\$" hdrs9
+	test_todo grep "^To: \"R. E. Cipient\" <rcipient@example.com>\$" hdrs9
 '
 
-test_expect_failure 'configuration To: header (rfc2047)' '
+test_expect_success 'configuration To: header (rfc2047)' '
 	git config format.to "R Ä Cipient <rcipient@example.com>" &&
 	git format-patch --stdout main..side >patch9 &&
 	sed -e "/^\$/q" patch9 >hdrs9 &&
-	grep "^To: =?UTF-8?q?R=20=C3=84=20Cipient?= <rcipient@example.com>\$" hdrs9
+	test_todo grep "^To: =?UTF-8?q?R=20=C3=84=20Cipient?= <rcipient@example.com>\$" hdrs9
 '
 
 # check_patch <patch>: Verify that <patch> looks like a half-sane
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 8978709b231798..aee10bd0706052 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1010,8 +1010,9 @@ list_contains () {
 # with env, the env and its corresponding variable settings will be
 # stripped before we we test the command being run.
 #
-# test_todo() allows any of the assertions beginning test_ such as
-# test_cmp in addition the commands allowed by test_must_fail().
+# test_todo() allows "grep" or any of the assertions beginning test_
+# such as test_cmp in addition the commands allowed by
+# test_must_fail().
 
 test_must_fail_acceptable () {
 	local name
@@ -1050,9 +1051,21 @@ test_must_fail_acceptable () {
 		fi
 		return 1
 		;;
-	test_*)
-		test "$name" = "todo"
-		return $?
+	grep|test_*)
+		if test "$name" = "todo"
+		then
+			test_todo_command_="$1"
+			return 0
+		fi
+		return 1
+		;;
+	'!')
+		if test "$name" = "todo" && test "$2" = "grep"
+		then
+			test_todo_command_=grep
+			return 0
+		fi
+		return 1
 		;;
 	*)
 		return 1
@@ -1061,6 +1074,7 @@ test_must_fail_acceptable () {
 }
 
 test_must_fail_helper () {
+	test_todo_command_=
 	test_must_fail_name_="$1"
 	shift
 	case "$1" in
@@ -1077,8 +1091,27 @@ test_must_fail_helper () {
 		echo >&7 "test_$test_must_fail_name_: only 'git' is allowed: $*"
 		return 1
 	fi
+	test_invert_exit_code_=
+	if test "$1" = "!"
+	then
+		test_invert_exit_code_=1
+		shift
+	fi
 	"$@" 2>&7
 	exit_code=$?
+	if test -n "$test_invert_exit_code_"
+	# We only invert the exit code of grep. An exit code greater
+	# than 1 indicates an error rather than a failed match so
+	# we only want to swap zero and one.
+	then
+		if test $exit_code -eq 0
+		then
+			exit_code=1
+		elif test $exit_code -eq 1
+		then
+			exit_code=0
+		fi
+	fi
 	if test $exit_code -eq 0 && ! list_contains "$_test_ok" success
 	then
 		echo >&4 "test_$test_must_fail_name_: command succeeded: $*"
@@ -1098,6 +1131,15 @@ test_must_fail_helper () {
 	then
 		echo >&4 "test_$test_must_fail_name_: valgrind error: $*"
 		return 1
+	else
+		case "$test_todo_command_" in
+		grep)
+			if test $exit_code -ne 1
+			then
+			       echo >&4 "test_todo: $test_todo_command_ error: $*"
+			       return 1
+			fi;;
+		esac
 	fi
 
 	return 0
@@ -1116,8 +1158,8 @@ test_must_fail_helper () {
 #	'
 #
 # This test will fail if "git foo" fails or err is unexpectedly empty.
-# test_todo can be used with "git" or any of the "test_*" assertions
-# such as test_cmp().
+# test_todo can be used with "git", "grep" or any of the "test_*"
+# assertions such as test_cmp().
 
 test_todo () {
 	if test "$test_todo_" = "test_expect_failure"

From c0d955c2d1fa234ad2e4a8aad467b991030ccf47 Mon Sep 17 00:00:00 2001
From: Phillip Wood <phillip.wood@dunelm.org.uk>
Date: Sun, 2 Oct 2022 16:57:05 +0100
Subject: [PATCH 3/3] [RFC] test_todo: allow [verbose] test as the command

For simplicity test_todo() allows verbose to precede any valid
command.  As POSIX specifies that a return code greater than one is an
error rather than a failed test we take care not to hide that.

I'm in two minds about this patch. Generally it is better to use one
of our test helpers such as test_cmp() rather than calling test
directly.  There are so few instances of test being used within
test_expect_failure() (the conversions here are not exhaustive but
there are not many more) that it would probably be better to convert
the tests by using the appropriate helper rather than supporting
calling test as the command to test_todo().

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 t/t0000-basic.sh        | 20 +++++++++++++++++---
 t/t0050-filesystem.sh   |  4 ++--
 t/t3903-stash.sh        | 12 ++++++------
 t/test-lib-functions.sh | 17 +++++++++++------
 4 files changed, 36 insertions(+), 17 deletions(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index db5c2059eb5893..93d3930d9f6956 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -198,6 +198,14 @@ test_expect_success 'subtest: test_todo allowed arguments' '
 		"echo a >a && test_todo grep --invalid-option b <a"
 	test_expect_success "test_todo detects ! grep errors" \
 		"echo a >a && test_todo ! grep --invalid-option -v b <a"
+	test_expect_success "test_todo accepts test" \
+		"test_todo test -z a"
+	test_expect_success "test_todo detects test errors" \
+		"test_todo test a xxx b"
+	test_expect_success "test_todo skips verbose and accepts good command" \
+		"test_todo verbose test -z a"
+	test_expect_success "test_todo skips verbose and rejects bad command" \
+		"test_todo verbose false"
 	test_done
 	EOF
 	check_sub_test_lib_test acceptable-test-todo <<-\EOF
@@ -213,9 +221,15 @@ test_expect_success 'subtest: test_todo allowed arguments' '
 	> #	echo a >a && test_todo grep --invalid-option b <a
 	> not ok 8 - test_todo detects ! grep errors
 	> #	echo a >a && test_todo ! grep --invalid-option -v b <a
-	> # still have 4 known breakage(s)
-	> # failed 4 among remaining 4 test(s)
-	> 1..8
+	> not ok 9 - test_todo accepts test # TODO known breakage
+	> not ok 10 - test_todo detects test errors
+	> #	test_todo test a xxx b
+	> not ok 11 - test_todo skips verbose and accepts good command # TODO known breakage
+	> not ok 12 - test_todo skips verbose and rejects bad command
+	> #	test_todo verbose false
+	> # still have 6 known breakage(s)
+	> # failed 6 among remaining 6 test(s)
+	> 1..12
 	EOF
 '
 
diff --git a/t/t0050-filesystem.sh b/t/t0050-filesystem.sh
index 325eb1c3cd0add..16b933a4c8d9fa 100755
--- a/t/t0050-filesystem.sh
+++ b/t/t0050-filesystem.sh
@@ -100,7 +100,7 @@ test_expect_success CASE_INSENSITIVE_FS 'add directory (with different case)' '
 	test_cmp expected actual
 '
 
-test_expect_failure CASE_INSENSITIVE_FS 'add (with different case)' '
+test_expect_success CASE_INSENSITIVE_FS 'add (with different case)' '
 	git reset --hard initial &&
 	rm camelcase &&
 	echo 1 >CamelCase &&
@@ -108,7 +108,7 @@ test_expect_failure CASE_INSENSITIVE_FS 'add (with different case)' '
 	git ls-files >tmp &&
 	camel=$(grep -i camelcase tmp) &&
 	test $(echo "$camel" | wc -l) = 1 &&
-	test "z$(git cat-file blob :$camel)" = z1
+	test_todo test "z$(git cat-file blob :$camel)" = z1
 '
 
 test_expect_success "setup unicode normalization tests" '
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 376cc8f4ab8429..afc5b4126f3adb 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -556,7 +556,7 @@ test_expect_success 'unstash must re-create the file' '
 	test bar = "$(cat file)"
 '
 
-test_expect_failure 'stash directory to file' '
+test_expect_success 'stash directory to file' '
 	git reset --hard &&
 	mkdir dir &&
 	echo foo >dir/file &&
@@ -567,12 +567,12 @@ test_expect_failure 'stash directory to file' '
 	git stash save "directory to file" &&
 	test_path_is_dir dir &&
 	test foo = "$(cat dir/file)" &&
-	test_must_fail git stash apply &&
-	test bar = "$(cat dir)" &&
+	test_todo test_must_fail git stash apply &&
+	test_todo test bar = "$(cat dir)" &&
 	git reset --soft HEAD^
 '
 
-test_expect_failure 'stash file to directory' '
+test_expect_success 'stash file to directory' '
 	git reset --hard &&
 	rm file &&
 	mkdir file &&
@@ -581,8 +581,8 @@ test_expect_failure 'stash file to directory' '
 	test_path_is_file file &&
 	test bar = "$(cat file)" &&
 	git stash apply &&
-	test_path_is_file file/file &&
-	test foo = "$(cat file/file)"
+	test_todo test_path_is_file file/file &&
+	test_todo test foo = "$(cat file/file)"
 '
 
 test_expect_success 'giving too many ref arguments does not modify files' '
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index aee10bd0706052..068a0702809a5a 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1010,8 +1010,8 @@ list_contains () {
 # with env, the env and its corresponding variable settings will be
 # stripped before we we test the command being run.
 #
-# test_todo() allows "grep" or any of the assertions beginning test_
-# such as test_cmp in addition the commands allowed by
+# test_todo() allows "grep", "test" or any of the assertions beginning
+# test_ such as test_cmp in addition the commands allowed by
 # test_must_fail().
 
 test_must_fail_acceptable () {
@@ -1019,6 +1019,11 @@ test_must_fail_acceptable () {
 	name="$1"
 	shift
 
+	if test "$name" = "todo" && test "$1" = "verbose"
+	then
+		shift
+	fi
+
 	if test "$1" = "env"
 	then
 		shift
@@ -1051,7 +1056,7 @@ test_must_fail_acceptable () {
 		fi
 		return 1
 		;;
-	grep|test_*)
+	grep|test|test_*)
 		if test "$name" = "todo"
 		then
 			test_todo_command_="$1"
@@ -1133,7 +1138,7 @@ test_must_fail_helper () {
 		return 1
 	else
 		case "$test_todo_command_" in
-		grep)
+		grep|test)
 			if test $exit_code -ne 1
 			then
 			       echo >&4 "test_todo: $test_todo_command_ error: $*"
@@ -1158,8 +1163,8 @@ test_must_fail_helper () {
 #	'
 #
 # This test will fail if "git foo" fails or err is unexpectedly empty.
-# test_todo can be used with "git", "grep" or any of the "test_*"
-# assertions such as test_cmp().
+# test_todo can be used with "git", "grep", "test" or any of the
+# "test_*" assertions such as test_cmp().
 
 test_todo () {
 	if test "$test_todo_" = "test_expect_failure"