Re: [RFC PATCH 1/2] test that git status works with merge conflict in .gitmodules

From: Junio C Hamano <gitster@pobox.com>
Date: May 13, 2011, 05:47 UTC
Message-ID: <7v1v03b19e.fsf@alter.siamese.dyndns.org>
In-Reply-To: <4DCC4AE0.6020902@web.de>

Jens Lehmann <Jens.Lehmann@web.de> writes:

Show 9 quoted lines

> From: Heiko Voigt <hvoigt@hvoigt.net>
>
> For example: Two users independently adding a submodule will result in a
> merge conflict in .gitmodules. Since configuration of the status and
> diff machinery depends on the file being parseable they currently
> fail to produce useable output in case .gitmodules is marked with a
> merge conflict.
>
> Signed-off-by: Heiko Voigt <hvoigt@hvoigt.net>

The basic idea (both patches) seems sound.

Show 15 quoted lines

> diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh
> index c8d50a6..3b1806c 100755
> --- a/t/t7506-status-submodule.sh
> +++ b/t/t7506-status-submodule.sh
> @@ -4,17 +4,21 @@ test_description='git status for submodule'
>
>  . ./test-lib.sh
>
> -test_expect_success 'setup' '
> -	test_create_repo sub &&
> +test_create_repo_with_commit () {
> +	test_create_repo $1 &&
>  	(
> -		cd sub &&
> +		cd $1 &&

Missing dq, i.e. "$1" (two instances).

> +cat > status_expect << EOF

	cat >status_expect <<\EOF

Unless you actively mean the here-text has variables that need to be substituted, always quote the end-of-here-text delimiter. That would reduce the mental burden of people who have to read the script, as they do not have to look for a potential substitution that does not exist.

Show 11 quoted lines

> +# On branch merge_conflict_gitmodules
> +# Changes to be committed:
> +#
> +#	new file:   sub1
> +#
> +# Unmerged paths:
> +#   (use "git add/rm <file>..." as appropriate to mark resolution)
> +#
> +#	both added:         .gitmodules
> +#
> +EOF

Is it absolutely necessary to run this test by comparing the output of human readable version of "git status" output? It makes it impossible to test under gettext-poison. Can you use "git status -s" output instead?

Show 8 quoted lines

> ...
> +test_expect_failure 'diff --submodule with merge conflict in .gitmodules' '
> +	(
> +		cd super &&
> +		git diff --submodule > ../diff_submodule_actual 2>&1
> +	) &&
> +	test_cmp diff_submodule_actual diff_submodule_expect
> +'

You have too many ">" redirect followed by an unnecessary SP. There are existing examples in the test, please do not make things worse.