Re: [PATCH 04/30] directory rename detection: basic testcases

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 14, 2017, 02:03 UTC
Message-ID: <xmqqmv3p7h1p.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAGZ79kbmt7jt13D-HY90+LBaCHsqvDOYnrmJ41hR3YsgEceirg@mail.gmail.com>

Stefan Beller <sbeller@google.com> writes:

Show 19 quoted lines

> On Fri, Nov 10, 2017 at 11:05 AM, Elijah Newren <newren@gmail.com> wrote:
>> Signed-off-by: Elijah Newren <newren@gmail.com>
>> ...
>> +#      B
>> +#      o
>> +#     / \
>> +#  A o   ?
>> +#     \ /
>> +#      o
>> +#      C
>> + ...
>> +# Testcase 1a, Basic directory rename.
>> +#   Commit A: z/{b,c}
>> +#   Commit B: y/{b,c}
>> +#   Commit C: z/{b,c,d,e/f}
>
> (minor thought:)
> After rereading the docs above this is clear; I wonder if instead of A, B, C
> a notation of Base, ours, theirs would be easier to understand?

I had a similar thought, but as long as everything in this file is consistent, as we have that picture upfront, I am OK with it. FWIW, t1000 uses O (original--common ancestor) A and B, which was the notation commonly used in our codebase since the early days when we needed to call them with single letters.

Show 9 quoted lines

>> +test_expect_success '1a-setup: Simple directory rename detection' '
>> +test_expect_failure '1a-check: Simple directory rename detection' '
>
> Thanks for splitting the setup and the check into two different test cases!
>
>
>> +       git checkout B^0 &&
>
> Any reason for ^0 ? (to make clear it is a branch?)

I think it is to make it clear that no matter what this test does (or fails to do), the branch B is *not* affected by it because we'd be playing on a detached head.

Show 7 quoted lines

>> +test_expect_success '1b-setup: Merge a directory with another' '
>> +       git rm -rf . &&
>> +       git clean -fdqx &&
>> +       rm -rf .git &&
>> +       git init &&
>
> This is quite a strong statement to start a test with.

Yes. If a test before this one did cd ../.. and forgot to come back, we'd be in trouble. If we want a fresh repository perhaps test-create-repo inside the trash repository may be a less evil option.