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

[Ralf Wildenhues]

[ Jim, I have a question below ]

* Stefano Lattarini wrote on Sat, Jan 15, 2011 at 03:16:51PM CET:
> On Saturday 15 January 2011, Ralf Wildenhues wrote:
> > * 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 [...]

Granted, but why stop with SKIP and disallow more informative FAILs?
At least we can define the respective functions and use them in new
tests.

> 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

Sure.  But let's reap other benefits as well while we're at it.

> > 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):
[...]

> > 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.

Apologies for being a bit dense.

> 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?

Well, I'm not sure if using gnulib's init.sh outright is a good goal
right away.  We've had so much testsuite churn now, and many tests fail
at the moment, that I'm tempted to call for a couple of weeks of "no
testsuite changes except fixing FAILures".

Back to my original point though: even if we don't use init.sh right
away, it is a good idea to make new code of ours similar to gnulib's,
with the goal of being more mergeable at some point in the future.
And while we do that, we can already fold back bugfixes to gnulib where
appropriate.  As a first step, we can copy code from gnulib.

Jim, gnulib/tests/init.sh is GPLv3+ right now.  We would like to use
code added/modified in commit v0.0-3988-g0ae8d63.  Do you mind if we
copy that into Automake's testsuite setup, currently GPLv2+?  AFAICS
you're the sole author of that and prior changes around these lines.

Oh, Stefano, can you please start the assignment process for gnulib?
If that is not possible, then I'll have to do these changes sometime.

> 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.).

Certainly not; a
  ME_=$me

avoids that really useless churn; alternatively a s/ME_/me/g for the
parts taken from init.sh.  Doing one of these is prudent for any merging
of pending branches anyway.  A global s/me/ME_/g can be done later when
we actually turn to use init.sh (and all branches using $me have long
been dead).

> > 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?

Well, skip_ should cope with not being passed any message: it shouldn't
output anything in that case.  That's a change we could submit to
gnulib.  (If gnulib wants to require a message, that can be done with
maintainer checks.)

All in all, this calls for a three-step conversion (can be done in one
commit, but avoids the sort of bug I found in the prior patch
submission):

- s/Exit 77/skip_/g
- check/fix any occurrences in here-documents manually:
  /<</,/^E[NO][DF]/{ /skip_/p; }
- edit some or all skip_ occurrences to add a short explanation
  (less than (79 - strlen (": skipped test: ") - length of longest
  test name) characters).

Cheers,
Ralf