Side effects in Git's test suite, was Re: [PATCH] revert: optionally refer to commit in the "reference" format

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Junio,

On Mon, 23 May 2022, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> +test_expect_success 'identification of reverted commit (reference)' '
> >> +	git checkout --detach to-ident &&
> >> +	git revert --reference --no-edit HEAD &&
> >> +	git cat-file commit HEAD >actual.raw &&
> >> +	grep "^This reverts " actual.raw >actual &&
> >> +	echo "This reverts commit $(git show -s --pretty=reference HEAD^)." >expect &&
> >> +	test_cmp expect actual
> >> +'
> >
> > If it was up to me, I would combine these three test cases, if only to
> > help the `--run=<single-number>` case (the latter two depend on the
> > side effect of the first one to create a `to-ident` tag).
>
> I wonder if our prereq infrastructure is lightweight and scalable enough
> so that we can easily add a support a pseudo-prerequisite PREVIOUS that
> lets us say
>
> 	test_expect_success PREVIOUS "identification ..." '
> 		...
> 	'
>
> to mean that this test requires the previous test has not been
> skipped.

In theory, this sounds good to me.

In practice, however, side effects are awful and make everything harder,
from developing code to debugging to helping new contributors. I wish we
would do away with them altogether and have something more akin to the
before/after constructs known from e.g. TestNG (think `@BeforeTest` and
`@BeforeClass`).

One option would be to mark `setup` steps completely differently, sort of
imitating the prereq infrastructure instead of using
`test_expect_success`. Kind of prereqs, but required to pass.

This could potentially allow us to randomize the order in which the test
cases are run, to identify and fix (unintended) side effects.

A complication is that we have nothing in the way of `@AfterClass`, i.e.
we do not have a way to, say, run an Apache instance for the lifecycle of
a given test script _and tear it down at the end_.

Another, rather obvious complication is that we have 17 years of commit
history introducing side effects left and right :laughing:

Ciao,
Dscho