[PATCH] t4014: fix call to `test_expect_success ()`

[PATCH] t4014: fix call to `test_expect_success ()`

[Patrick Steinhardt]

We have added a couple of new tests to t4014 in 6005932d95
(format-patch: add ability to use alt cover format, 2026-03-07). One of
the tests has typoed the call to `test_expect_success ()` and instead
invokes `test_expected_success ()`. Fix this.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Hi,

this fixes a test bug in one of the new tests introduced via
"mf/format-patch-cover-letter-format". Thanks!

Patrick
---
 t/t4014-format-patch.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 7c67bdf922..4f8967e283 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -393,7 +393,7 @@ test_expect_success 'cover letter with subject, author and count' '
 	test_line_count = 1 result
 '
 
-test_expected_success 'cover letter with author and count' '
+test_expect_success 'cover letter with author and count' '
 	test_when_finished "git reset --hard HEAD~1" &&
 	test_when_finished "rm -rf patches result test_file" &&
 	touch test_file &&

---
base-commit: 927a571e75d06037d46dc9ef5fe26b0dc37bbff6
change-id: 20260324-b4-pks-t4014-fix-test-execution-b2fb17a880f9

Re: [PATCH] t4014: fix call to `test_expect_success ()`

[Mirko Faina]

On Tue, Mar 24, 2026 at 03:52:30PM +0100, Patrick Steinhardt wrote:
> We have added a couple of new tests to t4014 in 6005932d95
> (format-patch: add ability to use alt cover format, 2026-03-07). One of
> the tests has typoed the call to `test_expect_success ()` and instead
> invokes `test_expected_success ()`. Fix this.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> Hi,
> 
> this fixes a test bug in one of the new tests introduced via
> "mf/format-patch-cover-letter-format". Thanks!
> 
> Patrick

This has already been fixed in a follow-up series under
mf/format-patch-cover-letter-format [1].

Thank you

[1] https://lore.kernel.org/git/5d061d6398bae368a7cc95700b5df44854d1d8e8.1774284699.git.mroik@delayed.space

Re: [PATCH] t4014: fix call to `test_expect_success ()`

[Junio C Hamano]

Mirko Faina <mroik@delayed.space> writes:

> On Tue, Mar 24, 2026 at 03:52:30PM +0100, Patrick Steinhardt wrote:
>> We have added a couple of new tests to t4014 in 6005932d95
>> (format-patch: add ability to use alt cover format, 2026-03-07). One of
>> the tests has typoed the call to `test_expect_success ()` and instead
>> invokes `test_expected_success ()`. Fix this.
>> 
>> Signed-off-by: Patrick Steinhardt <ps@pks.im>
>> ---
>> Hi,
>> 
>> this fixes a test bug in one of the new tests introduced via
>> "mf/format-patch-cover-letter-format". Thanks!
>> 
>> Patrick
>
> This has already been fixed in a follow-up series under
> mf/format-patch-cover-letter-format [1].
>
> Thank you
>
> [1] https://lore.kernel.org/git/5d061d6398bae368a7cc95700b5df44854d1d8e8.1774284699.git.mroik@delayed.space

Could either of you remind us why "make test" did not catch this?

Thanks.

Re: [PATCH] t4014: fix call to `test_expect_success ()`

[Mirko Faina]

On Tue, Mar 24, 2026 at 08:38:49AM -0700, Junio C Hamano wrote:
> Could either of you remind us why "make test" did not catch this?

My bad. At the time when I ran it I simply saw no failing tests and
assumed everything worked fine. Next time I'll check that the actual
name of the test is present in the output.

Re: [PATCH] t4014: fix call to `test_expect_success ()`

[Junio C Hamano]

Mirko Faina <mroik@delayed.space> writes:

> On Tue, Mar 24, 2026 at 08:38:49AM -0700, Junio C Hamano wrote:
>> Could either of you remind us why "make test" did not catch this?
>
> My bad. At the time when I ran it I simply saw no failing tests and
> assumed everything worked fine. Next time I'll check that the actual
> name of the test is present in the output.

No, I wasn't complaining a human tester not running tests.

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" &&
                rm -f "$REAL/objects/$(objpath $SHA)" &&
                git update-index --add bar &&
                test_path_is_file "$REAL/index" &&
                test_path_is_file "$REAL/objects/$(objpath $SHA)"

        ok 6 - check update-index
        ...
        expecting success of 0002.13 'enter_repo strict mode':
                head=$(git -C enter_repo rev-parse HEAD) &&
                ...
                test_cmp expected actual

        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".

Re* [PATCH] t4014: fix call to `test_expect_success ()`

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> 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 <t/test-lib.sh>, 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<RETURN>" 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>

         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 <gitster@pobox.com>
---
 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

[PATCH] t6002: make test "set -e" clean

[Junio C Hamano]

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.

We often use 

      val=$(expr expression)

only for the computation, and it is good that "expr" exits non-zero
with syntactically invalid expression (it exits with 2) and other
errors (with 3).

"expr" however also exits with "1" if it yields 0 or null X-<.

Make sure we do not fail unnecessarily under "set -e".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * It was fun to figure this one out.

 t/t6002-rev-list-bisect.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git i/t/t6002-rev-list-bisect.sh w/t/t6002-rev-list-bisect.sh
index daa009c9a1..1bd720d240 100755
--- i/t/t6002-rev-list-bisect.sh
+++ w/t/t6002-rev-list-bisect.sh
@@ -27,9 +27,9 @@ test_bisection_diff()
 	# Test if bisection size is close to half of list size within
 	# tolerance.
 	#
-	_bisect_err=$(expr $_list_size - $_bisection_size \* 2)
+	_bisect_err=$(expr $_list_size - $_bisection_size \* 2) && test $? -le 1
 	test "$_bisect_err" -lt 0 && _bisect_err=$(expr 0 - $_bisect_err)
-	_bisect_err=$(expr $_bisect_err / 2) ; # floor
+	_bisect_err=$(expr $_bisect_err / 2) && test $? -le 1; # floor
 
 	test_expect_success \
 	"bisection diff $_bisect_option $_head $* <= $_max_diff" \

[PATCH] test-lib: catch misspelt 'test_expect_successo'

[Junio C Hamano]

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.

All tests dot-source "test-lib.sh" as the first thing to do.
Starting the script with "set -e" immediately reveals one place in
the test framework itself that is not clean.

The test framework runs "$GIT_BINARY" without any argument. We
expect it to exit with status 1 (i.e., "git<RETURN>" that spits out
the list of common commands) as a sign that we have an instance of
Git that we want to test.  We cannot quite say

    git
    if test $? != 1; then you have not built git; fi

as the first invocation that exits non-zero is caught with "set -e".

Work this around by rewriting the construct like so:

    status=0; git || status=$?
    if test $status != 1; then you have not built git; fi

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * As we look into other breakages, we may discover more breakages
   in the test framework that need to be fixed, but this change
   alone seems to get thing going for many test scripts.

 t/test-lib.sh | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git i/t/test-lib.sh w/t/test-lib.sh
index 70fd3e9baf..a2aa97fba3 100644
--- i/t/test-lib.sh
+++ w/t/test-lib.sh
@@ -17,6 +17,9 @@
 
 # Test the binaries we have just built.  The tests are kept in
 # t/ subdirectory and are run in 'trash directory' subdirectory.
+
+set -e
+
 if test -z "$TEST_DIRECTORY"
 then
 	# ensure that TEST_DIRECTORY is an absolute path so that it
@@ -143,8 +146,8 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 ################################################################
 # 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
+status=0 && "$GIT_BINARY" >/dev/null || status=$?
+if test $status != 1
 then
 	if test -n "$GIT_TEST_INSTALLED"
 	then

[PATCH] t0008: make test "set -e" clean

[Junio C Hamano]

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 piece of script used "grep" to filter out its input purely for its
output, but of course, "grep" reports with its exit value when it
did not see any hits, which didn't mesh quite well with "set -e".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t0008-ignores.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git i/t/t0008-ignores.sh w/t/t0008-ignores.sh
index db8bde280e..8edb08d9c2 100755
--- i/t/t0008-ignores.sh
+++ w/t/t0008-ignores.sh
@@ -122,7 +122,7 @@ test_expect_success_multiple () {
 	fi
 	testname="$1" expect_all="$2" code="$3"
 
-	expect_verbose=$( echo "$expect_all" | grep -v '^::	' )
+	expect_verbose=$( echo "$expect_all" | grep -v '^::	' ) || :
 	expect=$( echo "$expect_verbose" | sed -e 's/.*	//' )
 
 	test_expect_success $prereq "$testname${no_index_opt:+ with $no_index_opt}" '

[PATCH] t7450: make test "set -e" clean

[Junio C Hamano]

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.

Often we write "A && test_expect_success ..." and want it to mean
"If and only if A holds true, this needs to be tested", but under
"set -e", this will cause failure when A does not hold true.  We
need to write "!A || test_expect_success ..." if we want to run the
test conditionally.

Or write it properly with if/then/fi, perhaps like:

	if ! A
	then
		test_expect_success ...
	fi

Make sure we do not fail unnecessarily under "set -e".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t7450-bad-git-dotfiles.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git i/t/t7450-bad-git-dotfiles.sh w/t/t7450-bad-git-dotfiles.sh
index f512eed278..047e4085d7 100755
--- i/t/t7450-bad-git-dotfiles.sh
+++ w/t/t7450-bad-git-dotfiles.sh
@@ -220,7 +220,7 @@ check_dotx_symlink () {
 		)
 	'
 
-	test -n "$refuse_index" &&
+	test -z "$refuse_index" ||
 	test_expect_success "refuse to load symlinked $name into index ($type)" '
 		test_must_fail \
 			git -C $dir \

Re: [PATCH] t7450: make test "set -e" clean

[Eric Sunshine]

On Tue, Mar 24, 2026 at 2:32 PM Junio C Hamano <gitster@pobox.com> wrote:
> 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.
>
> Often we write "A && test_expect_success ..." and want it to mean
> "If and only if A holds true, this needs to be tested", but under
> "set -e", this will cause failure when A does not hold true.  We
> need to write "!A || test_expect_success ..." if we want to run the
> test conditionally.
>
> Or write it properly with if/then/fi, perhaps like:
>
>         if ! A
>         then
>                 test_expect_success ...
>         fi
>
> Make sure we do not fail unnecessarily under "set -e".
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> diff --git i/t/t7450-bad-git-dotfiles.sh w/t/t7450-bad-git-dotfiles.sh
> @@ -220,7 +220,7 @@ check_dotx_symlink () {
> -       test -n "$refuse_index" &&
> +       test -z "$refuse_index" ||
>         test_expect_success "refuse to load symlinked $name into index ($type)" '
>                 test_must_fail \
>                         git -C $dir \

I suppose this is the absolute minimum change to make this work, but
typically we would handle this sort of case by defining a PREREQ,
wouldn't we? Using a PREREQ would also set a better example for those
new to the codebase.

Re: [PATCH] t7450: make test "set -e" clean

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Tue, Mar 24, 2026 at 2:32 PM Junio C Hamano <gitster@pobox.com> wrote:
>> 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.
>>
>> Often we write "A && test_expect_success ..." and want it to mean
>> "If and only if A holds true, this needs to be tested", but under
>> "set -e", this will cause failure when A does not hold true.  We
>> need to write "!A || test_expect_success ..." if we want to run the
>> test conditionally.
>>
>> Or write it properly with if/then/fi, perhaps like:
>>
>>         if ! A
>>         then
>>                 test_expect_success ...
>>         fi
>>
>> Make sure we do not fail unnecessarily under "set -e".
>>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>> diff --git i/t/t7450-bad-git-dotfiles.sh w/t/t7450-bad-git-dotfiles.sh
>> @@ -220,7 +220,7 @@ check_dotx_symlink () {
>> -       test -n "$refuse_index" &&
>> +       test -z "$refuse_index" ||
>>         test_expect_success "refuse to load symlinked $name into index ($type)" '
>>                 test_must_fail \
>>                         git -C $dir \
>
> I suppose this is the absolute minimum change to make this work, but
> typically we would handle this sort of case by defining a PREREQ,
> wouldn't we? Using a PREREQ would also set a better example for those
> new to the codebase.

In some situations, maybe, but I do not think this one is a good fit
for a prerequisite, whose typical pattern is "let's see what we have
in the executing platform environment just once, and act accordingly".

This is a "the outside helper function is repeatedly called, and the
caller may or may not call it with an option, depending on which
this extra test may or may not make sense to run, so run this one
conditionally".

Re: [PATCH] test-lib: catch misspelt 'test_expect_successo'

[Jeff King]

On Tue, Mar 24, 2026 at 11:13:26AM -0700, Junio C Hamano wrote:

> 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.

Using "-e" makes me very nervous, given all of its quirks. Granted, most
of them are related to it _not_ kicking in when you'd want it to, but I
worry it will create false positive/negative headaches.

In the past I've caught errors outside of the test snippet by noticing
cruft on stderr. This is especially obvious if you use "prove", which
captures stdout and gives a nice display (which the extra stderr then
makes uglier).

I wonder if we could automate / formalize that. If we do this hacky
patch on master:

diff --git a/t/Makefile b/t/Makefile
index ab8a5b54aa..f57180cc7b 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -79,7 +79,7 @@ prove: pre-clean $(TEST_LINT)
 	$(MAKE) clean-except-prove-cache
 
 $(T):
-	@echo "*** $@ ***"; '$(TEST_SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS)
+	echo "*** $@ ***"; '$(TEST_SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS) 2>$@.stderr
 
 $(UNIT_TESTS):
 	@echo "*** $@ ***"; $@

then:

  cd t
  make test
  for i in *.stderr; do test -s $i && echo $i; done

catches the problem in t4014 and nothing else. Note that it _doesn't_
work with --verbose-log, though, as that redirects stderr to stdout
(which is going to the log). It might be possible to do something
cleaner and more clever within test-lib.sh, though.

-Peff

Re: [PATCH] test-lib: catch misspelt 'test_expect_successo'

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Tue, Mar 24, 2026 at 11:13:26AM -0700, Junio C Hamano wrote:
>
>> 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.
>
> Using "-e" makes me very nervous, given all of its quirks. Granted, most
> of them are related to it _not_ kicking in when you'd want it to, but I
> worry it will create false positive/negative headaches.

After looking at a few scripts, I am not suffering from such
headaches yet; it does not look too bad.  I'll stop this effort for
now, but with a handful of patches I already sent, more than 80-90%
of the entire test scripts that I run are now "set -e" clean, I
think.  Note that I do not run svn, cvs, or p4 tests ;-)

> In the past I've caught errors outside of the test snippet by noticing
> cruft on stderr. This is especially obvious if you use "prove", which
> captures stdout and gives a nice display (which the extra stderr then
> makes uglier).
> I wonder if we could automate / formalize that.

That's a thought.

Re: [PATCH] test-lib: catch misspelt 'test_expect_successo'

[Jeff King]

On Tue, Mar 24, 2026 at 12:48:24PM -0700, Junio C Hamano wrote:

> > Using "-e" makes me very nervous, given all of its quirks. Granted, most
> > of them are related to it _not_ kicking in when you'd want it to, but I
> > worry it will create false positive/negative headaches.
> 
> After looking at a few scripts, I am not suffering from such
> headaches yet; it does not look too bad.  I'll stop this effort for
> now, but with a handful of patches I already sent, more than 80-90%
> of the entire test scripts that I run are now "set -e" clean, I
> think.  Note that I do not run svn, cvs, or p4 tests ;-)

Clean in the sense that you don't _notice_ any problems. But there may
be lurking ones. For example, given this:

  set -e
  foo() {
	false
	echo foo
  }

what would you expect the output to be for:

  echo before &&
  foo &&
  echo after

versus:

  echo before &&
  foo

Whether that "false" triggers "-e" depends on where in the &&-chain the
call to the containing function is. So things that are not problems now
may suddenly become ones when far-away code is changed.

Maybe it's enough that people would notice and debug them when they
happen (if "set -e" is in test-lib.sh), and they wouldn't come up all
that much. I dunno. I just have been bitten enough by "-e" quirks that
I'm wary.

-Peff

Re: Re* [PATCH] t4014: fix call to `test_expect_success ()`

[Patrick Steinhardt]

On Tue, Mar 24, 2026 at 10:13:09AM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> 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 <t/test-lib.sh>, 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<RETURN>" 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>
> 
>          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.

Yeah, I was playing around with the same idea yesterday, but got pulled
into some meetings and thus couldn't finish that work.

> 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 think it's overall not that bad, and I've got something that's almost
done. It's an easy win for students indeed, but in this case I'd rather
make our test suite a bit more robust sooner rather than later :)

I'll likely have something later today. Thanks!

Patrick