Re: [PATCH v1 1/2] checkout: add test to demonstrate regression with checkout -b on initial commit

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Jan 18, 2019, 19:23 UTC
Message-ID: <20190118192331.GN840@szeder.dev>
In-Reply-To: <20190118185558.17688-2-peartben@gmail.com>

On Fri, Jan 18, 2019 at 01:55:57PM -0500, Ben Peart wrote:

> From: Ben Peart <benpeart@microsoft.com>
> 
> Commit fa655d8411 checkout: optimize "git checkout -b <new_branch>" introduced

Style nit: fa655d8411 (checkout: optimize "git checkout -b <new_branch>", 2018-08-16)

Furthermore, please wrap the commit message at a width of around 70 or so chars.

Show 18 quoted lines

> an unintentional change in behavior for 'checkout -b' after doing a
> 'clone --no-checkout'.  Add a test to demonstrate the changed behavior to be
> used in a later patch to verify the fix.
> 
> Signed-off-by: Ben Peart <benpeart@microsoft.com>
> ---
>  t/t2018-checkout-branch.sh | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
> index 2131fb2a56..35999b3adb 100755
> --- a/t/t2018-checkout-branch.sh
> +++ b/t/t2018-checkout-branch.sh
> @@ -198,4 +198,15 @@ test_expect_success 'checkout -B to the current branch works' '
>  	test_dirty_mergeable
>  '
>  
> +test_expect_success 'checkout -b after clone --no-checkout does a checkout of HEAD' '

As this test is supposed to demonstrate a regression, this should be test_expect_failure (to be flipped to test_expect_success in the next patch fixing the regression).

> +	git init src &&
> +	echo hi > src/a &&

Style nit: no space between redirection and filename, but...

> +	git -C src add . &&
> +	git -C src commit -m "initial commit" &&

The above three lines could be replaced by a single

  test_commit -C src a

command.

> +	rev="$(git -C src rev-parse HEAD)" &&
> +	git clone --no-checkout src dest &&
> +	git -C dest checkout "$rev" -b branch &&
> +	test_must_fail test -f dest/a

And here the test should expect the file to be there even when demonstrating the regression.

Please use the 'test_path_is_file' helper instead of 'test -f', as it gives a useful error on failure.

Show 6 quoted lines

> +'
> +
>  test_done
> -- 
> 2.19.1.gvfs.1.16.g9d1374d
>