All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Mike Hommey <mh@glandium.org>,
	git@vger.kernel.org
Subject: Re: [PATCH] t6024-recursive-merge.sh: hide spurious output when not running verbosely
Date: Fri, 29 Feb 2008 23:10:05 -0500	[thread overview]
Message-ID: <20080301041005.GA8969@coredump.intra.peff.net> (raw)
In-Reply-To: <7v1w6vb9w4.fsf@gitster.siamese.dyndns.org>

On Fri, Feb 29, 2008 at 03:50:03PM -0800, Junio C Hamano wrote:

> Actually, I think this might be a bit more sensible approach.
> 
> -- >8 --
> tests: allow optional clean-up phrase to expect_success/failure
> 
> When one test modifies the state of the test repository that the later
> tests may depend on, you may want to add a clean-up action that is run
> regardless of the outcome of the main part of the test.
> 
> This can now be specified as the third parameter to test_expect_success
> and test_expect_failure functions.

I think your heart is in the right place with this patch, but I doubt
that it is going to be all that productive in practice. Most tests
consist of a long list of commands, and cleaning up properly after every
possible failure case is going to be a lot of work. And worse, since the
tests generally _don't_ fail, you have no way to test that your cleanup
is reasonable.

So I think we will end up in the case where a few failed tests will
properly clean themselves up and let further tests proceed, but most
failures will leave a big question. In other words, what problem have we
solved?  If tests N and N+k both fail, would you, even with this patch,
suspect N+k of actually failing, or would you first go and debug test N?
Is that any different than what you do now?

-Peff

  reply	other threads:[~2008-03-01  4:11 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-29 22:23 [PATCH] t6024-recursive-merge.sh: hide spurious output when not running verbosely Mike Hommey
2008-02-29 22:53 ` Jeff King
2008-02-29 22:58   ` Mike Hommey
2008-02-29 23:01     ` Jeff King
2008-02-29 23:15     ` Junio C Hamano
2008-02-29 23:54       ` Jeff King
2008-02-29 23:34 ` Johannes Schindelin
2008-02-29 23:50   ` Junio C Hamano
2008-03-01  4:10     ` Jeff King [this message]
2008-03-01  4:27       ` Shawn O. Pearce
2008-03-01  4:28         ` Jeff King
2008-03-01  5:40           ` Junio C Hamano

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=20080301041005.GA8969@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mh@glandium.org \
    /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.