Re: [PATCH] Improve tests for detached worktree in git-submodule

[Junio C Hamano <gitster@pobox.com>]

Daniel Graña <dangra@gmail.com> writes:

> Signed-off-by: Daniel Graña <dangra@gmail.com>
> ---
>  t/t7409-submodule-detached-worktree.sh |   31 ++++++++++++++++++++++++-------
>  1 files changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/t/t7409-submodule-detached-worktree.sh b/t/t7409-submodule-detached-worktree.sh
> index db75642..d88f400 100755
> --- a/t/t7409-submodule-detached-worktree.sh
> +++ b/t/t7409-submodule-detached-worktree.sh
> @@ -15,7 +15,11 @@ TEST_NO_CREATE_REPO=1
>  test_expect_success 'submodule on detached working tree' '
>  	git init --bare remote &&
>  	test_create_repo bundle1 &&
> -	(cd bundle1 && test_commit "shoot") &&
> +	(
> +		cd bundle1 &&
> +		test_commit "shoot" &&
> +		git rev-list --max-count=1 HEAD > "$TRASH_DIRECTORY/expect"

Better written as

	git rev-parse --verify HEAD >../expect

methinks.

> +	) &&
>  	mkdir home &&
>  	(
>  		cd home &&
> @@ -23,14 +27,27 @@ test_expect_success 'submodule on detached working tree' '
>  		git clone --bare ../remote .dotfiles &&
>  		git submodule add ../bundle1 .vim/bundle/sogood &&
>  		test_commit "sogood" &&
> +		(
> +			unset GIT_WORK_TREE GIT_DIR &&
> +			cd .vim/bundle/sogood &&
> +			git rev-list --max-count=1 HEAD > actual &&
> +			test_cmp actual "$TRASH_DIRECTORY/expect"

Likewise.

	git rev-parse --verify HEAD >actual &&
        test_cmp ../expect actual

As test_cmp turns into "diff -u", comparing expect against actual
(instead of the other way around) shows the deviation in a more
readable way when your test fails.

> +		) &&
>  		git push origin master
> -	) &&
> +	)
>  	mkdir home2 &&
>  	(
>  		cd home2 &&
> -		export GIT_WORK_TREE="$(pwd)" GIT_DIR="$(pwd)/.dotfiles" &&
>  		git clone --bare ../remote .dotfiles &&
> -		git submodule update --init
> +		export GIT_WORK_TREE="$(pwd)" GIT_DIR="$(pwd)/.dotfiles" &&

So you used to clone with the two environment variables in effect to
create the .otfiles, but now you don't use them while cloning.  That
makes more sense to me, especially the .otfiles is created "bare".

> +		git checkout master &&

So you populate the newly created home2 working tree out of the .otfiles
repository in it.

> +		git submodule update --init &&
> +		(
> +			unset GIT_WORK_TREE GIT_DIR &&
> +			cd .vim/bundle/sogood &&
> +			git rev-list --max-count=1 HEAD > actual &&
> +			test_cmp actual "$TRASH_DIRECTORY/expect"

Likewise.

> +		)
>  	)
>  '
>  
> @@ -42,6 +59,7 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '
>  		git clone --bare ../remote "$GIT_DIR" &&
>  		git config core.bare false &&
>  		git config core.worktree .. &&
> +		git checkout master &&
>  		git submodule add ../bundle1 .vim/bundle/dupe &&
>  		test_commit "dupe" &&
>  		git push origin master
> @@ -52,9 +70,8 @@ test_expect_success 'submodule on detached working pointed by core.worktree' '
>  		git config core.bare false &&
>  		git config core.worktree .. &&
>  		git pull &&
> -		git submodule update &&
> -		git submodule status &&
> -		test -d .vim/bundle/dupe
> +		git submodule update --init &&
> +		test -e .vim/bundle/dupe/shoot.t

Is the "existence" the only thing you care about?  That's not all
that different from the old test that only checked the existence of
the directory dupe, no?

>  	)
>  '