All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] t4018: drop "debugging" cat from hunk-header tests
Date: Thu, 16 Jan 2020 22:56:55 +0000	[thread overview]
Message-ID: <20200116225655.GD71299@google.com> (raw)
In-Reply-To: <20200116183423.GA3011203@coredump.intra.peff.net>

Jeff King wrote:

> We run a series of hunk-header tests in a loop, and each one does this:
>
>   test_when_finished 'cat actual' &&      # for debugging only
>
> This is pretty pointless. When the test succeeds, we waste time running
> a useless cat process. If you're debugging a failure with "-i", then we
> won't run the when-finished part at all. So it helps only if you're
> running with something like "--verbose-log".
>
> Since we expect the tests to succeed most of the time, a better way to
> do this would be a helper that checks the output and dumps "actual" only
> when it fails. But it's probably not even worth the effort, as anyone
> debugging a failure could just run with "-i" and investigate the
> "actual" file themselves.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> Just noticed this when working with t4018 on an unrelated problem.
>
> I could be convinced otherwise on the final paragraph, but I think it
> would only be worth it if we added a general test_grep() helper and used
> it in more places.
>
>  t/t4018-diff-funcname.sh | 1 -
>  1 file changed, 1 deletion(-)

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Thanks.

Sure, a test_grep would be nice for CI cases (especially for
Heisenbugs), but in its absence I agree that this patch is the right
thing to do.

  reply	other threads:[~2020-01-16 22:56 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-16 17:51 [PATCH] Makefile: use compat regex with SANITIZE=address Jeff King
2020-01-16 18:34 ` [PATCH] t4018: drop "debugging" cat from hunk-header tests Jeff King
2020-01-16 22:56   ` Jonathan Nieder [this message]
2020-01-17 17:37 ` [PATCH] Makefile: use compat regex with SANITIZE=address Junio C Hamano
2020-01-17 17:49   ` Jeff King
2020-01-17 23:39     ` brian m. carlson
2020-01-18  3:34       ` Jeff King

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=20200116225655.GD71299@google.com \
    --to=jrnieder@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.