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

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

From
Patrick Steinhardt <ps@pks.im>
Date
Mar 25, 2026, 07:07 UTC
Message-ID
<acOJmBluqb5SvjpW@pks.im>
In-Reply-To
<xmqqcy0t178a.fsf_-_@gitster.g>
On Tue, Mar 24, 2026 at 10:13:09AM -0700, Junio C Hamano wrote:
Show 78 quoted lines
> 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.

Show 5 quoted lines
> 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
Previous: Junio C Hamano
Message 16 of 16 in “t4014: fix call to `test_expect_success ()`”
  1. t4014: fix call to `test_expect_success ()`Patrick Steinhardt, Mar 24, 2026
  2. Mirko FainaMar 24, 2026
  3. Junio C HamanoMar 24, 2026
  4. Mirko FainaMar 24, 2026
  5. Junio C HamanoMar 24, 2026
  6. Re* [PATCH] t4014: fix call to `test_expect_success ()`Junio C Hamano, Mar 24, 2026
  7. t6002: make test "set -e" cleanJunio C Hamano, Mar 24, 2026
  8. test-lib: catch misspelt 'test_expect_successo'Junio C Hamano, Mar 24, 2026
  9. Jeff KingMar 24, 2026
  10. Junio C HamanoMar 24, 2026
  11. Jeff KingMar 25, 2026
  12. t0008: make test "set -e" cleanJunio C Hamano, Mar 24, 2026
  13. t7450: make test "set -e" cleanJunio C Hamano, Mar 24, 2026
  14. Eric SunshineMar 24, 2026
  15. Junio C HamanoMar 24, 2026
  16. Patrick SteinhardtMar 25, 2026

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.