git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>,
	"brian m . carlson" <sandals@crustytoothpaste.net>,
	Eric Sunshine <sunshine@sunshineco.com>,
	Elijah Newren <newren@gmail.com>, Eric Wong <e@80x24.org>
Subject: Re: [PATCH 4/4] tests: get rid of $_x05 from the test suite
Date: Thu, 11 Mar 2021 11:29:33 +0100	[thread overview]
Message-ID: <87tupigf02.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <xmqq8s6uh4a2.fsf@gitster.g>


On Thu, Mar 11 2021, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>
>> Remove the last users of the $_x05 variable from the tests. It turns
>> out that all of these tests can be rewritten unambiguously to simply
>> use [0-9a-f]* instead.
>
> I am unsure about the "unambiguously" part, e.g.
>
>> -	sed -e "s/ $_x05[0-9a-f]*	/ X	/" <current >check &&
>> +	sed -e "s/ [0-9a-f]*	/ X	/" <current >check &&
>
> does't the preimage say "we expect at least 5 hexdigits to be shown
> here"?  The postimage lets even an empty string to pass. 
>
>> In the case of the tree matching we're relying on there being a <TAB>
>> after the SHA (but a space between the modes and type), then in some
>> of the other tests here that an abbreviated SHA is at the start of the
>> line, etc.
>
> Sure, but these being tests, I am not sure we should be assuming the
> correct input to these transformations.
>
>>  test_expect_success 'ls-tree --abbrev=5' '
>>  	git ls-tree --abbrev=5 $tree >current &&
>> -	sed -e "s/ $_x05[0-9a-f]*	/ X	/" <current >check &&
>> +	sed -e "s/ [0-9a-f]*	/ X	/" <current >check &&
>>  	cat >expected <<\EOF &&
>>  100644 blob X	1.txt
>>  100644 blob X	2.txt
>
> This one is particularly iffy.  The --abbrev=5 test is designed to
> ensure that the resulting abbreviated object names are at least 5
> hexdigits long, even when the repository is so small that only 4
> hexdigits are sufficient to avoid ambiguity, while allowing the
> output to be longer than specified 5 (when 5 turns out to be
> insufficient for disambiguation).
>
> So, I dunno.

Yes, I think this patch should be dropped. Do you mind just dropping the
4/4 and having it be a 3-patch series? I can also re-submit a v2 like
that if it's easier...

I saw the feedback on 3/4, yeah I also think that "cut" is a bit nasty,
but probably not worth spending more time on it...

My assumption in writing this patch was that it was fine because we test
the details of abbrev behavior somewhere else, surely...

But is it turns out we don't, I was misrecalling that some version of my
testing for abbreviations in [1] had landed.

But alas it didn't. From rebasing it now (it's far from submission
quality yet) I see that the lack of testing on abbreviations in the
interim has silently caused some regressions on master.

E.g. we seemingly unintentionally started accepting "-c core.abbrev="
(empty string) in a9ecaa06a7 (core.abbrev=no disables abbreviations,
2020-09-01) is a side-effect of starting to accept boolean values. Also
"git branch --abbrev=-12345" became a non-error in the interim (haven't
dug, but some of the refactoring to parse_options() I assume..).

So yeah, I think this patch is a bad idea now, our test coverage for
abbreviations is really hanging on by a thread.

1. https://lore.kernel.org/git/20180608224136.20220-1-avarab@gmail.com/

  reply	other threads:[~2021-03-11 10:30 UTC|newest]

Thread overview: 116+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-28 23:34 [PATCH v5 00/39] SHA-256, part 3/3 brian m. carlson
2020-07-28 23:34 ` [PATCH v5 01/39] t: make test-bloom initialize repository brian m. carlson
2020-07-28 23:34 ` [PATCH v5 02/39] t1001: use $ZERO_OID brian m. carlson
2020-07-28 23:34 ` [PATCH v5 03/39] t3305: make hash agnostic brian m. carlson
2020-07-28 23:34 ` [PATCH v5 04/39] t3404: prepare 'short SHA-1 collision' tests for SHA-256 brian m. carlson
2020-07-28 23:34 ` [PATCH v5 05/39] t6100: make hash size independent brian m. carlson
2020-07-28 23:34 ` [PATCH v5 06/39] t6101: " brian m. carlson
2020-07-28 23:34 ` [PATCH v5 07/39] t6301: " brian m. carlson
2020-07-28 23:34 ` [PATCH v5 08/39] t6500: specify test values for SHA-256 brian m. carlson
2020-07-28 23:34 ` [PATCH v5 09/39] t6501: avoid hard-coded objects brian m. carlson
2020-07-28 23:34 ` [PATCH v5 10/39] t7003: compute appropriate length constant brian m. carlson
2020-07-28 23:34 ` [PATCH v5 11/39] t7063: make hash size independent brian m. carlson
2020-07-28 23:34 ` [PATCH v5 12/39] t7201: abstract away SHA-1-specific constants brian m. carlson
2020-07-28 23:34 ` [PATCH v5 13/39] t7102: " brian m. carlson
2020-07-28 23:34 ` [PATCH v5 14/39] t7400: make hash size independent brian m. carlson
2020-07-28 23:34 ` [PATCH v5 15/39] t7405: " brian m. carlson
2020-07-28 23:34 ` [PATCH v5 16/39] t7506: avoid checking for SHA-1-specific constants brian m. carlson
2020-07-28 23:34 ` [PATCH v5 17/39] t7508: use $ZERO_OID instead of hard-coded constant brian m. carlson
2020-07-28 23:34 ` [PATCH v5 18/39] t8002: make hash size independent brian m. carlson
2020-07-29  2:24   ` Eric Sunshine
2020-07-29 22:31     ` brian m. carlson
2020-07-28 23:34 ` [PATCH v5 19/39] t8003: " brian m. carlson
2020-07-28 23:34 ` [PATCH v5 20/39] t8011: " brian m. carlson
2020-07-28 23:34 ` [PATCH v5 21/39] t9300: abstract away SHA-1-specific constants brian m. carlson
2020-07-28 23:34 ` [PATCH v5 22/39] t9300: use $ZERO_OID instead of hard-coded object ID brian m. carlson
2020-07-28 23:34 ` [PATCH v5 23/39] t9301: make hash size independent brian m. carlson
2020-07-28 23:34 ` [PATCH v5 24/39] t9350: " brian m. carlson
2020-07-28 23:34 ` [PATCH v5 25/39] t9500: ensure that algorithm info is preserved in config brian m. carlson
2020-07-28 23:34 ` [PATCH v5 26/39] t9700: make hash size independent brian m. carlson
2020-07-28 23:34 ` [PATCH v5 27/39] t5308: make test work with SHA-256 brian m. carlson
2020-07-28 23:34 ` [PATCH v5 28/39] t0410: mark test with SHA1 prerequisite brian m. carlson
2020-07-28 23:34 ` [PATCH v5 29/39] http-fetch: set up git directory before parsing pack hashes brian m. carlson
2020-07-29  2:36   ` Eric Sunshine
2020-07-28 23:34 ` [PATCH v5 30/39] builtin/verify-pack: implement an --object-format option brian m. carlson
2020-07-28 23:34 ` [PATCH v5 31/39] bundle: add new version for use with SHA-256 brian m. carlson
2020-07-29  2:53   ` Eric Sunshine
2020-07-29 22:28     ` brian m. carlson
2020-07-28 23:34 ` [PATCH v5 32/39] setup: add support for reading extensions.objectformat brian m. carlson
2020-07-28 23:34 ` [PATCH v5 33/39] Enable SHA-256 support by default brian m. carlson
2020-07-29  2:57   ` Eric Sunshine
2020-07-28 23:34 ` [PATCH v5 34/39] t: add test_oid option to select hash algorithm brian m. carlson
2020-07-28 23:34 ` [PATCH v5 35/39] t: allow testing different hash algorithms via environment brian m. carlson
2020-07-28 23:34 ` [PATCH v5 36/39] t: make SHA1 prerequisite depend on default hash brian m. carlson
2020-07-29  3:01   ` Eric Sunshine
2020-07-28 23:34 ` [PATCH v5 37/39] ci: run tests with SHA-256 brian m. carlson
2020-07-28 23:34 ` [PATCH v5 38/39] docs: add documentation for extensions.objectFormat brian m. carlson
2020-07-28 23:34 ` [PATCH v5 39/39] t: remove test_oid_init in tests brian m. carlson
2020-07-29 23:13 ` [PATCH v6 00/39] SHA-256, part 3/3 brian m. carlson
2020-07-29 23:13   ` [PATCH v6 01/39] t: make test-bloom initialize repository brian m. carlson
2020-07-29 23:13   ` [PATCH v6 02/39] t1001: use $ZERO_OID brian m. carlson
2020-07-29 23:13   ` [PATCH v6 03/39] t3305: make hash agnostic brian m. carlson
2020-07-29 23:13   ` [PATCH v6 04/39] t3404: prepare 'short SHA-1 collision' tests for SHA-256 brian m. carlson
2020-07-29 23:13   ` [PATCH v6 05/39] t6100: make hash size independent brian m. carlson
2020-07-29 23:13   ` [PATCH v6 06/39] t6101: " brian m. carlson
2020-07-29 23:13   ` [PATCH v6 07/39] t6301: " brian m. carlson
2020-07-29 23:13   ` [PATCH v6 08/39] t6500: specify test values for SHA-256 brian m. carlson
2020-07-29 23:13   ` [PATCH v6 09/39] t6501: avoid hard-coded objects brian m. carlson
2020-07-29 23:13   ` [PATCH v6 10/39] t7003: compute appropriate length constant brian m. carlson
2020-07-29 23:14   ` [PATCH v6 11/39] t7063: make hash size independent brian m. carlson
2020-07-29 23:14   ` [PATCH v6 12/39] t7201: abstract away SHA-1-specific constants brian m. carlson
2020-07-29 23:14   ` [PATCH v6 13/39] t7102: " brian m. carlson
2020-07-29 23:14   ` [PATCH v6 14/39] t7400: make hash size independent brian m. carlson
2020-07-29 23:14   ` [PATCH v6 15/39] t7405: " brian m. carlson
2020-07-29 23:14   ` [PATCH v6 16/39] t7506: avoid checking for SHA-1-specific constants brian m. carlson
2020-07-29 23:14   ` [PATCH v6 17/39] t7508: use $ZERO_OID instead of hard-coded constant brian m. carlson
2020-07-29 23:14   ` [PATCH v6 18/39] t8002: make hash size independent brian m. carlson
2020-07-29 23:14   ` [PATCH v6 19/39] t8003: " brian m. carlson
2020-07-29 23:14   ` [PATCH v6 20/39] t8011: " brian m. carlson
2020-07-29 23:14   ` [PATCH v6 21/39] t9300: abstract away SHA-1-specific constants brian m. carlson
2020-07-29 23:14   ` [PATCH v6 22/39] t9300: use $ZERO_OID instead of hard-coded object ID brian m. carlson
2020-07-29 23:14   ` [PATCH v6 23/39] t9301: make hash size independent brian m. carlson
2020-07-29 23:14   ` [PATCH v6 24/39] t9350: " brian m. carlson
2020-07-29 23:14   ` [PATCH v6 25/39] t9500: ensure that algorithm info is preserved in config brian m. carlson
2020-07-29 23:14   ` [PATCH v6 26/39] t9700: make hash size independent brian m. carlson
2020-07-29 23:14   ` [PATCH v6 27/39] t5308: make test work with SHA-256 brian m. carlson
2020-07-29 23:14   ` [PATCH v6 28/39] t0410: mark test with SHA1 prerequisite brian m. carlson
2020-07-29 23:14   ` [PATCH v6 29/39] http-fetch: set up git directory before parsing pack hashes brian m. carlson
2020-07-29 23:14   ` [PATCH v6 30/39] builtin/verify-pack: implement an --object-format option brian m. carlson
2020-07-29 23:14   ` [PATCH v6 31/39] bundle: add new version for use with SHA-256 brian m. carlson
2020-07-29 23:14   ` [PATCH v6 32/39] setup: add support for reading extensions.objectformat brian m. carlson
2020-07-29 23:14   ` [PATCH v6 33/39] repository: enable SHA-256 support by default brian m. carlson
2020-07-29 23:14   ` [PATCH v6 34/39] t: add test_oid option to select hash algorithm brian m. carlson
2020-07-29 23:14   ` [PATCH v6 35/39] t: allow testing different hash algorithms via environment brian m. carlson
2020-07-29 23:14   ` [PATCH v6 36/39] t: make SHA1 prerequisite depend on default hash brian m. carlson
2020-07-29 23:14   ` [PATCH v6 37/39] ci: run tests with SHA-256 brian m. carlson
2020-07-29 23:14   ` [PATCH v6 38/39] docs: add documentation for extensions.objectFormat brian m. carlson
2020-07-29 23:14   ` [PATCH v6 39/39] t: remove test_oid_init in tests brian m. carlson
2020-07-30  2:48   ` [PATCH v6 00/39] SHA-256, part 3/3 Eric Sunshine
2020-07-30 16:18     ` Junio C Hamano
2021-03-10 12:04   ` Ævar Arnfjörð Bjarmason
2021-03-10 16:36     ` Elijah Newren
2021-03-10 17:08       ` Jeff King
2021-03-10 17:06     ` [PATCH 0/3] sha256 fixes for filter-branch Jeff King
2021-03-10 17:07       ` [PATCH 1/3] t7003: test ref rewriting explicitly Jeff King
2021-03-11  0:10         ` Ævar Arnfjörð Bjarmason
2021-03-11  1:55           ` brian m. carlson
2021-03-11  2:24             ` Jeff King
2021-03-11  2:49               ` brian m. carlson
2021-03-10 17:07       ` [PATCH 2/3] filter-branch: drop multiple-ancestor warning Jeff King
2021-03-10 22:16         ` Junio C Hamano
2021-03-10 17:07       ` [PATCH 3/3] filter-branch: drop $_x40 glob Jeff King
2021-03-10 22:23         ` Junio C Hamano
2021-03-11  0:14         ` [PATCH 0/4] bisect + tests: remove old $_x05, $_x35 and $_x40 variables Ævar Arnfjörð Bjarmason
2021-03-11  0:14         ` [PATCH 1/4] git-bisect: remove unused SHA-1 $x40 shell variable Ævar Arnfjörð Bjarmason
2021-03-11  0:14         ` [PATCH 2/4] test-lib: remove unused $_x40 and $_z40 variables Ævar Arnfjörð Bjarmason
2021-03-11  1:02           ` Junio C Hamano
2021-03-11  0:14         ` [PATCH 3/4] shortlog tests: rewrite to get rid of --abbrev=35 hardcoding Ævar Arnfjörð Bjarmason
2021-03-11  1:18           ` Junio C Hamano
2021-03-11 19:15           ` Jeff King
2021-03-11  0:14         ` [PATCH 4/4] tests: get rid of $_x05 from the test suite Ævar Arnfjörð Bjarmason
2021-03-11  1:23           ` Junio C Hamano
2021-03-11 10:29             ` Ævar Arnfjörð Bjarmason [this message]
2021-03-11 17:40               ` Junio C Hamano
2021-03-10 17:54       ` [PATCH 0/3] sha256 fixes for filter-branch Elijah Newren
2021-03-10 22:24         ` Junio C Hamano
2021-03-10 23:32       ` brian m. carlson

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=87tupigf02.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=e@80x24.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    --cc=sandals@crustytoothpaste.net \
    --cc=sunshine@sunshineco.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).