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

Re: [PATCH] tests: add a special setup where prerequisites fail

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
May 14, 2019, 08:53 UTC
Message-ID
<nycvar.QRO.7.76.6.1905140945220.44@tvgsbejvaqbjf.bet>
In-Reply-To
<20190513183242.10600-1-avarab@gmail.com>
Hi Ævar,
On Mon, 13 May 2019, Ævar Arnfjörð Bjarmason wrote:
Show 12 quoted lines
> As discussed in [1] there's a regression in the "pu" branch now
> because a new test implicitly assumed that a previous test guarded by
> a prerequisite had been run. Add a "GIT_TEST_FAIL_PREREQS" special
> test setup where we'll skip (nearly) all tests guarded by
> prerequisites, allowing us to easily emulate those platform where we
> don't run these tests.
>
> As noted in the documentation I'm adding I'm whitelisting the SYMLINKS
> prerequisite for now. A lot of tests started failing if we lied about
> not supporting symlinks. It's also unlikely that we'll have a failing
> test due to a hard dependency on symlinks without that being the
> obvious cause, so for now it's not worth the effort to make it work.
I don't know... In Git for Windows, the SYMLINKS prereq is not met.

(Side note: Windows 10 already supports symlinks for quite some time, even for non-admin developers, but the fact that Git's test suite is implemented in shell script bites us yet one more time: MSYS2 has a completely different idea what symlinks are. It uses the "system file" bit that only exists on Windows, and if that is set, reads the beginning of the file, and if that reads "!<symlink>" (interpreted as ASCII), then the rest of that system file is interpreted as the symlink target.)

So it makes me worry if you say that you had to exclude the SYMLINK prereq. Maybe all the dependent tests have different prereqs that just so happen *also* not to be met on Windows?

Show 27 quoted lines
> 1. https://public-inbox.org/git/nycvar.QRO.7.76.6.1905131531000.44@tvgsbejvaqbjf.bet/
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>
> On Mon, May 13 2019, Johannes Schindelin wrote:
>
> > [...]
> > Namely, when test cases 51 and 52 are skipped because of a missing GPG
> > prerequisite [*1*], and those two are obviously required to run for the
> > `git merge to fail in your test case, as you can very easily verify by
> > downloading the artifact containing the `trash directory.t7600-merge`
> > directory and re-running the last steps on Linux (where the `git -c
> > rerere.enabled=true merge master` *succeeds*).
> >
> > In fact, you can very, very easily emulate the whole situation on your box
> > by running:
> >
> > 	sh t7600-merge.sh -i -v -x --run=1-50,53-59
> >
> > And then you can fix your test case so that it does not need to rely on
> > test cases that may, or may not, have run previously.
>
> I think it would be better to more pro-actively spot this sort of
> thing in the future, so here's a patch to do that. It passes on
> "master", but fails on "pu" due to the issue with the one test being
> discussed here.

It does drive me nuts that the `--run=<N>` option exists (thank you, Slavica, for teaching me!) and is so poorly supported by our test suite.

For example, if t7600.59 fails, it would make a ton of sense to run
	sh t7600-merge.sh -i -v -x --run=59
right?

Except that we frequently have at least one "test case" whose only purpose is to set things up.

But we never declare explicitly "test case 59 requires test case 51 to run first".

We do not even declare the test cases: we execute them immediately. So we would not even be able to juggle them about, e.g. run them in reverse (which would otherwise be the easiest way to get rid of almost all side effects).

I think in the long run, we'll have to drag Git's test suite into the 21st century (kicking and screaming, I'm sure), to have a more declarative style, with those features that one might know from Mocha, Jest, JUnit, xUnit.NET, etc.

Back to your patch: it only catches prereq problems. But the `--run=59` thing would still not be addressed.

What would you think about a mode where random test cases are skipped? It would have to make sure to provide a way to recreate the problem, e.g. giving a string that defines exactly which test cases were skipped.

I am *sure* that tons of test scripts would fail with that, and we would probably have to special-case the `setup` "test cases", and we would have to clean up quite a few scripts to *not* execute random stuff outside of `test_expect_*`...

Show 9 quoted lines
> diff --git a/t/README b/t/README
> index 6404f33e19..9747971d58 100644
> --- a/t/README
> +++ b/t/README
> @@ -334,6 +334,15 @@ that cannot be easily covered by a few specific test cases. These
>  could be enabled by running the test suite with correct GIT_TEST_
>  environment set.
>
> +GIT_TEST_FAIL_PREREQS<non-empty?> fails all prerequisites. This is
Did you mean to insert `=` after `GIT_TEST_FAIL_PREREQS`?
Show 10 quoted lines
> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
> index 819c24d10e..c20209324c 100755
> --- a/t/t4202-log.sh
> +++ b/t/t4202-log.sh
> @@ -352,7 +352,7 @@ test_expect_success 'log with grep.patternType configuration and command line' '
>  	test_cmp expect actual
>  '
>
> -test_expect_success 'log with various grep.patternType configurations & command-lines' '
> +test_expect_success !FAIL_PREREQS 'log with various grep.patternType configurations & command-lines' '
Is this an indication of a bug in this test case?
Show 13 quoted lines
>  	git init pattern-type &&
>  	(
>  		cd pattern-type &&
> diff --git a/t/t7405-submodule-merge.sh b/t/t7405-submodule-merge.sh
> index 7855bd8648..aa33978ed2 100755
> --- a/t/t7405-submodule-merge.sh
> +++ b/t/t7405-submodule-merge.sh
> @@ -417,7 +417,7 @@ test_expect_failure 'directory/submodule conflict; keep submodule clean' '
>  	)
>  '
>
> -test_expect_failure 'directory/submodule conflict; should not treat submodule files as untracked or in the way' '
> +test_expect_failure !FAIL_PREREQS 'directory/submodule conflict; should not treat submodule files as untracked or in the way' '
Same here?
Show 13 quoted lines
>  	test_when_finished "git -C directory-submodule/path reset --hard" &&
>  	test_when_finished "git -C directory-submodule reset --hard" &&
>  	(
> diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
> index 2e1bb61b41..7d7b396c23 100755
> --- a/t/t7810-grep.sh
> +++ b/t/t7810-grep.sh
> @@ -412,7 +412,7 @@ do
>  		test_cmp expected actual
>  	'
>
> -	test_expect_success !PCRE "grep $L with grep.patterntype=perl errors without PCRE" '
> +	test_expect_success !FAIL_PREREQS,!PCRE "grep $L with grep.patterntype=perl errors without PCRE" '
And here?
Show 9 quoted lines
>  		test_must_fail git -c grep.patterntype=perl grep "foo.*bar"
>  	'
>
> @@ -1234,7 +1234,7 @@ test_expect_success PCRE 'grep --perl-regexp pattern' '
>  	test_cmp expected actual
>  '
>
> -test_expect_success !PCRE 'grep --perl-regexp pattern errors without PCRE' '
> +test_expect_success !FAIL_PREREQS,!PCRE 'grep --perl-regexp pattern errors without PCRE' '
And here?
Show 9 quoted lines
>  	test_must_fail git grep --perl-regexp "foo.*bar"
>  '
>
> @@ -1249,7 +1249,7 @@ test_expect_success LIBPCRE2 "grep -P with (*NO_JIT) doesn't error out" '
>
>  '
>
> -test_expect_success !PCRE 'grep -P pattern errors without PCRE' '
> +test_expect_success !FAIL_PREREQS,!PCRE 'grep -P pattern errors without PCRE' '
And here?
Show 31 quoted lines
>  	test_must_fail git grep -P "foo.*bar"
>  '
>
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 8270de74be..0367cec5fd 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -309,6 +309,26 @@ test_unset_prereq () {
>  }
>
>  test_set_prereq () {
> +	if test -n "$GIT_TEST_FAIL_PREREQS"
> +	then
> +		case "$1" in
> +		# The "!" case is handled below with
> +		# test_unset_prereq()
> +		!*)
> +			;;
> +		# (Temporary?) whitelist of things we can't easily
> +		# pretend not to support
> +		SYMLINKS)
> +			;;
> +		# Inspecting whether GIT_TEST_FAIL_PREREQS is on
> +		# should be unaffected.
> +		FAIL_PREREQS)
> +			;;
> +		*)
> +			return
> +		esac
> +	fi
> +

I would probably have done that on the reading side rather than the writing side ;-)

Thanks for starting this! Dscho

Show 19 quoted lines
>  	case "$1" in
>  	!*)
>  		test_unset_prereq "${1#!}"
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 908ddb9c46..6fabafebb3 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -1607,3 +1607,7 @@ test_lazy_prereq SHA1 '
>  test_lazy_prereq REBASE_P '
>  	test -z "$GIT_TEST_SKIP_REBASE_P"
>  '
> +
> +test_lazy_prereq FAIL_PREREQS '
> +	test -n "$GIT_TEST_FAIL_PREREQS"
> +'
> --
> 2.21.0.1020.gf2820cf01a
>
>
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 19 of 27 in “What's cooking in git.git (May 2019, #01; Thu, 9)”
  1. Junio C HamanoMay 8, 2019
  2. Denton LiuMay 8, 2019
  3. Elijah NewrenMay 8, 2019
  4. Phillip WoodMay 9, 2019
  5. Johannes SchindelinMay 10, 2019
  6. Phillip WoodMay 13, 2019
  7. Duy NguyenMay 9, 2019
  8. en/fast-export-encoding, was Re: What's cooking in git.git (May 2019, #01; Thu, 9)Johannes Schindelin, May 9, 2019
  9. Elijah NewrenMay 10, 2019
  10. Johannes SixtMay 10, 2019
  11. Johannes SchindelinMay 10, 2019
  12. nd/merge-quit, was Re: What's cooking in git.git (May 2019, #01; Thu, 9)Johannes Schindelin, May 9, 2019
  13. Duy NguyenMay 10, 2019
  14. Johannes SchindelinMay 13, 2019
  15. Johannes SchindelinMay 13, 2019
  16. Eric SunshineMay 13, 2019
  17. Johannes SchindelinMay 13, 2019
  18. tests: add a special setup where prerequisites failÆvar Arnfjörð Bjarmason, May 13, 2019
  19. Johannes SchindelinMay 14, 2019
  20. Ævar Arnfjörð BjarmasonMay 14, 2019
  21. Johannes SchindelinMay 14, 2019
  22. Ævar Arnfjörð BjarmasonMay 14, 2019
  23. Johannes SchindelinMay 14, 2019
  24. tests: mark two failing tests under FAIL_PREREQSÆvar Arnfjörð Bjarmason, Jun 20, 2019
  25. Johannes SchindelinJun 21, 2019
  26. Ævar Arnfjörð BjarmasonJun 21, 2019
  27. Junio C HamanoJun 21, 2019

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.