From: Junio C Hamano Date: Tue, 24 Mar 2026 17:13:09 GMT Subject: Re* [PATCH] t4014: fix call to `test_expect_success ()` Message-ID: In-Reply-To: Junio C Hamano writes: > I was wondering if we can make the test framework better so that a > misspelt test_expect_success would cause a louder failure than what > we have now, which is something like: > > ... > ok 5 - check hash-object > > t0002-gitfile.sh: line 46: test_expect_successo: command not found > expecting success of 0002.6 'check update-index': > test_path_is_missing "$REAL/index" && > ... > ok 13 - enter_repo strict mode > > # passed all 13 test(s) > 1..13 > > when I corrupt the 6th test of a random script. > > diff --git i/t/t0002-gitfile.sh w/t/t0002-gitfile.sh > index dfbcdddbcc..d65f664914 100755 > --- i/t/t0002-gitfile.sh > +++ w/t/t0002-gitfile.sh > @@ -43,7 +43,7 @@ test_expect_success 'check hash-object' ' > test_path_is_file "$REAL/objects/$(objpath $SHA)" > ' > > -test_expect_success 'check cat-file' ' > +test_expect_successo 'check cat-file' ' > git cat-file blob $SHA >actual && > test_cmp bar actual > ' > > There is no indication of something bad happened, other than > "command not found" and 13 tests passed instead of 14 the script > has, which nobody knows. > > So, no, it hardly is your fault. > > I wonder if the test framework is safe to run with "set -e". It turns out that the test framework itself is not so clean. If I add "set -e" near the beginning of , the first roadblock we hit is this one: # It appears that people try to run tests without building... GIT_BINARY="${GIT_TEST_INSTALLED:-$GIT_BUILD_DIR}/git$X" "$GIT_BINARY" >/dev/null if test $? != 1 then ... complain that you haven't built and ... exit 1 fi With "set -e", "$GIT_BINARY" we expect to exit with status 1 (i.e., "git" that spits out the list of common commands) as a sign that we have an instance of Git that we want to test is not even allowed to do so. I did this single liner at the end of t/test-lib.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git c/t/test-lib.sh w/t/test-lib.sh index 70fd3e9baf..4a80933487 100644 --- c/t/test-lib.sh +++ w/t/test-lib.sh @@ -1971,3 +1971,5 @@ test_lazy_prereq FSMONITOR_DAEMON ' git version --build-options >output && grep "feature: fsmonitor--daemon" output ' + +set -e and started running "make test". I see some failures I haven't yet looked into, but it seems promising. Fixing all may involve finding and fixing little things like the attached patch. I am not sure if this would be a good microproject canidate for the next year. There are a handful of them that multiple students can work on independently, but some of them require familiarity with the test framework and shell scripting. I'll stop at marking this #leftoverbits but it probably is not for microproject. ---- >8 ---- Subject: [PATCH] t4032: make test "set -e" clean In order to catch mistakes like misspelling "test_expect_success", we would like to eventually be able to run our test suite with the "-e" option on. A few shell construct used in this test were not ready. Make them so. * "git config --unset VAR" can fail when VAR is not defined. * The author of "test -f X && run test that uses X" written here really wanted to say "if file X is there, then run the test", not "file X must exist and the test using it must succeed". The proper way to express it is to say "test ! -f X || use X". Signed-off-by: Junio C Hamano --- t/t4032-diff-inter-hunk-context.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git c/t/t4032-diff-inter-hunk-context.sh w/t/t4032-diff-inter-hunk-context.sh index bada0cbd32..efcd863126 100755 --- c/t/t4032-diff-inter-hunk-context.sh +++ w/t/t4032-diff-inter-hunk-context.sh @@ -17,7 +17,7 @@ f() { t() { use_config= - git config --unset diff.interHunkContext + git config --unset diff.interHunkContext || : case $# in 4) hunks=$4; cmd="diff -U$3";; @@ -40,7 +40,7 @@ t() { test $(git $cmd $file | grep '^@@ ' | wc -l) = $hunks " - test -f $expected && + test ! -f $expected || test_expect_success "$label: check output" " git $cmd $file | grep -v '^index ' >actual && test_cmp $expected actual