All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Philip Oakley <philipoakley@iee.org>,
	Git Mailing List <git@vger.kernel.org>,
	Daniel Barkalow <barkalow@iabervon.org>,
	Ann T Ropea <bedhanger@gmx.de>
Subject: Re: [PATCH v2 6/6] Testing: provide tests requiring them with ellipses after SHA-1 values
Date: Tue, 14 Nov 2017 12:20:11 +0900	[thread overview]
Message-ID: <xmqqwp2t5yxg.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20171113223654.27732-6-bedhanger@gmx.de> (Ann T. Ropea's message of "Mon, 13 Nov 2017 23:36:54 +0100")

Ann T Ropea <bedhanger@gmx.de> 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 <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.

> 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

      reply	other threads:[~2017-11-14  3:20 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-05 16:27 [PATCH 1/3] checkout: describe_detached_head: remove 3dots after committish Ann T Ropea
2017-11-05 16:27 ` [PATCH 2/3] Documentation: user-manual: limit potentially confusing usage of 3dots (and 2dots) Ann T Ropea
2017-11-05 16:27 ` [PATCH 3/3] Documentation: revisions: add note about 3dots usages as continuation indications Ann T Ropea
2017-11-06  4:34   ` Junio C Hamano
2017-11-06  2:45 ` [PATCH 1/3] checkout: describe_detached_head: remove 3dots after committish Junio C Hamano
2017-11-07  0:30   ` Philip Oakley
2017-11-07  0:52 ` Junio C Hamano
2017-11-07  2:53 ` Ann T Ropea
2017-11-07 23:25   ` Philip Oakley
2017-11-08  1:59     ` Junio C Hamano
2017-11-09 23:15       ` Philip Oakley
2017-11-13 22:36         ` [PATCH v2 1/6] config: introduce core.printsha1ellipsis Ann T Ropea
2017-11-13 22:36         ` [PATCH v2 2/6] checkout: describe_detached_head: remove ellipsis after committish Ann T Ropea
2017-11-13 22:36         ` [PATCH v2 3/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value Ann T Ropea
2017-11-14  3:08           ` Junio C Hamano
2017-11-19 17:38             ` Ann T Ropea
2017-11-20  1:48               ` Junio C Hamano
2017-11-19 18:41             ` [PATCH v3 1/5] checkout: describe_detached_head: remove ellipsis after committish Ann T Ropea
2017-11-20  3:35               ` Junio C Hamano
2017-11-19 18:41             ` [PATCH v3 2/5] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value Ann T Ropea
2017-11-19 19:11               ` Eric Sunshine
2017-11-19 18:41             ` [PATCH v3 3/5] Documentation: user-manual: limit usage of ellipsis Ann T Ropea
2017-11-19 19:15               ` Eric Sunshine
2017-11-24 23:53                 ` [PATCH v4 1/6] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value Ann T Ropea
2017-11-25  5:01                   ` Junio C Hamano
2017-11-26  3:17                     ` Junio C Hamano
2017-11-26  3:19                       ` Junio C Hamano
2017-11-26  3:25                       ` Junio C Hamano
2017-12-03 21:27                       ` [PATCH v5 1/7] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot") Ann T Ropea
2017-12-04 16:52                         ` Junio C Hamano
2017-12-03 21:27                       ` [PATCH v5 2/7] Documentation: user-manual: limit usage of ellipsis Ann T Ropea
2017-12-03 21:27                       ` [PATCH v5 3/7] print_sha1_ellipsis: introduce helper Ann T Ropea
2017-12-03 21:27                       ` [PATCH v5 4/7] checkout: describe_detached_head: remove ellipsis after committish Ann T Ropea
2017-12-04 16:46                         ` Junio C Hamano
2017-12-04 23:13                           ` [PATCH v6 " Ann T Ropea
2017-12-05 16:03                             ` Junio C Hamano
2017-12-06  0:20                               ` [PATCH v7 " Ann T Ropea
2017-12-06 16:47                                 ` Junio C Hamano
2017-12-06 22:02                                   ` Ann T Ropea
2017-12-03 21:27                       ` [PATCH v5 5/7] t4013: prepare for upcoming "diff --raw --abbrev" output format change Ann T Ropea
2017-12-03 21:27                       ` [PATCH v5 6/7] diff: diff_aligned_abbrev: remove ellipsis after abbreviated SHA-1 value Ann T Ropea
2017-12-03 21:27                       ` [PATCH v5 7/7] t4013: test new output from diff --abbrev --raw Ann T Ropea
2017-11-24 23:53                 ` [PATCH v4 2/6] checkout: describe_detached_head: remove ellipsis after committish Ann T Ropea
2017-11-24 23:53                 ` [PATCH v4 3/6] Documentation: user-manual: limit usage of ellipsis Ann T Ropea
2017-11-24 23:53                 ` [PATCH v4 4/6] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot") Ann T Ropea
2017-11-24 23:53                 ` [PATCH v4 5/6] Documentation: git: document GIT_PRINT_SHA1_ELLIPSIS Ann T Ropea
2017-11-24 23:53                 ` [PATCH v4 6/6] Testing: provide existing tests requiring them with ellipses after SHA-1 values Ann T Ropea
2017-11-19 18:41             ` [PATCH v3 4/5] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot") Ann T Ropea
2017-11-19 18:41             ` [PATCH v3 5/5] Testing: provide tests requiring them with ellipses after SHA-1 values Ann T Ropea
2017-11-20  4:06               ` Junio C Hamano
2017-11-20 12:25                 ` Philip Oakley
2017-11-22  5:53                   ` Junio C Hamano
2017-11-22 23:41                     ` Philip Oakley
2017-11-24  0:40                       ` Junio C Hamano
2017-11-13 22:36         ` [PATCH v2 4/6] Documentation: user-manual: limit usage of ellipsis Ann T Ropea
2017-11-13 22:36         ` [PATCH v2 5/6] Documentation: revisions: fix typo: "three dot" ---> "three-dot" (in line with "two-dot") Ann T Ropea
2017-11-13 22:36         ` [PATCH v2 6/6] Testing: provide tests requiring them with ellipses after SHA-1 values Ann T Ropea
2017-11-14  3:20           ` Junio C Hamano [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqwp2t5yxg.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=barkalow@iabervon.org \
    --cc=bedhanger@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=philipoakley@iee.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.