"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
Re: [rfc] regress: add line number to failure output
To:
Mark Jamsek <mark@jamsek.com>
Cc:
Christian Weisgerber <naddy@mips.inka.de>, gameoftrees@openbsd.org
Date:
Tue, 14 Feb 2023 19:05:03 +0100

Download raw body.

Thread
On 2023/02/14 23:47:08 +1100, Mark Jamsek <mark@jamsek.com> wrote:
> On 23-02-14 01:24PM, Stefan Sperling wrote:
> > On Tue, Feb 14, 2023 at 11:11:26PM +1100, Mark Jamsek wrote:
> > > On 23-02-14 12:47PM, Christian Weisgerber wrote:
> > > > Mark Jamsek:
> > > > 
> > > > > I made adjusting regress for the new fetch behaviour change more
> > > > > difficult than necessary because of a copypasta error where the reported
> > > > > failure returned a bogus test function. Silly error on my part but to
> > > > > figure out the cause of the failure I added $LINENO to the output such

I just had a similar experience trying to understand the failures in
the regress after some changes in my sha256 branch.  The regress suite
is really nice, but when you change something and random tests starts
to fail, especially those that are a bit long, it's not immediate to
track the issue.

I'd like to suggest a slightly different format for the error though:

	filename.sh:$LINENO: test failed; leaving data in $testroot

would be awesome.

(on a similar note, it would be nice to being able to run only
specific tests named on the command line without editing the file)

TIL of $LINENO btw!

> > > > Portability concern: Although LINENO is in POSIX, it's only covered
> > > > as part of an extension.  dash, the somewhat popular minimalist sh
> > > > on Linux, does not support it ... or at least older versions of
> > > > dash still used by stable versions of Debian don't support LINENO.

FWIW dash 0.5.12 as in the alpine repo has $LINENO working

> > > Thanks, naddy! I wasn't too sure of its portability impact so was hoping
> > > you or someone as knowledgeable would have some insight.
> > > 
> > > It's really not an important change; it just helped with a problem of my
> > > own making and I thought to share it in case it might have some broader
> > > utility that appeals to others. But our tests already do provide enough
> > > info to isolate any problems so I think it might be best to forget this,
> > > at least for now.
> > > 
> > > Thanks again :)
> > 
> > I agree that this would be nice to have, but portability is indeed
> > a concern for -portable. It would also be nice to have a solution
> > that only tweaks test_done in one place rather than having to pass
> > additional parameters. But I don't have no idea how that could be done.
> 
> I'm not sure either; we can't use $LINENO in test_done() because it'll
> just show the line number inside common.sh:test_done(). This could be
> done with some macros in C but I've no idea in shell.

what is actually the issue for -portable?  we're not running with set
-u, so if the shell used doesn't know that $LINENO is special it will
just replace it with the empty string.  Worst case scenario, $LINENO
could be defined in the environment.  Am i missing something?

Anyway, I like the outcome and it would have helped me a little in the
past days, but maybe it's too much boilerplate?

> > Maybe it is time to rewrite the tests in Rust?
> 
> :D rust all the things! /s

time to roll some m4 over the tests?  :P