Re: [PATCH 2/4] test: due not pass T380.1 for the wrong reasons

[Michael J Gruber <git@grubix.eu>]

David Bremner venit, vidit, dixit 2022-02-09 23:12:24:
> Michael J Gruber <git@grubix.eu> writes:
> 
> > If gdb is missing then some files are never written to so that the
> > comparisons of non-existing files succeeds for the wrong reason,
> > claiming that `notmch new` is idempotent when it was in fact never run.
> >
> > Catch this and (for lack of a better spot) set up the files with a
> > reason for the FAIL.
> >
> 
> I'm a bit confused by this. For me if gdb is missing I get
> 
>  missing prerequisites: gdb(1)
>  SKIP   all tests in T380-atomicity
> 
> ...
> 
> All 1842 tests behaved as expected (8 expected failures).
> All tests in 1 file skipped.
> 
> Do I misunderstand something, or is prerequisite detection not working
> for you?

It does now, my understanding has been mislead by a few things. The
issues were:

On one of Fedora's arches which is emulated in the COPR test
environment, gdb was present (just like in all of them) but apparantly
failed to set any break points. T380 spew out something like

cat: outcount: No such file or directory
./T380-atomicity.sh: line 79: ((: i < : syntax error: operand expected
(error token is "< ")

followed by wrong PASS/FAIL etc.

When analysing this, I was confused by the way
test_require_external_prereq works and the "if" in T380 (as opposed to how
test_require_external_prereq is used in other tests). Over at git.git,
we have test setup code in functions which don't get executed if
prerequisites fail. I guess the "if" emulates that, but then the actual
tests in T380 are outside the if block and use files and variables which
are created in the if block. So, this is something to fix anyways.

In order to try things out locally, I changed

if test_require_external_prereq gdb; then

into

if test_require_external_prereq gggdb; then

I know now that `test_require_external_prereq binary` does not test for
the presence of binary, nor does it abort the test script. It merely
checks among the set of "predefined external prerequisites", and this
would be much clearer if "binary" wasn't an argument to that function
but a suffix: `test_require_external_prereq_gdb` immediately says "I'm
something predefined".

Until now, I wondered how `test_require_external_prereq` could skip the
PASS/FAIL outout if it could not skip the previous part of the test
script. This is not clear from its implementation.

I know now that indeed text_expect-success etc. call
test_check_missing_external_prereqs_ for the current subtest and skip
the output.

Add to this the fact that the tests needing sfsexp or asan (and probably
others) do things yet differently and call "test_done" immediately, so
that no SKIP appears. And those were the only ones skipped at all here ...

In effect, I knoew SKIP only from manually skipping tests (NOTMUCH_SKIP_TESTS).

I'm not complaining, on the other hand I don't fail too bad about myself
for being confused by this ;)

So, in the long run I think test_setup() is worth thinking about.

In the short run, initialising variables and files which are used is
still a good thing, but I would have to rewrite some commit messages.

I'll wait until it's clear how to handle style, though: switch to printf
from echo whenever I touch those lines (leading to mixed use) or keeing
style and leaving the style change for another series.

Cheers,
Michael