From: Lars Hjemli <hjemli@gmail.com>
Date: Sun, 23 Sep 2007 10:24:03 GMT
Subject: Re: [PATCH 1/5] Add test-script for git-merge porcelain
Message-ID: <8c5c35580709230324m65c5399ds8c7ce21df4461e66@mail.gmail.com>
In-Reply-To: <7vwsuidx1a.fsf@gitster.siamese.dyndns.org>

On 9/23/07, Junio C Hamano <gitster@pobox.com> wrote:
> Lars Hjemli <hjemli@gmail.com> writes:
>
> > This test-script tries to excercise the porcelainish aspects of git-merge.
>
> It does exercise; no need for "tries to" ;-).

Heh, ok

> > +     cmp -s file result.1
> > +'
>
> Unless there is a compelling reason otherwise, I'd prefer tests
> with a test vector like this to be spelled like this:
>
>         diff -u result.1 file
>
> That is, compare to show the difference actual output might have
> from the expected result.  It's easier to spot the difference
> when you later break things this way.

Good point, I'll change it.

>
> > +test_expect_success 'merge c1 with c2' '
> > +     git reset --hard c1 &&
> > +     git merge c2 &&
> > +     test "$c1" = "$(git rev-parse HEAD^1)" &&
> > +     test "$c2" = "$(git rev-parse HEAD^2)"
> > +'
>
> We might also want to test:
>
>  - the index is merged;
>
>  - the working tree matches the index;
>
>  - the merge message (e.g. "git show -s --pretty=format:%s
>    HEAD") is as expected;

Yes, sorry for being lazy ;-)  I'll add more thorough verification

>
> Otherwise I think it is a good idea to add these tests.
>
> By the way, I think squash_message() leaves a wrong message
> template for an Octopus, which might be worth fixing.

Ok, I'll try to detect it in the test-script and send a proper fix.

Thanks for the review.

-- 
larsh