Skip to content

Conversation

@harry-hov
Copy link

@harry-hov harry-hov commented Mar 2, 2020

Regression test to ensure that the bug (i.e. extra /.git/. in worktree path when called in '.git' directory) has been fixed correctly.

@dscho
Copy link
Member

dscho commented Mar 2, 2020

Maybe insert a "test" verb between the "t2402:" prefix and the noun "worktree path"?

Also, please reduce the cover letter so that it does not repeat the commit message :-)

The bug which reports an extra `/.git/.` in worktree path when called in
'.git' directory already has been fixed. But unfortunately, the regression
test to ensure this behavior has been forgotten.
Here is that test.

Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Hariom Verma <hariom18599@gmail.com>
@harry-hov harry-hov force-pushed the verify-get_main_worktree-fix branch from a2d1756 to 8921df1 Compare March 2, 2020 18:32
@harry-hov harry-hov changed the title t2402: worktree path when called in .git directory t2402: test worktree path when called in .git directory Mar 2, 2020
@harry-hov
Copy link
Author

@dscho Do I need to submit this ?

@dscho
Copy link
Member

dscho commented Mar 4, 2020

Yes 😀

@harry-hov
Copy link
Author

/submit

@gitgitgadget
Copy link

gitgitgadget bot commented Mar 4, 2020

@gitgitgadget
Copy link

gitgitgadget bot commented Mar 4, 2020

On the Git mailing list, Eric Sunshine wrote (reply to this):

On Wed, Mar 4, 2020 at 2:00 AM Hariom Verma via GitGitGadget
<gitgitgadget@gmail.com> wrote:
> The bug which reports an extra `/.git/.` in worktree path when called in
> '.git' directory already has been fixed. But unfortunately, the regression
> test to ensure this behavior has been forgotten.
> Here is that test.

For readers not involved in the discussion, this test was requested by
[1], but the topic graduated to "next" before it could be re-rolled.
It probably ought to be queued atop
'hv/receive-denycurrent-everywhere'.

[1]: https://lore.kernel.org/git/CAPig+cTh-uu-obh9aeDOV9ptbVwRmkujgucbu9ei1Qa3qSNG_A@mail.gmail.com/

> Signed-off-by: Hariom Verma <hariom18599@gmail.com>
> ---
> diff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh
> @@ -151,4 +151,10 @@ test_expect_success 'linked worktrees are sorted' '
> +test_expect_success 'worktree path when called in .git directory' '
> +       git worktree list> list1 &&
> +       git -C .git worktree list> list2 &&

Nit: Style is for the redirection operator to stick to the file:

    git worktree list >list1 &&
    git -C .git worktree list >list2 &&

> +       test_cmp list1 list2
> +'

The test itself makes sense.

@gitgitgadget
Copy link

gitgitgadget bot commented Mar 4, 2020

On the Git mailing list, Junio C Hamano wrote (reply to this):

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Wed, Mar 4, 2020 at 2:00 AM Hariom Verma via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>> The bug which reports an extra `/.git/.` in worktree path when called in
>> '.git' directory already has been fixed. But unfortunately, the regression
>> test to ensure this behavior has been forgotten.
>> Here is that test.
>
> For readers not involved in the discussion, this test was requested by
> [1], but the topic graduated to "next" before it could be re-rolled.
> It probably ought to be queued atop
> 'hv/receive-denycurrent-everywhere'.
>
> [1]: https://lore.kernel.org/git/CAPig+cTh-uu-obh9aeDOV9ptbVwRmkujgucbu9ei1Qa3qSNG_A@mail.gmail.com/

Your notes to those watching from sidelines are greatly appreciated.

>> Signed-off-by: Hariom Verma <hariom18599@gmail.com>
>> ---
>> diff --git a/t/t2402-worktree-list.sh b/t/t2402-worktree-list.sh
>> @@ -151,4 +151,10 @@ test_expect_success 'linked worktrees are sorted' '
>> +test_expect_success 'worktree path when called in .git directory' '
>> +       git worktree list> list1 &&
>> +       git -C .git worktree list> list2 &&
>
> Nit: Style is for the redirection operator to stick to the file:

;-)  Thanks for being a careful reader.

>     git worktree list >list1 &&
>     git -C .git worktree list >list2 &&
>
>> +       test_cmp list1 list2
>> +'
>
> The test itself makes sense.

@gitgitgadget
Copy link

gitgitgadget bot commented Mar 5, 2020

This branch is now known as hv/receive-denycurrent-everywhere.

@gitgitgadget
Copy link

gitgitgadget bot commented Mar 5, 2020

This patch series was integrated into pu via git@4a2e91d.

@gitgitgadget
Copy link

gitgitgadget bot commented Mar 5, 2020

This patch series was integrated into next via git@7d37222.

@gitgitgadget
Copy link

gitgitgadget bot commented Mar 5, 2020

This patch series was integrated into master via git@4a2e91d.

@gitgitgadget
Copy link

gitgitgadget bot commented Mar 5, 2020

Closed via 4a2e91d.

@harry-hov harry-hov deleted the verify-get_main_worktree-fix branch March 18, 2020 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants