Re: [PATCH 2/5] Tests defs: new subroutine `skip' for test skipping.

[Stefano Lattarini]

On Saturday 15 January 2011, Ralf Wildenhues wrote:
> Hi Stefano,
> 
> * Stefano Lattarini wrote on Mon, Nov 15, 2010 at 06:23:21PM CET:
> > Tests defs: new subroutine `skip' for test skipping.
> 
> I don't like having this patch alone.  If we switch to functions, then
> we should so wholesale, and at once, and fully.
>
OK, but the main point of this patch was *not* to introduce more functions
in the "testing framework", only to ensure that SKIP'd tests offer a more
informative message about the reasons of the SKIP  (OK, the description
line in the commit message has been really really badly chosen then.  Sorry
about this).

I introduced the skip function only to reduce code duplication, since
using:
  skip "this test shouldn't be run on a win32-like system"
is clearer and less error-prone than having to resort to:
  echo "this test shouldn't be run on a win32-like system" >&2
  Exit 77

> Also, there is
> precedent in {coreutils,gnulib}/tests/init.sh which has more features
> than this implementation and I think we should follow (and/or improve
> upon if that is not sufficient):
> 
>   # Print warnings (e.g., about skipped and failed tests) to this file number.
>   # Override by defining to say, 9, in init.cfg, and putting say,
>   # "export ...ENVVAR_SETTINGS...; exec 9>&2; $(SHELL)" in the definition
>   # of TESTS_ENVIRONMENT in your tests/Makefile.am file.
>   # 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; }
> 
> 
> Generally, when we change something, it is a good idea to avoid NIH.
> (That doesn't mean we have to change to remove existing NIH, but avoid
> new.  The point being that stuff that is new is untested and broken.)
>
Sorry, but I don't fully understand what you're requesting here.

You're stating that we should start using init.sh in Automake?  If yes,
I (mostly) agree (while the init.sh is not yet fully adequate for our
purposes, it shouldn't be diffucult to make it more flexible; and the
advantage given by code reuse would be obvious).  The main problem here
is that I lack a copyright assignment for Gnulib, so I can't help in
making init.sh flexible enough for our needs :-(

On the other hand, if you're stating that we should try to be as
init.sh-compatible as possible (in view of a possible future passage
to init.sh), I agree even more, and I could amend the patch to use
`skip_' instead of `skip'.

Or again, are you stating that our `skip' subroutine should be
implemented through (basically) a copy & paste from init.sh?  This
would be fine with me too (but might require a couple of preliminary
patches, such as reaming `$me' to `$ME_' in the testsuite, etc.).

> That said, I found minor issues in the proposed patch which I'm listing
> so you take measures to avoid them when redoing.  This kind of patch
> should really be written with a script, not manually.
>
Ah, but how can a script devise the correct, non-preexisting
messages for the various possible SKIPs?

-*-*-

BTW, I agree with your nits, and fix the patch accordingly.

Thanks,
  Stefano