Re: [PATCH v2 6/6] Testing: provide tests requiring them with ellipses after SHA-1 values

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 14, 2017, 03:20 UTC
Message-ID: <xmqqwp2t5yxg.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20171113223654.27732-6-bedhanger@gmx.de>

Ann T Ropea <bedhanger@gmx.de> writes:

Show 9 quoted lines

> Where needed, we arrange for invocations of Git as if
>
>    "-c core.printsha1ellipsis=true"
>
> had been specified on the command-line.  This furnishes ellipses in the
> output which then matches what is expected.
>
> Signed-off-by: Ann T Ropea <bedhanger@gmx.de>
> ---

I am not a huge fan of exposing undocumented implementation details to the test scripts that much. There are three in t13xx series that mention GIT_CONFIG_PARAMETERS, but these are about testing the config mechanism itself.

An end-user script would instead be doing "git -c var=val" for each invocation but this one is being lazy because it does not want to bother identifying which "git" invocation needs the treatment and also it does not want to keep maintaining it, which is understandable, but it feels dirty.

This makes me wonder if core.printsha1ellipsis should really be a configuration variable in the first place. Wouldn't an environment variable be more appropriate? After all, the users of scripts that would be broken by this series would need to the same thing as what we do to our tests to keep them working while they update them.

Show 75 quoted lines

> v2: rename patch series & focus on removal of ellipses
>  t/t3040-subprojects-basic.sh | 12 ++++++++++++
>  t/t4013-diff-various.sh      | 12 ++++++++++++
>  t/t9300-fast-import.sh       | 12 ++++++++++++
>  3 files changed, 36 insertions(+)
>
> diff --git a/t/t3040-subprojects-basic.sh b/t/t3040-subprojects-basic.sh
> index 0a4ff6d824a0..63b85bfdd4f9 100755
> --- a/t/t3040-subprojects-basic.sh
> +++ b/t/t3040-subprojects-basic.sh
> @@ -3,6 +3,18 @@
>  test_description='Basic subproject functionality'
>  . ./test-lib.sh
>  
> +# Some of the tests expect an ellipsis after the (abbreviated)
> +# SHA-1 value.  The code below results in Git being called with
> +# "-c core.printsha1ellipsis=true" which satisfies those tests.
> +do_print_sha1_ellipsis="'core.printsha1ellipsis=true'"
> +if test -z "${GIT_CONFIG_PARAMETERS}"
> +then
> +	GIT_CONFIG_PARAMETERS="${do_print_sha1_ellipsis}"
> +else
> +	GIT_CONFIG_PARAMETERS="${GIT_CONFIG_PARAMETERS} ${do_print_sha1_ellipsis}"
> +fi
> +export GIT_CONFIG_PARAMETERS
> +
>  test_expect_success 'setup: create superproject' '
>  	: >Makefile &&
>  	git add Makefile &&
> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
> index c515e3e53fee..8ee14c7c6796 100755
> --- a/t/t4013-diff-various.sh
> +++ b/t/t4013-diff-various.sh
> @@ -7,6 +7,18 @@ test_description='Various diff formatting options'
>  
>  . ./test-lib.sh
>  
> +# Some of the tests expect an ellipsis after the (abbreviated)
> +# SHA-1 value.  The code below results in Git being called with
> +# "-c core.printsha1ellipsis=true" which satisfies those tests.
> +do_print_sha1_ellipsis="'core.printsha1ellipsis=true'"
> +if test -z "${GIT_CONFIG_PARAMETERS}"
> +then
> +	GIT_CONFIG_PARAMETERS="${do_print_sha1_ellipsis}"
> +else
> +	GIT_CONFIG_PARAMETERS="${GIT_CONFIG_PARAMETERS} ${do_print_sha1_ellipsis}"
> +fi
> +export GIT_CONFIG_PARAMETERS
> +
>  LF='
>  '
>  
> diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
> index d47560b6343d..6cc41b90dafa 100755
> --- a/t/t9300-fast-import.sh
> +++ b/t/t9300-fast-import.sh
> @@ -7,6 +7,18 @@ test_description='test git fast-import utility'
>  . ./test-lib.sh
>  . "$TEST_DIRECTORY"/diff-lib.sh ;# test-lib chdir's into trash
>  
> +# Some of the tests expect an ellipsis after the (abbreviated)
> +# SHA-1 value.  The code below results in Git being called with
> +# "-c core.printsha1ellipsis=true" which satisfies those tests.
> +do_print_sha1_ellipsis="'core.printsha1ellipsis=true'"
> +if test -z "${GIT_CONFIG_PARAMETERS}"
> +then
> +	GIT_CONFIG_PARAMETERS="${do_print_sha1_ellipsis}"
> +else
> +	GIT_CONFIG_PARAMETERS="${GIT_CONFIG_PARAMETERS} ${do_print_sha1_ellipsis}"
> +fi
> +export GIT_CONFIG_PARAMETERS
> +
>  verify_packs () {
>  	for p in .git/objects/pack/*.pack
>  	do