git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"John Cai" <johncai86@gmail.com>,
	"Elijah Newren" <newren@gmail.com>,
	"Derrick Stolee" <stolee@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH 2/7] test-lib-functions: add and use a "test_todo" helper
Date: Fri, 18 Mar 2022 01:33:57 +0100	[thread overview]
Message-ID: <patch-2.7-0834a3ed049-20220318T002951Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-0.7-00000000000-20220318T002951Z-avarab@gmail.com>

Introduce a new "test_todo" function for use with the new
"test_expect_todo" function, and convert a couple of tests to use it,
including one added in aecad374ae7 (reflog-walk: don't segfault on
non-commit sha1's in the reflog, 2016-01-05).

A good thing about "test_expect_failure" is that it allowed for
writing the sort of tests added in aecad374ae7, where we declare what
we'd like to eventually get, but just mark that we're still failing to
get that.

For cases like the one-line "t7815-grep-binary.sh" test adjusted in
the preceding commit to use "test_expect_todo" it's obvious what we'd
eventually like to do, to remove the "test_must_fail" and convert
"test_expect_todo" to "test_expect_success".

But in the cases being adjusted here it would be a regression to
simply remove the "test_line_count", as the "expect" part of it is
what we'd eventually like to have pass.

By introducing a "test_todo" we can both assert how something
currently fails, and how it should behave once the relevant feature is
implemented (or a bug is fixed).

The "test_todo" function will always run the check for what we
eventually want, and BUG() out if it succeeds. We'll also BUG() out if
neither succeed.

This way we can ensure that unlike "test_expect_failure" blocks the
assertions aren't dead or broken code. A few uses of
"test_expect_failure" in the test suite (not altered here) are
hopelessly broken, and could never pass even if a bug was fixed or a
given feature was implemented.

We just haven't noticed because we've been treating all failures in
"test_expect_failure" equally, using "test_expect_todo" with this
"test_todo" helper is a better way forward.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/t0000-basic.sh        | 37 ++++++++++++++++++++
 t/t1410-reflog.sh       |  4 +--
 t/t6403-merge-file.sh   |  9 +++--
 t/test-lib-functions.sh | 77 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 123 insertions(+), 4 deletions(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index e46638f1f7b..bbe48d63b16 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -204,6 +204,43 @@ test_expect_success 'subtest: mixed results: a mixture of all possible results'
 	EOF
 '
 
+test_expect_success 'subtest: test_expect_todo with test_todo' '
+	write_and_run_sub_test_lib_test test-expect-todo <<-\EOF &&
+	test_expect_todo "command not implemented yet" "
+		test_todo \
+			--want \"git unimplemented\" \
+			--expect \"test_must_fail git unimplemented\"
+	"
+	test_done
+	EOF
+	check_sub_test_lib_test test-expect-todo <<-\EOF
+	> not ok 1 - command not implemented yet # TODO known breakage
+	> # still have 1 known breakage(s)
+	> 1..1
+	EOF
+'
+
+test_expect_success 'subtest: test_expect_todo with test_todo: prefix + suffix arguments' '
+	write_and_run_sub_test_lib_test test-expect-todo-pfx <<-\EOF &&
+	test_expect_todo "prefix argument for test_todo" "
+		echo x >want &&
+		echo y >expect &&
+		cp expect actual &&
+		test_todo test_cmp \
+			--want want \
+			--expect expect \
+			-- \
+			actual
+	"
+	test_done
+	EOF
+	check_sub_test_lib_test test-expect-todo-pfx <<-\EOF
+	> not ok 1 - prefix argument for test_todo # TODO known breakage
+	> # still have 1 known breakage(s)
+	> 1..1
+	EOF
+'
+
 test_expect_success 'subtest: --verbose option' '
 	write_and_run_sub_test_lib_test_err t1234-verbose --verbose <<-\EOF &&
 	test_expect_success "passing test" true
diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
index 68f69bb5431..fcd5ff4e313 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -369,9 +369,9 @@ test_expect_success 'no segfaults for reflog containing non-commit sha1s' '
 	git reflog refs/tests/tree-in-reflog
 '
 
-test_expect_failure 'reflog with non-commit entries displays all entries' '
+test_expect_todo 'reflog with non-commit entries displays all entries' '
 	git reflog refs/tests/tree-in-reflog >actual &&
-	test_line_count = 3 actual
+	test_todo test_line_count --want "= 3" --expect "= 2" -- actual
 '
 
 # This test takes a lock on an individual ref; this is not supported in
diff --git a/t/t6403-merge-file.sh b/t/t6403-merge-file.sh
index 2f421d967ab..12b334af85c 100755
--- a/t/t6403-merge-file.sh
+++ b/t/t6403-merge-file.sh
@@ -82,9 +82,14 @@ test_expect_success "merge without conflict (--quiet)" '
 	git merge-file --quiet test.txt orig.txt new2.txt
 '
 
-test_expect_failure "merge without conflict (missing LF at EOF)" '
+test_expect_todo "merge without conflict (missing LF at EOF)" '
 	cp new1.txt test2.txt &&
-	git merge-file test2.txt orig.txt new4.txt
+	test_todo \
+		test_expect_code \
+		--expect 1 \
+		--want 0 \
+		-- \
+		git merge-file test2.txt orig.txt new4.txt
 '
 
 test_expect_failure "merge result added missing LF" '
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index a219b126d93..53335393b9b 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -959,6 +959,83 @@ test_path_is_missing () {
 	fi
 }
 
+# Usage: test_todo [<common-prefix>] <options> [-- <common-suffix>...]
+#	--want <want>
+#		The condition we'd like. Injected between
+#		<common-prefix> and <common-suffix> arguments.
+#	--expect <expect>
+#		The condition we have now. Injected in the same way as
+#		the arguments to --want.
+#
+# test_todo is a wrapper for use with "test_expect_todo". It declares
+# an outcome we want, and one we currently expect:
+#
+#	test_todo --want true --expect false
+#
+# It can also take a <common-prefix> prefix along with
+# <common-suffix>... parameters after a "--", e.g.:
+#
+#	# We want 1 line, not 2
+# 	test_todo test_line_count --want "= 1" --expect "= 2" -- actual
+#
+# Here both variants of the "test_line_count" will be run to assert
+# that the "want" variant doesn't pass yet, and that the "expect"
+# variant describes the current behavior.
+#
+# Because we run both neither of them can mutate the test
+# state. I.e. they must be read-only commands such as "wc -l", and not
+# a state-altering command such as "rm".
+test_todo () {
+	local common_fn= &&
+	local have_want= &&
+	local want= &&
+	local expect= &&
+	local have_expect= &&
+	while test $# != 0
+	do
+		case "$1" in
+		--want)
+			want="$2" &&
+			have_want=t &&
+			shift
+			;;
+		--expect)
+			expect="$2" &&
+			have_expect=t &&
+			shift
+			;;
+		--)
+			shift &&
+			break
+			;;
+		*)
+			if test -n "$common_fn"
+			then
+				BUG "the <common-fn> can only be given once" &&
+				return 1
+			fi &&
+			common_fn="$1"
+			;;
+		esac
+		shift
+	done &&
+	if test "$have_want$have_expect" != "tt"
+	then
+		BUG "test_todo must get a --want <want> and --expect <expect>"
+	fi &&
+
+	if $common_fn $want "$@"
+	then
+		BUG "a test_todo succeeded with --want ('$want').  Turn it into a test_expect_success + $@ $want?" &&
+		return 1
+	elif $common_fn $expect "$@"
+	then
+		say "a test_todo will succeed with --expect ('$expect'), we eventually want '$want' instead" >&3 &&
+		return 0
+	fi &&
+	BUG "a test_todo didn't pass with either --want ('$want') or --expect ('$expect')"
+}
+
 # test_line_count checks that a file has the number of lines it
 # ought to. For example:
 #
-- 
2.35.1.1436.g756b814e59f


  parent reply	other threads:[~2022-03-18  0:34 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-18  0:33 [PATCH 0/7] test-lib-functions: a better "test_expect_failure" Ævar Arnfjörð Bjarmason
2022-03-18  0:33 ` [PATCH 1/7] test-lib: add a "test_expect_todo", similar to "test_expect_failure" Ævar Arnfjörð Bjarmason
2022-03-18 18:59   ` Junio C Hamano
2022-03-18 20:50     ` Junio C Hamano
2022-03-18 23:07       ` Ævar Arnfjörð Bjarmason
2022-03-19  7:12         ` Junio C Hamano
2022-03-19 11:11           ` Ævar Arnfjörð Bjarmason
2022-03-20 15:13             ` Phillip Wood
2022-03-20 18:07               ` Junio C Hamano
2022-03-22 14:43                 ` Derrick Stolee
2022-03-23 22:13                   ` Junio C Hamano
2022-03-24 11:24                     ` Phillip Wood
2022-03-18  0:33 ` Ævar Arnfjörð Bjarmason [this message]
2022-03-18  0:33 ` [PATCH 3/7] tests: allow test_* in "test_must_fail_acceptable" for "test_todo" Ævar Arnfjörð Bjarmason
2022-03-18  0:33 ` [PATCH 4/7] test-lib-functions: add and use a "todo_test_cmp" helper Ævar Arnfjörð Bjarmason
2022-03-18  0:34 ` [PATCH 5/7] test-lib-functions: add and use a "todo_test_path" helper Ævar Arnfjörð Bjarmason
2022-03-18  0:34 ` [PATCH 6/7] test-lib-functions: make test_todo support a --reset Ævar Arnfjörð Bjarmason
2022-03-18  0:34 ` [PATCH 7/7] sparse tests: convert a TODO test to use "test_expect_todo" Ævar Arnfjörð Bjarmason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=patch-2.7-0834a3ed049-20220318T002951Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=johncai86@gmail.com \
    --cc=newren@gmail.com \
    --cc=stolee@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).