On 12/10/2019 02:36, Derrick Stolee wrote:
> On 10/11/2019 12:50 PM, James Coglan wrote:
>> On 10/10/2019 18:19, Derrick Stolee wrote:
>>> On 10/10/2019 12:13 PM, James Coglan via GitGitGadget wrote:
>>>> +++ b/t/t4215-log-skewed-merges.sh
>>>> @@ -0,0 +1,42 @@
>>>> +#!/bin/sh
>>>> +
>>>> +test_description='git log --graph of skewed merges'
>>>> +
>>>> +. ./test-lib.sh
>>>> +
>>>> +test_expect_success 'setup left-skewed merge' '
>>>
>>>
>>> Could you skew this example to include a left-skewed octopus merge
>>> (and use fewer Git processes) with the following:
>>>
>>>     git checkout --orphan _a && test_commit A &&
>>>     git switch -c _b _a && test_commit B &&
>>>     git switch -c _c _a && test_commit C &&
>>>     git switch -c _d _a && test_commit D && git switch -c _e _b && git 
>>> merge --no-ff _c _d E &&
>>>     git switch -c _f _a && git merge --no-ff _d -m F &&     git checkout _a 
>>> && git merge --no-ff _b _c _e _f -m G
>>> and I think the resulting output will be:
>>>
>>> *-----.   G
>>> |\ \ \ \
>>> | | | | * F
>>> | |_|_|/|
>>> |/| | | |
>>> | | | * | E
>>> | |_|/|\|
>>> |/| | | |
>>> | | |/  * D
>>> | |_|__/
>>> |/| |
>>> | | * C
>>> | |/
>>> |/|
>>> | * B
>>> |/
>>> * A
>>
>> At this point in the history, commit E won't render like that -- this is 
>> before the change that flattens edges that fuse with the merge's last 
>> parent. I think the display of this history at this point will be:
>>
>>      *-----.   G
>>      |\ \ \ \
>>      | | | | * F
>>      | |_|_|/|
>>      |/| | | |
>>      | | | * |   E
>>      | |_|/|\ \
>>      |/| |/ / /
>>      | | | | /
>>      | | | |/
>>      | | | * D
>>      | |_|/
>>      |/| |
>>      | | * C
>>      | |/
>>      |/|
>>      | * B
>>      |/
>>      * A
>>
>> Is there a particular reason for wanting to include this test case? What 
>> particular combination of states is it designed to test? (My guess is that 
>> it includes an octopus merge where the original test does not.) I'd be happy 
>> to add it at the appropriate point in the history if it's adding coverage 
>> not provided by the other tests.
> 
> Thanks for correcting my test case. It also helps that you would show the 
> change in behavior in your later commits.
> 
> My reason to include this test is because it includes a regular merge and an 
> octopus merge, both of which have a skewed render. Many times logic that 
> applies to a normal merge breaks with octopus merges, so I try to include 
> them whenever possible.

Thanks, I've now incorporated your suggested test into my patch. I had to amend 
it slightly as it turns out the above history is not valid; G is not a possible 
merge because one of its parents (C) is an ancestor of another (E). The actual 
example I've added is this:

        *-----.   0_H
        |\ \ \ \
        | | | | * 0_G
        | |_|_|/|
        |/| | | |
        | | | * \   0_F
        | |_|/|\ \
        |/| | | |/
        | | | | * 0_E
        | |_|_|/
        |/| | |
        | | * | 0_D
        | | |/
        | | * 0_C
        | |/
        |/|
        | * 0_B
        |/
        * 0_A

I've also added another commit before beginning this work that adds the example 
from the cover letter, so you can see it changing with each commit, namely this 
history:

        *   H
        |\
        | *   G
        | |\
        | | * F
        | | |
        | |  \
        | *-. \   E
        | |\ \ \
        |/ / / /
        | | | /
        | | |/
        | | * D
        | * | C
        | |/
        * | B
        |/
        * A

Reply via email to