Re: [PATCH 2/5] t/t5520: explicitly unset rebase.autostash

[Eric Sunshine <sunshine@sunshineco.com>]

On Tue, Mar 29, 2016 at 9:29 AM, Mehul Jain <mehul.jain2029@gmail.com> wrote:
> t/t5520: explicitly unset rebase.autostash

As with patch 1/5, this subject is written at too low a level, talking
about details of the patch rather than giving a high-level overview.
What the patch is really doing is ensuring consistent conditions
within the test even if some future change pollutes the global
configuration. Maybe:

    t5520: ensure consistent test conditions

or:

    t5520: make test expectations explicit

or something.

> Tests title suggest that tests are done with rebase.autostash unset,
> but doesn not take any action to make sure that it is indeed unset.

This is just paraphrasing my earlier review comment[1], however,
"suggest" is a weak argument for why this change is desirable. State
instead that this change ensures a consistent condition for tests in
which rebase.autostash should not be set and protects against some
future change polluting the global configuration.

> Make sure that rebase.autostash is unset by explicitly setting it.

The patch itself looks ok.

[1]: http://article.gmane.org/gmane.comp.version-control.git/289860

> Signed-off-by: Mehul Jain <mehul.jain2029@gmail.com>
> ---
>  t/t5520-pull.sh | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
> index 5be39df..9ee2218 100755
> --- a/t/t5520-pull.sh
> +++ b/t/t5520-pull.sh
> @@ -279,6 +279,7 @@ test_expect_success 'pull --rebase --autostash & rebase.autostash=false' '
>  '
>
>  test_expect_success 'pull --rebase: --autostash & rebase.autostash unset' '
> +       test_unconfig rebase.autostash &&
>         git reset --hard before-rebase &&
>         echo dirty >new_file &&
>         git add new_file &&
> @@ -307,6 +308,7 @@ test_expect_success 'pull --rebase --no-autostash & rebase.autostash=false' '
>  '
>
>  test_expect_success 'pull --rebase --no-autostash & rebase.autostash unset' '
> +       test_unconfig rebase.autostash &&
>         git reset --hard before-rebase &&
>         echo dirty >new_file &&
>         git add new_file &&
> --
> 2.7.1.340.g69eb491.dirty