Re: [PATCH v3] stash: avoid recursive hard reset on submodules

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

Jakob Jarmar <jakob.j@gmail.com> writes:

> diff --git a/t/t3906-stash-submodule.sh b/t/t3906-stash-submodule.sh
> index d7219d6f8f..83106fa958 100755
> --- a/t/t3906-stash-submodule.sh
> +++ b/t/t3906-stash-submodule.sh
> @@ -1,6 +1,6 @@
>  #!/bin/sh
>  
> -test_description='stash apply can handle submodules'
> +test_description='stash can handle submodules'

Good attention to the detail ;-)

> +setup_basic() {

Style.  SP on both sides of () in our shell scripts (as seen in the
existing shell function in the same file).

> +	git init sub &&
> +	(
> +		cd sub &&
> +		test_commit sub_file
> +	) &&
> +	git init main &&
> +	(
> +		cd main &&
> +		git submodule add ../sub &&
> +		test_commit main_file
> +	) &&
> +	test_when_finished "rm -rf main sub"

Have test_when_finished that removes main and sub _before_ you start
creating sub and main.  

When the &&-cascade breaks anywhere, the control may not even reach
your test_when_finished that registers the clean-up procedure.
Imagine "git init sub" succeeds but "git init main" somehow
fails---you still want to clean up "sub".

Other than that, looks reasonably well done.  

Thanks for working on this.


> +}
> +
> +test_expect_success 'stash push with submodule.recurse=true preserves dirty submodule worktree' '
> +	setup_basic &&
> +	(
> +		cd main &&
> +		git config submodule.recurse true &&
> +		echo "x" >main_file.t &&
> +		echo "y" >sub/sub_file.t &&
> +		git stash push &&
> +		test_must_fail git -C sub diff --quiet
> +	)
> +'
> +
> +test_expect_success 'stash push and pop with submodule.recurse=true preserves dirty submodule worktree' '
> +	setup_basic &&
> +	(
> +		cd main &&
> +		git config submodule.recurse true &&
> +		echo "x" >main_file.t &&
> +		echo "y" >sub/sub_file.t &&
> +		git stash push &&
> +		git stash pop &&
> +		test_must_fail git -C sub diff --quiet
> +	)
> +'
> +
>  test_done