Re: [v2 PATCH 2/2] reset: add tests for git reset -

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Mar 10, 2015, 17:35 UTC
Message-ID: <CAPig+cQekpyaCd45O0NTijUqxvdTyNiZo1bXeuRKsmmYudwHMw@mail.gmail.com>
In-Reply-To: <1426001883-6423-2-git-send-email-dyoucme@gmail.com>

On Tue, Mar 10, 2015 at 11:38 AM, Sundararajan R <dyoucme@gmail.com> wrote:

> reset: add tests for git reset -

Since this patch is changing the tests rather than 'reset' itself, you'd likely want to say:

    t7102: add 'reset -' tests

> The failure case which occurs on teaching git is taught the '-' shorthand
> is when there exists no branch pointed to by '@{-1}'.

ECANNOTPARSE

Show 10 quoted lines

> The ambiguous cases occur when there exist files named '-' or '@{-1}' in
> the work tree. These are also treated as failure cases but here the user
> is given advice as to how he can proceed.
>
> Add tests to check the handling of these cases.
> Also add a test to verify that reset - behaves like reset @{-1} when none
> of the above cases are true.
>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Helped-by: Torsten BÃ¶gershausen <tboegi@web.de>

Torsten already pointed out this botch.

Show 19 quoted lines

> Helped-by: Eric Sunshine <sunshine@sunshineco.com>
> Helped-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
> Signed-off-by: Sundararajan R <dyoucme@gmail.com>
> ---
> diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
> index 98bcfe2..c05dab0 100755
> --- a/t/t7102-reset.sh
> +++ b/t/t7102-reset.sh
> @@ -568,4 +568,94 @@ test_expect_success 'reset --mixed sets up work tree' '
>         test_cmp expect actual
>  '
>
> +test_expect_success 'reset - with no @{-1} should fail' '
> +       git init new &&
> +       (
> +               cd new &&
> +               test_must_fail git reset - 2>actual
> +       ) &&
> +       test_i18ngrep "unknown revision" new/actual

Broken &&-chain here and throughout the patch.

> +       test_when_finished rm -rf new

If one of the statements in the test before this point fails, then test_when_finished() will never be invoked, which means that the "rm -rf new" cleanup action will never be run. Here, and throughout the patch, you need to invoke test_when_finished() at the earliest point possible so that the cleanup is effective even if some other part of the test fails. In this case, register the cleanup either just before or just after git-init.

Show 19 quoted lines

> +'
> +
> +test_expect_success 'reset - with @{-1} and no file named - or @{-1} should succeed' '
> +       git init new &&
> +       (
> +               cd new &&
> +               echo "Hey" >new_file &&
> +               git add new_file &&
> +               git commit -m "first_commit" &&
> +               git checkout -b new_branch &&
> +               >new_file &&
> +               git add new_file &&
> +               git reset - &&
> +               git status -uno >file1 &&
> +               git add new_file &&
> +               git reset @{-1} &&
> +               git status -uno >file2
> +       ) &&
> +       test_cmp new/file1 new/file2

Broken &&-chain.

Show 6 quoted lines

> +       test_when_finished rm -rf new
> +'
> +
>  test_done
> --
> 2.1.0