All of lore.kernel.org
 help / color / mirror / Atom feed
From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] tests: send "bug in the test script" errors to the script's stderr
Date: Tue, 20 Nov 2018 13:09:57 +0100	[thread overview]
Message-ID: <20181120120957.GY30222@szeder.dev> (raw)
In-Reply-To: <20181120105825.GB26263@sigill.intra.peff.net>

On Tue, Nov 20, 2018 at 05:58:25AM -0500, Jeff King wrote:
> On Tue, Nov 20, 2018 at 05:45:28AM -0500, Jeff King wrote:
> 
> > On Tue, Nov 20, 2018 at 12:34:04AM +0100, SZEDER Gábor wrote:
> > 
> > > > I do notice that many of the existing "FATAL:" errors use descriptor 5,
> > > > which goes to stdout. I'm not sure if those should actually be going to
> > > > stderr (or if there's some TAP significance to those lines).
> > > 
> > > I chose to send these messages to standard error, because they are,
> > > well, errors.  TAP only cares about stdout, but by aborting the test
> > > script in BUG(), error() or die() we are already violating TAP anyway,
> > > so I don't think it matters whether we send "bug in test script" or
> > > "FATAL" errors to stdout or stderr.
> > 
> > Yeah, I agree it probably doesn't matter. I was mostly suggesting to
> > make the existing ">&5" into ">&7" for consistency. But I don't think
> > that needs to block your patch.
> 
> By the way, I did check to see whether this might help the situation
> where running under "prove" we see only "Dubious..." and not the actual
> error() message produced by the test script. But no, it eats both stdout
> and stderr. You can sneak a line through by prepending it with "#", but
> only if "prove -o" is used (and even then, it's hard to associate it
> with a particular test when you're running many in parallel).

Just to be clear: I don't mind if in some combination of test
harnesses and test options a "bug in the test script" message doesn't
reach the terminal as long as I get a clearly visible error from
somewhere.

> So I guess the status quo is not too bad: prove says "dubious", and then
> you re-run the test with "./t1234-foo.sh -v -i" and you get to see the
> error.

And with '--verbose-log' the "bug in the test script" message goes to
the test's log as well, even when it has to go through fd 7 first, so
if you use 'prove' and your GIT_TEST_OPTS includes '--verbose-log'
then you can just look at the log, there's no need to re-run the test.


      reply	other threads:[~2018-11-20 12:10 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-19 13:13 [PATCH] tests: send "bug in the test script" errors to the script's stderr SZEDER Gábor
2018-11-19 19:44 ` Jeff King
2018-11-19 23:34   ` SZEDER Gábor
2018-11-20 10:45     ` Jeff King
2018-11-20 10:58       ` Jeff King
2018-11-20 12:09         ` SZEDER Gábor [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181120120957.GY30222@szeder.dev \
    --to=szeder.dev@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.