[PATCH 3/3] tests: add a special test setup that runs "git fsck" before exiting

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

Add the ability to run the tests with GIT_TEST_FSCK=true in the
environment. If set we'll run "git fsck" at the end of every test, and
those tests that fail need to annotate what their failure was.

The goal is to detect regressions in fsck that our tests might
otherwise miss. We had one such regression in c68b489e56 ("fsck: parse
loose object paths directly", 2017-01-13) released with Git 2.12.0,
which wasn't spotted more than a year and a half later during the
2.20.0 window.

As it turns out there already was a test for what triggerd that bug
all along in the form of t5000-tar-tree.sh, we just weren't running
"git fsck" at the end[1].

That specific bug has been fixed in ("check_stream_sha1(): handle
input underflow", 2018-10-30)[1], but since we have a demonstrable
history of not anticipating which tests which would make "git fsck"
fail need to be made part of the "git fsck" test suite let's add this
test mode to cover potential blind spots. The "git fsck" command is
also something where we might expect that during our RC windows users
aren't actively testing on already corrupt repositories, so "in the
wild" test coverage will be spotty, so we need all the help we can
get.

1. https://public-inbox.org/git/878t2fkxrn.fsf@evledraar.gmail.com/
2. https://public-inbox.org/git/20181030232312.GB32038@sigill.intra.peff.net/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 t/README                |  5 +++++
 t/t0000-basic.sh        | 26 ++++++++++++++++++++++++++
 t/test-lib-functions.sh |  2 ++
 t/test-lib.sh           | 33 +++++++++++++++++++++++++++++++++
 4 files changed, 66 insertions(+)

diff --git a/t/README b/t/README
index 8847489640..092f78b3d7 100644
--- a/t/README
+++ b/t/README
@@ -343,6 +343,11 @@ of the index for the whole test suite by bypassing the default number of
 cache entries and thread minimums. Setting this to 1 will make the
 index loading single threaded.
 
+GIT_TEST_FSCK=<boolean> if true arranges for "git fsck" to be run at
+the end of the test scripts. Those tests that fail will need to set a
+"GIT_TEST_FSCK_TESTS" variable before we enter "test_done" with a test
+fragment to test that fsck.{out,err} is the expected failure.
+
 Naming Tests
 ------------
 
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 4d23373526..8e667e6691 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -19,6 +19,7 @@ modification *should* take notice and update the test vectors here.
 '
 
 . ./test-lib.sh
+unset GIT_TEST_FSCK
 
 try_local_x () {
 	local x="local" &&
@@ -393,6 +394,31 @@ test_expect_success 'GIT_SKIP_TESTS sh pattern' "
 	)
 "
 
+test_expect_success 'GIT_TEST_FSCK=true' "
+	test_when_finished 'sane_unset GIT_TEST_FSCK' &&
+	GIT_TEST_FSCK=true &&
+	export GIT_TEST_FSCK &&
+	run_sub_test_lib_test run-git-fsck-test \
+		'--run basic' --run='1 3 5' <<-\\EOF &&
+	for i in 1 2 3 4 5 6
+	do
+		test_expect_success \"passing test #\$i\" 'true'
+	done
+	GIT_TEST_FSCK=true test_done
+	EOF
+	check_sub_test_lib_test run-git-fsck-test <<-\\EOF
+	> ok 1 - passing test #1
+	> ok 2 # skip passing test #2 (--run)
+	> ok 3 - passing test #3
+	> ok 4 # skip passing test #4 (--run)
+	> ok 5 - passing test #5
+	> ok 6 # skip passing test #6 (--run)
+	> ok 7 # skip git fsck at end (due to GIT_TEST_FSCK) (expected to succeed) (--run)
+	> # passed all 7 test(s)
+	> 1..7
+	EOF
+"
+
 test_expect_success '--run basic' "
 	run_sub_test_lib_test run-basic \
 		'--run basic' --run='1 3 5' <<-\\EOF &&
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 78d8c3783b..7d002ff5aa 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -470,6 +470,7 @@ test_expect_success () {
 # Usage: test_external description command arguments...
 # Example: test_external 'Perl API' perl ../path/to/test.pl
 test_external () {
+	unset GIT_TEST_FSCK
 	test "$#" = 4 && { test_prereq=$1; shift; } || test_prereq=
 	test "$#" = 3 ||
 	error >&5 "bug in the test script: not 3 or 4 parameters to test_external"
@@ -511,6 +512,7 @@ test_external () {
 # Like test_external, but in addition tests that the command generated
 # no output on stderr.
 test_external_without_stderr () {
+	unset GIT_TEST_FSCK
 	# The temporary file has no (and must have no) security
 	# implications.
 	tmp=${TMPDIR:-/tmp}
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 897e6fcc94..5f7f5595e3 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -454,6 +454,8 @@ GIT_EXIT_OK=
 trap 'die' EXIT
 trap 'exit $?' INT
 
+GIT_TEST_FSCK_TESTS=
+
 # The user-facing functions are loaded from a separate file so that
 # test_perf subshells can have them too
 . "$TEST_DIRECTORY/test-lib-functions.sh"
@@ -789,7 +791,36 @@ test_at_end_hook_ () {
 	:
 }
 
+_test_done_fsck() {
+	desc='git fsck at end (due to GIT_TEST_FSCK)'
+	if test -n "$GIT_TEST_FSCK_TESTS"
+	then
+		test_expect_success "$desc (expected to fail)" '
+			test_must_fail git fsck 2>fsck.err >fsck.out
+		'
+		test_expect_success "$desc (expected to fail) -- assert failure mode" "
+			test_path_exists fsck.err &&
+			test_path_exists fsck.out &&
+			$GIT_TEST_FSCK_TESTS
+		"
+	else
+		test_expect_success "$desc (expected to succeed)" '
+			git fsck
+		'
+	fi
+}
+
 test_done () {
+	# Don't want to run this under TEST_NO_CREATE_REPO, otherwise
+	# we end up sloowly running "git fsck" against git.git
+	if test -z "$TEST_NO_CREATE_REPO" &&
+		    # test -n first so all --verbose output isn't
+		    # polluted with this check
+		    test -n "$GIT_TEST_FSCK" &&
+		    test_have_prereq TEST_FSCK
+	then
+		_test_done_fsck
+	fi
 	GIT_EXIT_OK=t
 
 	if test -z "$HARNESS_ACTIVE"
@@ -1268,3 +1299,5 @@ test_lazy_prereq CURL '
 test_lazy_prereq SHA1 '
 	test $(git hash-object /dev/null) = e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
 '
+
+test_lazy_prereq TEST_FSCK 'test-tool env-bool GIT_TEST_FSCK'
-- 
2.19.1.899.g0250525e69