Re: [PATCH v3 0/3] Git commit --patch (again)

From: Jeff King <peff@peff.net>
Date: May 10, 2011, 20:01 UTC
Message-ID: <20110510200146.GD14456@sigill.intra.peff.net>
In-Reply-To: <7vsjsnmnrv.fsf@alter.siamese.dyndns.org>

On Mon, May 09, 2011 at 04:56:20PM -0700, Junio C Hamano wrote:

Show 32 quoted lines

> > Yeah, that test that hardcodes the exact commit sequence is disgusting.
> > In the meantime...
> >
> > -- >8 --
> > From: Jeff King <peff@peff.net>
> > Subject: [PATCH] t7501.8: feed a meaningful command
> 
> And then on top of that, Conrad's "allow commit --interactive <path>"
> would come, with this squashed in.  The last "reset HEAD^" is nasty but I
> don't have enough energy to fix 12ace0b (Add test case for basic commit
> functionality., 2007-07-31) today.
> 
>  t/t7501-commit.sh |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
> index 3d2b14d..c2fd116 100755
> --- a/t/t7501-commit.sh
> +++ b/t/t7501-commit.sh
> @@ -41,11 +41,12 @@ test_expect_success \
>  	"echo King of the bongo >file &&
>  	test_must_fail git commit -m foo -a file"
>  
> -test_expect_success PERL 'cannot use paths with --interactive' '
> +test_expect_success PERL 'can use paths with --interactive' '
>  	echo bong-o-bong >file &&
>  	# 2: update, 1:st path, that is all, 7: quit
>  	( echo 2; echo 1; echo; echo 7 ) |
> -	test_must_fail git commit -m foo --interactive file
> +	git commit -m foo --interactive file &&
> +	git reset --hard HEAD^
>  '

Yeah, that reset is a hack. Looking through the tests that come after, I don't think the extra commit is silently hurting any of them, so it's really just the stupid hard-coded rev-list one.

I looked at rewriting it into something like "git log --format=%s", but I couldn't come up with a commit message that reasonably argued "this tests the same thing as the original". Because I haven't really figured out what the original is supposed to be testing. That we made commits? That we made a particular sequence of commits with those messages? With particular trees?

I feel like each test should be responsible for figuring out whether it committed or not (and AFAICT, they do). So I would be tempted to just delete the rev-list test.

-Peff