Re: [PATCH] tests: mark two failing tests under FAIL_PREREQS

[Junio C Hamano <gitster@pobox.com>]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> ... The effect of the FAIL_PREREQS mode is to set all
> prereqs to false, and therefore "test_have_prereq AUTOIDENT" is false,
> but "test_have_prereq !AUTOIDENT" is true.
>
> So this test that would otherwise get skipped gets run.
>
> I honestly didn't think much about these cases when I wrote dfe1a17df9
> ("tests: add a special setup where prerequisites fail", 2019-05-13), and
> now I'm not quite sure whether it should be considered a bug or a
> feature, but in the meantime this un-breaks the test suite under this
> mode.

Yeah, reading the above alone, anybody's knee-jerk reaction would be
that fail-prereqs is buggy, but then I am not sure how we can "fix"
that, short of forbidding a prereq like this !AUTOIDENT one from the
test suite.  The test "predicts" how 'git' used in the test body
would behave, and skips it if we know that the predicted behaviour
breaks the test.  Forcing the prereq using FAIL_PREREQS without
actually making a matching change to the behaviour of 'git' has no
chance of producing sane results.

>>> diff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh
>>> index 5868a87352..1f600e2cae 100755
>>> --- a/t/t0007-git-var.sh
>>> +++ b/t/t0007-git-var.sh
>>> @@ -17,7 +17,7 @@ test_expect_success 'get GIT_COMMITTER_IDENT' '
>>>  	test_cmp expect actual
>>>  '
>>>
>>> -test_expect_success !AUTOIDENT 'requested identites are strict' '
>>> +test_expect_success !FAIL_PREREQS,!AUTOIDENT 'requested identites are strict' '
>>>  	(
>>>  		sane_unset GIT_COMMITTER_NAME &&
>>>  		sane_unset GIT_COMMITTER_EMAIL &&

For this particular one, given the test used to set/unset AUTOIDENT
lazily, I am not sure if it even makes sense to keep this test.  If
we break the underlying ident machinery that would be caught by this
particular test, it seems to me that it is very likely that the test
used to lazily set up AUTOIDENT prereq would fail the same way, so
I am not sure protecting it with !AUTOIDENT would still keep this
one useful.

>>> diff --git a/t/t7502-commit-porcelain.sh b/t/t7502-commit-porcelain.sh
>>> index 5733d9cd34..14c92e4c25 100755
>>> --- a/t/t7502-commit-porcelain.sh
>>> +++ b/t/t7502-commit-porcelain.sh
>>> @@ -402,7 +402,7 @@ echo editor started >"$(pwd)/.git/result"
>>>  exit 0
>>>  EOF
>>>
>>> -test_expect_success !AUTOIDENT 'do not fire editor when committer is bogus' '
>>> +test_expect_success !FAIL_PREREQS,!AUTOIDENT 'do not fire editor when committer is bogus' '
>>>  	>.git/result &&
>>>
>>>  	echo >>negative &&
>>> --
>>> 2.22.0.455.g172b71a6c5
>>>
>>>