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

[Stefano Lattarini]

On Wednesday 19 January 2011, Ralf Wildenhues wrote:
> * Stefano Lattarini wrote on Tue, Jan 18, 2011 at 11:35:13PM CET:
> > On Tuesday 18 January 2011, Ralf Wildenhues wrote:
> > > * Stefano Lattarini wrote on Mon, Jan 17, 2011 at 10:55:55PM CET:
> > > > On Monday 17 January 2011, Ralf Wildenhues wrote:
> > > > > Why not document framework_failure_?
> > > > > 
> > > > Because it doesn't really fit into the setup pattern of automake tests: 
> > > > we
> > > > never really use complex setups (almost always, `cat', `echo' and maybe
> > > > `cp' are all that is used!), so there's little use for the function.
> > > 
> > > I'm not sure I understand:
> > > $ grep ' 99' tests/*
> > > 
> > > shows quite a few potential uses already in defs* and instspc-tests.sh.
> > >
> > But note that most of the uses of `[Ee]xit 99' in `instspc-tests.sh'
> > indicate usage errors or internal errors, not setup failures.
> 
> That's what setup failures are, from the Automake user POV.
>
> > > > OTOH, having a generic `hard_error_()' or `hard_fail_()' function might 
> > > > be
> > > > much more useful ...
> > > 
> > > Can you please be more specific?  What would be the semantic difference
> > > of your hard_error_ from framework_failure_?  I don't see any.
> > >
> > No semantic difference, only a more generic error message, as in e.g.:
> > 
> >  hard_error_() { warn_ "$ME_: hard/internal error: $@"; Exit 99; }
> > 
> > OK, no big deal, but I'd rather be precise if possible.
> 
> There is a message string that you can use to be precise.
> hard/internal looks ugly, I like the original.
> 
> Oh well, this is turning into a bikeshed issue.
>
Yes, and anyway, hard_error_() can simply be added later if deemed
useful.  Let's just drop this from now.

> > > > Note that I intended the above as a suggestion for the *user*, not for 
> > > > the
> > > > developer!  I know quite well that TESTS_ENVIRONMENT is a user-reserved
> > > > variable.
> > > > 
> > > > That said, I agree this might be seen as a usability issue.
> > > 
> > > Yes, and that's why we might want to have AM_TESTS_ENVIRONMENT as well.
> > >
> > In case I haven't made it clear before, I agree with this idea too.
> 
> But unfortunately that won't lead to well-defined code:
> With
>   AM_TESTS_ENVIRONMENT = foo=1
>   TESTS_ENVIRONMENT = foo=2
> 
> the
>   $(AM_TESTS_ENVIRONMENT) $(TESTS_ENVIRONMENT) ./test
> 
> will let Solaris sh pass foo=1 to the test, but bash will pass foo=2.
>
Oh joy, I hadn't thought about this YAPI (Yet Another Portability Issue).

> So I'm afraid that at the very least, AM_TESTS_ENVIRONMENT cannot have
> the same semantics (just set variables, no export, no semi-colon at end)
> as TESTS_ENVIRONMENT.  So if the developer needs to use `foo=1; export
> foo' anyway, there is not so much point in having separated *TESTS_SETUP
> variables any more.  Or so I think.
>
I must agree now.  Oh well, no big deal once TESTS_SETUP and AM_TESTS_SETUP
are in place.

> > > >   warn_ ()
> > > >   {
> > > >     echo "$@" 1>&$stderr_fileno_
> > > >   }
> > > > 
> > > > etc.  WDYT?
> > > 
> > > I'm with Jim on this one, vertical compactness can be useful too.
> > >
> > Honestly, not being a huge fan of the GNU formatting standards, I'm
> > mostly with Jim too here :-)
> > 
> > I just had the impression that you preferred consistency in these
> > issues ...
> 
> Yes, consistency is nice, mostly because it avoids patch noise, and
> there are still code indexers out there that require such formatting.
> Spaces are more important than newlines for readability though.
> 
> Cheers,
> Ralf
> 

Regards,
   Stefano