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

From: Junio C Hamano <[email-masked]>
Date: Tue, 14 Nov 2017 12:20:11 +0900

Ann T Ropea <[email-masked]> writes:

> 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 <[email-masked]>
> ---

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.

> 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