git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Oct 31, 2018, 12:42 UTC
Message-ID
<20181031124208.29451-4-avarab@gmail.com>
In-Reply-To
<20181030232337.GC32038@sigill.intra.peff.net>

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
Previous: Junio C HamanoNext: Torsten Bögershausen
Message 29 of 39 in “"git fsck" not detecting garbage at the end of blob object files...”
  1. John SzakmeisterJan 7, 2017
  2. Dennis KaarsemakerJan 7, 2017
  3. Jeff KingJan 8, 2017
  4. John SzakmeisterJan 13, 2017
  5. 0/6 loose-object fsck fixes/tighteningJeff King, Jan 13, 2017
  6. 1/6 t1450: refactor loose-object removalJeff King, Jan 13, 2017
  7. 2/6 sha1_file: fix error message for alternate objectsJeff King, Jan 13, 2017
  8. 3/6 t1450: test fsck of packed objectsJeff King, Jan 13, 2017
  9. 4/6 sha1_file: add read_loose_object() functionJeff King, Jan 13, 2017
  10. 5/6 fsck: parse loose object paths directlyJeff King, Jan 13, 2017
  11. Infinite loop regression in git-fsck in v2.12.0Ævar Arnfjörð Bjarmason, Oct 30, 2018
  12. Jeff KingOct 30, 2018
  13. Junio C HamanoOct 30, 2018
  14. Jeff KingOct 30, 2018
  15. Jeff KingOct 30, 2018
  16. 1/3 t1450: check large blob in trailing-garbage testJeff King, Oct 30, 2018
  17. 2/3 check_stream_sha1(): handle input underflowJeff King, Oct 30, 2018
  18. Junio C HamanoOct 31, 2018
  19. Jeff KingOct 31, 2018
  20. Junio C HamanoOct 31, 2018
  21. Jeff KingOct 31, 2018
  22. Jeff KingOct 31, 2018
  23. Junio C HamanoOct 31, 2018
  24. 3/3 cat-file: handle streaming failures consistentlyJeff King, Oct 30, 2018
  25. 0/3 Add a GIT_TEST_FSCK test modeÆvar Arnfjörð Bjarmason, Oct 31, 2018
  26. 1/3 tests: add a "env-bool" helper to test-toolÆvar Arnfjörð Bjarmason, Oct 31, 2018
  27. 2/3 tests: mark those tests where "git fsck" fails at the endÆvar Arnfjörð Bjarmason, Oct 31, 2018
  28. Junio C HamanoNov 1, 2018
  29. 3/3 tests: add a special test setup that runs "git fsck" before exitingÆvar Arnfjörð Bjarmason, Oct 31, 2018
  30. Torsten BögershausenOct 31, 2018
  31. Junio C HamanoOct 31, 2018
  32. Jeff KingOct 31, 2018
  33. Eric SunshineOct 31, 2018
  34. Jeff KingOct 31, 2018
  35. Ævar Arnfjörð BjarmasonOct 30, 2018
  36. Jeff KingOct 30, 2018
  37. 6/6 fsck: detect trailing garbage in all object typesJeff King, Jan 13, 2017
  38. John SzakmeisterJan 19, 2017
  39. John SzakmeisterJan 13, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.