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

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

Download raw body.

Thread
On 23-02-14 07:05PM, Omar Polo wrote:
> 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?

I had a little look to try accomplish this without passing $LINENO to
test_done() everywhere but came up empty :(

But, then I thought, for those older systems that don't have LINENO,
could we just keep our current output, and only print op's suggested
$filename.sh:$LINENO if it is supported?

That way, only meaningful information will be reported where it can,
with no change otherwise.

It probably feels like more of a boon than it is because both op and
I have lately had relevant situations where it is indeed useful, but
I'd also hazard a guess that they won't be the last :)

OTOH, it's a lot of $LINENO!

-- 
Mark Jamsek <fnc.bsdbox.org|got.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68