Re: [PATCH 4/4] worktree add: Add unit tests for --orphan

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Nov 4, 2022, 01:37 UTC
Message-ID: <221104.86k04bzeaa.gmgdl@evledraar.gmail.com>
In-Reply-To: <20221104010242.11555-5-jacobabel@nullpo.dev>

On Fri, Nov 04 2022, Jacob Abel wrote:

We usually add tests along with the feature, so this should be squashed into your 3/4.

Show 39 quoted lines

> Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
> ---
>  t/t2400-worktree-add.sh | 54 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
>
> diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
> index d587e0b20d..064e1336e2 100755
> --- a/t/t2400-worktree-add.sh
> +++ b/t/t2400-worktree-add.sh
> @@ -310,6 +310,26 @@ test_expect_success '"add" -B/--detach mutually exclusive' '
>  	test_must_fail git worktree add -B poodle --detach bamboo main
>  '
>
> +test_expect_success '"add" --orphan/-b mutually exclusive' '
> +	test_must_fail git worktree add --orphan poodle -b poodle bamboo main
> +'
> +
> +test_expect_success '"add" --orphan/-B mutually exclusive' '
> +	test_must_fail git worktree add --orphan poodle -B poodle bamboo main
> +'
> +
> +test_expect_success '"add" --orphan/--detach mutually exclusive' '
> +	test_must_fail git worktree add --orphan poodle --detach bamboo main
> +'
> +
> +test_expect_success '"add" --orphan/--no-checkout mutually exclusive' '
> +	test_must_fail git worktree add --orphan poodle --no-checkout bamboo main
> +'
> +
> +test_expect_success '"add" -B/--detach mutually exclusive' '
> +	test_must_fail git worktree add -B poodle --detach bamboo main
> +'
> +
> [...]
> +test_expect_success '"add --orphan"' '
> +	git worktree add --orphan neworphan orphandir main &&
> +	git -C orphandir symbolic-ref HEAD >actual &&
> +	echo refs/heads/neworphan >expected &&
> +	test_cmp expected actual &&

nit: we usually do:

	echo ...
	git ...
	test_cmp

So there's never doubt that the "expected" is before the command (i.e. not impacted by it).

> +	(
> +		cd orphandir &&
> +		git diff main
> +	)

Here you can avoid the sub-shell, with:

	git -C orphandir diff main

Show 6 quoted lines

> +'
> +
> +test_expect_success '"add --orphan" fails if the branch already exists' '
> +	git worktree add -b existingbranch orphandir2 main &&
> +	test_must_fail git worktree add --orphan existingbranch orphandir3 main &&
> +	[ ! -d orphandir3 ]

Don't use "[", use "test", but in this case use one of the "test -d", "test -f" etc. wrappers in test-lib-functions.sh.

Show 5 quoted lines

> +'
> +
> +test_expect_success '"add --orphan" fails if the commit-ish doesnt exist' '
> +	test_must_fail git worktree add --orphan badcommitish orphandir4 eee2222 &&
> +	[ ! -d orphandir4 ]

ditto.

Show 12 quoted lines

> +'
> +
> +test_expect_success '"add --orphan" with empty repository' '
> +	(
> +		mkdir emptyorphanrepo &&
> +		cd emptyorphanrepo &&
> +		GIT_DIR=".git" git init --bare &&
> +		git worktree add --orphan newbranch worktreedir &&
> +		git -C worktreedir symbolic-ref HEAD >actual &&
> +		echo refs/heads/newbranch >expected &&
> +		test_cmp expected actual
> +	)

Ditto we can avoid sub-shelling here, also when using sub-shells, make the "cd" the first command in it, if you keep it the "mkdir" should go outside of it.

But isn't this (untested, maybe I'm missing a subtlety):

	test_when_finished "rm -rf repo" &&
	GIT_DIR=.git git init --bare repo &&
	git -C worktree ...
	git -C worktree/subdir ..
	echo refs ...
	test_cmp ...

I.e. do we need to create the dir manually before "git init --bare"?