Re: [PATCH] {maint} tests: new subroutines for test skipping/failing

[Jim Meyering]

Ralf Wildenhues wrote:
...
> Why not document framework_failure_?
>
>>    For tests that use the `parallel-tests' Automake option, set the shell
>>    variable `parallel_tests' to "yes" before including ./defs.  Also,
>>    use for them a name that ends in `-p.test' and does not clash with any
>
>> --- a/tests/defs.in
>> +++ b/tests/defs.in
>
>> @@ -88,6 +88,31 @@ echo "$PATH"
>>  # (See note about `export' in the Autoconf manual.)
>>  export PATH
>>
>> +# Print warnings (e.g., about skipped and failed tests) to this file number.
>> +# Override by putting, say:
>> +#   stderr_fileno_=9; export stderr_fileno_; exec 9>&2; $(SHELL)
>> +# in the definition of TESTS_ENVIRONMENT.
>
> That may be good advice in the context of gnulib.  However, it describes
> what is essentially a bug, or at least usability issue, in Automake:
> that the test author cannot write to the original stderr with the
> parallel-tests driver any more, and has to use a workaround which breaks
> user overrides of TESTS_ENVIRONMENT.

Too true.  In most projects I maintain, TESTS_ENVIRONMENT is so
encumbered that it is effectively impossible for the user to override it.

> I think this should be addressed in the driver(s), ideally in a way that
> is both backward-compatible and allows the testsuite author to write
> identical code for the simple driver as for the parallel-tests driver.
> For example, am__check_pre could contain
>   $(TESTS_SETUP)
>
> or even
>   $(AM_TESTS_SETUP) $(TESTS_SETUP)
>
> before $(TESTS_ENVIRONMENT).  Then the developer could do
>   TESTS_SETUP = stderr_fileno_=9; export stderr_fileno_; exec 9>&2;
>
> What do you think?  I further wonder whether we should finally introduce
> $(AM_TESTS_ENVIRONMENT) and reserve the non-AM_ variable for environment
> settings for the user.

I like the idea of reserving one variable for package maintainers
and leaving the other for people who run "make".
Stefano's suggestion to make both variables AM_-prefixed
seems like a good one, so as not to impinge any more on
user variable name space.

> What do you think?
>
>> +# This is useful when using automake's parallel tests mode, to print
>> +# the reason for skip/failure to console, rather than to the .log files.
>> +: ${stderr_fileno_=2}
>> +
>> +warn_() { echo "$@" 1>&$stderr_fileno_; }
>> +fail_() { warn_ "$me: failed test: $@"; Exit 1; }
>> +skip_() { warn_ "$me: skipped test: $@"; Exit 77; }
>> +framework_failure_() { warn_ "$me: set-up failure: $@"; Exit 99; }
>
> space before ()

I don't mind adding spaces before () in gnulib's copy, if that makes
it easier for you.  However, I normally use a space there for readability
(in shell scripts, at least -- no risk of automatic formatters ;-),
but with those trailing underscores serving much the same purpose,
the existing formatting does not bother me at all.  Hmm... though
now that I think of it, with the existing formatting, it is perhaps
too easy to mistake those function names for their underscore-free
versions.  So I'll change it in gnulib.

However, I think that spreading those four function definitions onto 12
or more lines for the sake of formatting would represent a significant
net loss in readability.