Re: [PATCH v5 1/7] add simple tests of consistency across rebase types

From: Martin von Zweigbergk <martinvonz@gmail.com>
Date: Jun 4, 2013, 05:14 UTC
Message-ID: <CANiSa6hGLKATn3uUJKoi4917R4qoiyRHHJWo-p8vFrCZsg5joQ@mail.gmail.com>
In-Reply-To: <7v1u8ide2b.fsf@alter.siamese.dyndns.org>

On Mon, Jun 3, 2013 at 3:28 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 9 quoted lines

>> +
>> +# checks that the revisions in "$2" represent a linear range with the
>> +# subjects in "$1"
>> +test_linear_range () {
>> +     ! { git log --format=%p "$2" | sane_grep " " ;} &&
>
> An interesting way to spell:
>
>     test $(git rev-list --merges "$2" | wc -l) = 0

Heh, true. I'll change that. ("My" version was based on the one in git-rebase.sh, around line 495.)

>> +reset_rebase () {
>> +     git rebase --abort # may fail; ignore exit code
>
> test_might_fail to catch unusual exit codes?

Will change.

>> +# a---b---c
>> +#      \
>> +#       d---e

Show 7 quoted lines

>> +test_run_rebase () {
>> +     result=$1
>> +     shift
>> +     test_expect_$result "rebase $* fast-forwards if an ancestor of upstream" "
>
> The description is a non-sentence, and while I can tell what it
> wants to say, I do not have a good suggestion for rephrasing this.

Changing description to "... fast-forwards from an ancestor of upstream".

Show 7 quoted lines

> This is asking to rebase the history leading to b on top of e, but e
> already includes everything in b, so it just turns into a no-op of
> not moving from e.  So it is not even a fast-forward.
>
>> +             reset_rebase &&
>> +             git rebase $* e b &&
>> +             test_cmp_rev e HEAD

Well, "git rebase e b" is of course a kind of short form of "git checkout b && git rebase e". While it's true that the implementation doesn't bother checking out b first, that's just an optimization, but let me know if you meant something else.

Thanks. Will wait another day or two for further comments before I send another version.