diff mbox series

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

Message ID 20190118185558.17688-2-peartben@gmail.com (mailing list archive)
State New, archived
Headers show
Series Fix regression in checkout -b | expand

Commit Message

Ben Peart Jan. 18, 2019, 6:55 p.m. UTC
From: Ben Peart <benpeart@microsoft.com>

Commit fa655d8411 checkout: optimize "git checkout -b <new_branch>" introduced
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(+)

Comments

SZEDER Gábor Jan. 18, 2019, 7:23 p.m. UTC | #1
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.

> 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.

> +'
> +
>  test_done
> -- 
> 2.19.1.gvfs.1.16.g9d1374d
>
diff mbox series

Patch

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' '
+	git init src &&
+	echo hi > src/a &&
+	git -C src add . &&
+	git -C src commit -m "initial commit" &&
+	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
+'
+
 test_done