From: "brian m. carlson" <sandals@crustytoothpaste.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH 11/38] t7063: make hash size independent
Date: Sat, 11 Jul 2020 00:54:07 +0000 [thread overview]
Message-ID: <20200711005407.GL9782@camp.crustytoothpaste.net> (raw)
In-Reply-To: <xmqqmu46rieb.fsf@gitster.c.googlers.com>
[-- Attachment #1: Type: text/plain, Size: 2624 bytes --]
On 2020-07-11 at 00:43:08, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>
> > Use test_oid instead of hard-coding a fixed size all-zeros object ID.
> >
> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> > ---
> > t/t7063-status-untracked-cache.sh | 155 ++++++++++++++++--------------
> > 1 file changed, 83 insertions(+), 72 deletions(-)
>
> The stated objective does make sense, but ...
>
>
> > -info/exclude 13263c0978fb9fad16b2d580fb800b6d811c3ff0
> > -core.excludesfile 0000000000000000000000000000000000000000
> > +info/exclude $(test_oid exclude)
> > +core.excludesfile $ZERO_OID
> > exclude_per_dir .gitignore
> > flags 00000006
> > -/ e6fcc8f2ee31bae321d66afd183fcb7237afae6e recurse
> > -/done/ 0000000000000000000000000000000000000000 recurse valid
> > -/dthree/ 0000000000000000000000000000000000000000 recurse check_only valid
> > -/dtwo/ 0000000000000000000000000000000000000000 recurse check_only valid
> > +/ $(test_oid root) recurse
> > +/done/ $ZERO_OID recurse valid
> > +/dthree/ $ZERO_OID recurse check_only valid
> > +/dtwo/ $ZERO_OID recurse check_only valid
> > two
> > EOF
> > - test_cmp ../expect ../actual
> > + test_might_fail test_cmp ../expect ../actual
>
> Any "cmd" that is run under test_might_fail that is *not* used for
> its side effect is suspect---e.g. "we would try to remove this file
> as the test may have created it, but it is OK if the file does not
> exist and removal fails" is sort-of understandable, but I am having
> a hard time imagining in what situation it makes sense for a test to
> say "these two files may have the same contents but it is OK if that
> is not the case". There are a few others in this patch.
>
> Another topic in flight tightens the allowed usage of test_must_fail
> and test_might_fail helpers, and that is how I found this (because
> the tip of 'seen' does not pass the test), but regardless of that
> tightening, I am not sure what this "these two files may or may not
> be equal" is trying to achieve.
I agree this is suspect, and I'm not sure what I intended there. The
patch is unfortunately over two years old, so I don't recall exactly,
but it may have been flaky (in general) at the time I was changing the
test and I may have accidentally squashed that into the patch at the
time.
The test passes without it, so clearly the right thing is to remove the
test_might_fail, which I agree is bizarre. Feel free to do that when
picking it up, and I'll fix that in in the reroll.
--
brian m. carlson: Houston, Texas, US
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
next prev parent reply other threads:[~2020-07-11 0:54 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-10 2:46 [PATCH 00/38] SHA-256, part 3/3 brian m. carlson
2020-07-10 2:46 ` [PATCH 01/38] t: make test-bloom initialize repository brian m. carlson
2020-07-10 2:46 ` [PATCH 02/38] t1001: use $ZERO_OID brian m. carlson
2020-07-10 2:46 ` [PATCH 03/38] t3305: make hash agnostic brian m. carlson
2020-07-10 2:46 ` [PATCH 04/38] t3404: prepare 'short SHA-1 collision' tests for SHA-256 brian m. carlson
2020-07-10 2:46 ` [PATCH 05/38] t6100: make hash size independent brian m. carlson
2020-07-10 2:46 ` [PATCH 06/38] t6101: " brian m. carlson
2020-07-10 2:46 ` [PATCH 07/38] t6301: " brian m. carlson
2020-07-10 2:46 ` [PATCH 08/38] t6500: specify test values for SHA-256 brian m. carlson
2020-07-10 2:46 ` [PATCH 09/38] t6501: avoid hard-coded objects brian m. carlson
2020-07-10 2:47 ` [PATCH 10/38] t7003: compute appropriate length constant brian m. carlson
2020-07-10 2:47 ` [PATCH 11/38] t7063: make hash size independent brian m. carlson
2020-07-11 0:43 ` Junio C Hamano
2020-07-11 0:54 ` brian m. carlson [this message]
2020-07-10 2:47 ` [PATCH 12/38] t7201: abstract away SHA-1-specific constants brian m. carlson
2020-07-10 2:47 ` [PATCH 13/38] t7102: " brian m. carlson
2020-07-10 2:47 ` [PATCH 14/38] t7400: make hash size independent brian m. carlson
2020-07-10 2:47 ` [PATCH 15/38] t7405: " brian m. carlson
2020-07-10 2:47 ` [PATCH 16/38] t7506: avoid checking for SHA-1-specific constants brian m. carlson
2020-07-10 2:47 ` [PATCH 17/38] t7508: use $ZERO_OID instead of hard-coded constant brian m. carlson
2020-07-10 2:47 ` [PATCH 18/38] t8002: make hash size independent brian m. carlson
2020-07-10 2:47 ` [PATCH 19/38] t8003: " brian m. carlson
2020-07-10 2:47 ` [PATCH 20/38] t8011: " brian m. carlson
2020-07-10 2:47 ` [PATCH 21/38] t9300: abstract away SHA-1-specific constants brian m. carlson
2020-07-10 2:47 ` [PATCH 22/38] t9300: use $ZERO_OID instead of hard-coded object ID brian m. carlson
2020-07-10 2:47 ` [PATCH 23/38] t9301: make hash size independent brian m. carlson
2020-07-10 2:47 ` [PATCH 24/38] t9350: " brian m. carlson
2020-07-10 2:47 ` [PATCH 25/38] t9500: ensure that algorithm info is preserved in config brian m. carlson
2020-07-10 2:47 ` [PATCH 26/38] t9700: make hash size independent brian m. carlson
2020-07-10 2:47 ` [PATCH 27/38] t5308: make test work with SHA-256 brian m. carlson
2020-07-10 2:47 ` [PATCH 28/38] t0410: mark test with SHA1 prerequisite brian m. carlson
2020-07-10 2:47 ` [PATCH 29/38] http-fetch: set up git directory before parsing pack hashes brian m. carlson
2020-07-10 2:47 ` [PATCH 30/38] builtin/verify-pack: implement an --object-format option brian m. carlson
2020-07-10 2:47 ` [PATCH 31/38] setup: add support for reading extensions.objectformat brian m. carlson
2020-07-10 2:47 ` [PATCH 32/38] Enable SHA-256 support by default brian m. carlson
2020-07-10 2:47 ` [PATCH 33/38] t: add test_oid option to select hash algorithm brian m. carlson
2020-07-10 2:47 ` [PATCH 34/38] t: allow testing different hash algorithms via environment brian m. carlson
2020-07-10 2:47 ` [PATCH 35/38] t: make SHA1 prerequisite depend on default hash brian m. carlson
2020-07-12 2:13 ` Denton Liu
2020-07-12 17:34 ` Junio C Hamano
2020-07-10 2:47 ` [PATCH 36/38] ci: run tests with SHA-256 brian m. carlson
2020-07-10 15:09 ` Derrick Stolee
2020-07-10 2:47 ` [PATCH 37/38] docs: add documentation for extensions.objectFormat brian m. carlson
2020-07-10 20:21 ` Martin Ågren
2020-07-10 2:47 ` [PATCH 38/38] t: remove test_oid_init in tests brian m. carlson
2020-07-10 15:14 ` [PATCH 00/38] SHA-256, part 3/3 Derrick Stolee
2020-07-10 19:55 ` brian m. carlson
2020-07-10 20:15 ` Junio C Hamano
2020-07-11 0:37 ` Junio C Hamano
2020-07-11 1:06 ` 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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200711005407.GL9782@camp.crustytoothpaste.net \
--to=sandals@crustytoothpaste.net \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.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.
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.