Re: [PATCH 2/2] t6500: mark tests as SHA1 reliant

["brian m. carlson" <sandals@crustytoothpaste.net>]

[-- Attachment #1: Type: text/plain, Size: 2297 bytes --]

On Fri, Jul 28, 2017 at 03:14:49PM -0700, Junio C Hamano wrote:
> Stefan Beller <sbeller@google.com> writes:
> 
> > The first test marked relies on hard coded sha1:
> >
> > 	# We need to create two object whose sha1s start with 17
> > 	# since this is what git gc counts.  As it happens, these
> > 	# two blobs will do so.
> > 	test_commit 263 &&
> > 	test_commit 410 &&
> >
> > The next two seem to rely on state from the first one, I did not
> > investigate.
> 
> I am moderately negative on this approach, if it is meant to suggest
> the final shape of our test suite patch 1/2 started.
> 
> This script may be a good example you can use to demonstrate a much
> better approach.  As the above comment in the test shows, we want to
> create two objects whose object names begin with "17", and running
> test_commit with 263 and 410 at this point in the test was a way to
> achieve that when Git uses SHA-1 as its hash.
> 
> When we use a hash different from SHA-1, the exact strings 263 and
> 410 may change, but we should be able to find two other strings that
> has the same property (i.e. they results in objects that share the
> prefix "17").  Perhaps a better way forward for this kind of test is
> to parameterize these hardcoded constants and make it easier to use
> different values without having to change the rest of the script
> when we switch the hash function?  So perhaps have something like
> 
> 	case "$GIT_HASH_FUNCTION" in
> 	SHA-1)	
> 		TEST_17_1="263 410" ;;
> 	CORRUPT-SHA-1)	
> 		TEST_17_1="something else" ;;
>         esac

One approach I had considered taking is having a helper of some sort
that wrapped a simple key/value store.  We could pass the wrapper the
SHA-1 value (or, if necessary, an arbitrary key) and have it return the
proper value based on the given hash function.

That does have the downsides that the values may not present in the
tests themselves, and that people adding new tests will of course need
to run the test suite twice.  But it does make the tests easier to read.

Opinions on the desirability of this approach are of course welcome.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]