git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH v3 00/11] Hash-independent tests (part 3)
@ 2018-08-29  0:56 brian m. carlson
  2018-08-29  0:56 ` [PATCH v3 01/11] t: add tool to translate hash-related values brian m. carlson
                   ` (10 more replies)
  0 siblings, 11 replies; 19+ messages in thread
From: brian m. carlson @ 2018-08-29  0:56 UTC (permalink / raw)
  To: git
  Cc: Jeff King, Eric Sunshine, Nguyễn Thái Ngọc Duy,
	Torsten Bögershausen

This is the next in the series of improvements to make tests
hash-independent.

A range-diff is below.

Changes from v2:
* Fix a typo in "zero_2".
* Provide better matching of expected output.
* Add and use test_oid_init instead of filename-based test_oid_cache.
* Add test_set_hash.
* Provide better error checking in newly added test functions.
* Move t0000 constants into the test, removing the separate file.
* Switch to using a differently named temporary file in t0027.

Changes from v1:
* Adopt pure shell approach for helper.
* Add tests for the helpers.
* Explicitly refer to SHA-256 now that we know it will be NewHash.
* Updated t0000 to remove SHA1 prerequisite.
* Change name of helper from test_translate to test_oid.
* Add helper to cache information in the shell.
* Simplified lookup of HEAD in t0002.
* Switched to using existing helper function in t0027.
* Simplified handling of IDs in t0064.

brian m. carlson (11):
  t: add tool to translate hash-related values
  t0000: use hash translation table
  t0000: update tests for SHA-256
  t0002: abstract away SHA-1 specific constants
  t0027: make hash size independent
  t0064: make hash size independent
  t1006: make hash size independent
  t1400: switch hard-coded object ID to variable
  t1405: make hash size independent
  t1406: make hash-size independent
  t1407: make hash size independent

 t/oid-info/hash-info           |   8 ++
 t/oid-info/oid                 |  29 +++++
 t/t0000-basic.sh               | 211 ++++++++++++++++++++++-----------
 t/t0002-gitfile.sh             |  27 +++--
 t/t0027-auto-crlf.sh           |   6 +-
 t/t0064-sha1-array.sh          |  49 ++++----
 t/t1006-cat-file.sh            |   6 +-
 t/t1400-update-ref.sh          |   2 +-
 t/t1405-main-ref-store.sh      |   4 +-
 t/t1406-submodule-ref-store.sh |   6 +-
 t/t1407-worktree-ref-store.sh  |   4 +-
 t/test-lib-functions.sh        |  44 +++++++
 12 files changed, 283 insertions(+), 113 deletions(-)
 create mode 100644 t/oid-info/hash-info
 create mode 100644 t/oid-info/oid

range-diff:

1:  fb66b1ff7d !  1:  a897533a90 t: add tool to translate hash-related values
   @@ -3,19 +3,19 @@
        t: add tool to translate hash-related values
    
        Add a test function helper, test_oid, that produces output that varies
   -    depending on the hash in use.  Add an additional helper, test_oid_cache,
   -    that can be used to load data for test_oid, either through a list of
   -    filenames or on standard input.  Check that these functions work in
   +    depending on the hash in use.  Add two additional helpers,
   +    test_oid_cache, which can be used to load data for test_oid from
   +    standard input, and test_oid_init, which can be used to load certain
   +    fixed values from lookup charts.  Check that these functions work in
        t0000, as the rest of the testsuite will soon come to depend on them.
    
        Implement two basic lookup charts, one for common invalid or synthesized
        object IDs, and one for various facts about the hash function in use.
   -    Individual tests can use their own translation files to map object IDs
   -    or other data that needs to vary across hash functions.  Provide
   -    versions for both SHA-1 and SHA-256.
   +    Provide versions for both SHA-1 and SHA-256.
    
   -    Note that due to the implementation used, keys cannot be anything but
   -    shell identifier characters.
   +    Note that due to the implementation used, names used for lookup can
   +    currently consist only of shell identifier characters.  If this is a
   +    problem in the future, we can hash the names before use.
    
        Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
        Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
   @@ -56,7 +56,7 @@
    +007	sha256:0000000000000000000000000000000000000000000000000000000000000007
    +# All zeros or Fs missing one or two hex segments.
    +zero_1		sha1:000000000000000000000000000000000000000
   -+zero_2		sha256:000000000000000000000000000000000000000000000000000000000000000
   ++zero_1		sha256:000000000000000000000000000000000000000000000000000000000000000
    +zero_2		sha1:00000000000000000000000000000000000000
    +zero_2		sha256:00000000000000000000000000000000000000000000000000000000000000
    +ff_1		sha1:fffffffffffffffffffffffffffffffffffffff
   @@ -76,33 +76,39 @@
       EOF
     "
     
   -+test_oid_cache hash-info oid
   ++test_oid_init
    +
   -+test_expect_success 'test_oid_info provides sane info by default' '
   -+	test_oid zero &&
   ++test_expect_success 'test_oid provides sane info by default' '
    +	test_oid zero >actual &&
   -+	grep "00*" actual &&
   -+	test "$(test_oid hexsz)" -eq $(wc -c <actual) &&
   -+	test $(( $(test_oid rawsz) * 2)) -eq "$(test_oid hexsz)"
   ++	grep "^00*$" actual &&
   ++	rawsz="$(test_oid rawsz)" &&
   ++	hexsz="$(test_oid hexsz)" &&
   ++	test "$hexsz" -eq $(wc -c <actual) &&
   ++	test $(( $rawsz * 2)) -eq "$hexsz"
    +'
    +
   -+test_expect_success 'test_oid_info can look up data for SHA-1' '
   -+	test_detect_hash sha1 &&
   -+	test_oid zero >actual &&
   -+	grep "00*" actual &&
   -+	test $(wc -c <actual) -eq 40 &&
   -+	test "$(test_oid rawsz)" -eq 20 &&
   -+	test "$(test_oid hexsz)" -eq 40
   -+'
   -+
   -+test_expect_success 'test_oid_info can look up data for SHA-256' '
   ++test_expect_success 'test_oid can look up data for SHA-1' '
    +	test_when_finished "test_detect_hash" &&
   -+	test_detect_hash sha256 &&
   ++	test_set_hash sha1 &&
    +	test_oid zero >actual &&
   -+	grep "00*" actual &&
   ++	grep "^00*$" actual &&
   ++	rawsz="$(test_oid rawsz)" &&
   ++	hexsz="$(test_oid hexsz)" &&
   ++	test $(wc -c <actual) -eq 40 &&
   ++	test "$rawsz" -eq 20 &&
   ++	test "$hexsz" -eq 40
   ++'
   ++
   ++test_expect_success 'test_oid can look up data for SHA-256' '
   ++	test_when_finished "test_detect_hash" &&
   ++	test_set_hash sha256 &&
   ++	test_oid zero >actual &&
   ++	grep "^00*$" actual &&
   ++	rawsz="$(test_oid rawsz)" &&
   ++	hexsz="$(test_oid hexsz)" &&
    +	test $(wc -c <actual) -eq 64 &&
   -+	test "$(test_oid rawsz)" -eq 32 &&
   -+	test "$(test_oid hexsz)" -eq 64
   ++	test "$rawsz" -eq 32 &&
   ++	test "$hexsz" -eq 64
    +'
    +
     ################################################################
   @@ -117,38 +123,46 @@
       '
     }
    +
   ++test_set_hash () {
   ++	test_hash_algo="$1"
   ++}
   ++
    +test_detect_hash () {
   -+	if test -n "$1"
   -+	then
   -+		test_hash_algo="$1"
   -+	else
   -+		test_hash_algo='sha1'
   -+	fi
   ++	test_hash_algo='sha1'
   ++}
   ++
   ++test_oid_init () {
   ++	test_oid_cache <"$TEST_DIRECTORY/oid-info/hash-info" &&
   ++	test_oid_cache <"$TEST_DIRECTORY/oid-info/oid"
    +}
    +
    +test_oid_cache () {
    +	test -n "$test_hash_algo" || test_detect_hash
   -+	if test -n "$1"
   -+	then
   -+		while test -n "$1"
   -+		do
   -+			test_oid_cache <"$TEST_DIRECTORY/oid-info/$1"
   -+			shift
   -+		done
   -+		return $?
   -+	fi
    +	while read _tag _rest
    +	do
   -+		case $_tag in \#*)
   ++		case $_tag in
   ++		\#*)
   ++			continue;;
   ++		?*)
   ++			# non-empty
   ++			;;
   ++		*)
   ++			# blank line
    +			continue;;
   -+		esac
    +
   -+		_k="${_rest%:*}"
   -+		_v="${_rest#*:}"
   -+		eval "test_oid_${_k}_$_tag=\"$_v\""
   ++		esac &&
   ++
   ++		_k="${_rest%:*}" &&
   ++		_v="${_rest#*:}" &&
   ++		{ echo "$_k" | egrep '^[a-z0-9]+$' >/dev/null ||
   ++			error 'bug in the test script: bad hash algorithm'; } &&
   ++		eval "test_oid_${_k}_$_tag=\"\$_v\"" || return 1
    +	done
    +}
    +
    +test_oid () {
   -+	eval "printf '%s' \"\${test_oid_${test_hash_algo}_$1}\""
   ++	eval "
   ++		test -n \"\${test_oid_${test_hash_algo}_$1+set}\" &&
   ++		printf '%s' \"\${test_oid_${test_hash_algo}_$1}\"
   ++	"
    +}
2:  7b87aac814 =  2:  d63dc976f4 t0000: use hash translation table
3:  39464fb0cc !  3:  9e55529efd t0000: update tests for SHA-256
   @@ -16,11 +16,14 @@
    
        Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
    
   -diff --git a/t/oid-info/t0000 b/t/oid-info/t0000
   -new file mode 100644
   ---- /dev/null
   -+++ b/t/oid-info/t0000
   +diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
   +--- a/t/t0000-basic.sh
   ++++ b/t/t0000-basic.sh
    @@
   + ################################################################
   + # Basics of the basics
   + 
   ++test_oid_cache <<\EOF
    +path0f sha1:f87290f8eb2cbbea7857214459a0739927eab154
    +path0f sha256:638106af7c38be056f3212cbd7ac65bc1bac74f420ca5a436ff006a9d025d17d
    +
   @@ -59,15 +62,7 @@
    +
    +simpletree sha1:7bb943559a305bdd6bdee2cef6e5df2413c3d30a
    +simpletree sha256:1710c07a6c86f9a3c7376364df04c47ee39e5a5e221fcdd84b743bc9bb7e2bc5
   -
   -diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
   ---- a/t/t0000-basic.sh
   -+++ b/t/t0000-basic.sh
   -@@
   - ################################################################
   - # Basics of the basics
   - 
   -+test_oid_cache t0000
   ++EOF
    +
     # updating a new file without --add should fail.
     test_expect_success 'git update-index without --add should fail adding' '
   @@ -79,8 +74,7 @@
    -test_expect_success SHA1 'validate object ID of a known tree' '
    -	test "$tree" = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a
    +test_expect_success 'validate object ID of a known tree' '
   -+echo $tree &&
   -+	test "$tree" = $(test_oid simpletree)
   ++	test "$tree" = "$(test_oid simpletree)"
         '
     
     # Removing paths.
   @@ -109,10 +103,8 @@
    +	100644 $(test_oid subp3f) 0	path3/subp3/file3
    +	120000 $(test_oid subp3s) 0	path3/subp3/file3sym
       EOF
   -+	cat expected &&
       test_cmp expected current
     '
   - 
    @@
       tree=$(git write-tree)
     '
   @@ -120,7 +112,7 @@
    -test_expect_success SHA1 'validate object ID for a known tree' '
    -	test "$tree" = 087704a96baf1c2d1c869a8b084481e121c88b5b
    +test_expect_success 'validate object ID for a known tree' '
   -+	test "$tree" = $(test_oid root)
   ++	test "$tree" = "$(test_oid root)"
     '
     
     test_expect_success 'showing tree with git ls-tree' '
4:  27fdff34e3 !  4:  4d5b8d4025 t0002: abstract away SHA-1 specific constants
   @@ -28,8 +28,10 @@
       test_cmp expected actual
     '
    @@
   + 		cd enter_repo &&
           git worktree add  ../foo refs/tags/foo
       ) &&
   ++	head=$(git -C enter_repo rev-parse HEAD) &&
       git ls-remote foo >actual &&
    -	cat >expected <<-\EOF &&
    -	946e985ab20de757ca5b872b16d64e92ff3803a9	HEAD
   @@ -44,6 +46,7 @@
     '
     
     test_expect_success 'enter_repo strict mode' '
   ++	head=$(git -C enter_repo rev-parse HEAD) &&
       git ls-remote --upload-pack="git upload-pack --strict" foo/.git >actual &&
    -	cat >expected <<-\EOF &&
    -	946e985ab20de757ca5b872b16d64e92ff3803a9	HEAD
5:  bd31663386 <  -:  ---------- t0027: make hash size independent
-:  ---------- >  5:  6037c4d10a t0027: make hash size independent
6:  5d638b82ad =  6:  dfad561965 t0064: make hash size independent
7:  e147833d05 !  7:  8a860a7492 t1006: make hash size independent
   @@ -15,7 +15,7 @@
       test_cmp expect actual
     '
     
   -+test_oid_cache hash-info
   ++test_oid_init
    +
     tree_sha1=$(git write-tree)
    -tree_size=33
8:  78529a19b2 =  8:  1baaf8374a t1400: switch hard-coded object ID to variable
9:  a3c4726386 =  9:  232c043ed7 t1405: make hash size independent
10:  e35b54a199 = 10:  9d0207bc52 t1406: make hash-size independent
11:  2d4a478d0e = 11:  d292c6311f t1407: make hash size independent


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3 01/11] t: add tool to translate hash-related values
  2018-08-29  0:56 [PATCH v3 00/11] Hash-independent tests (part 3) brian m. carlson
@ 2018-08-29  0:56 ` brian m. carlson
  2018-08-29  4:05   ` Jonathan Nieder
  2018-08-29 12:37   ` Derrick Stolee
  2018-08-29  0:56 ` [PATCH v3 02/11] t0000: use hash translation table brian m. carlson
                   ` (9 subsequent siblings)
  10 siblings, 2 replies; 19+ messages in thread
From: brian m. carlson @ 2018-08-29  0:56 UTC (permalink / raw)
  To: git
  Cc: Jeff King, Eric Sunshine, Nguyễn Thái Ngọc Duy,
	Torsten Bögershausen

Add a test function helper, test_oid, that produces output that varies
depending on the hash in use.  Add two additional helpers,
test_oid_cache, which can be used to load data for test_oid from
standard input, and test_oid_init, which can be used to load certain
fixed values from lookup charts.  Check that these functions work in
t0000, as the rest of the testsuite will soon come to depend on them.

Implement two basic lookup charts, one for common invalid or synthesized
object IDs, and one for various facts about the hash function in use.
Provide versions for both SHA-1 and SHA-256.

Note that due to the implementation used, names used for lookup can
currently consist only of shell identifier characters.  If this is a
problem in the future, we can hash the names before use.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/oid-info/hash-info    |  8 ++++++++
 t/oid-info/oid          | 29 +++++++++++++++++++++++++++
 t/t0000-basic.sh        | 35 ++++++++++++++++++++++++++++++++
 t/test-lib-functions.sh | 44 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 116 insertions(+)
 create mode 100644 t/oid-info/hash-info
 create mode 100644 t/oid-info/oid

diff --git a/t/oid-info/hash-info b/t/oid-info/hash-info
new file mode 100644
index 0000000000..ccdbfdf974
--- /dev/null
+++ b/t/oid-info/hash-info
@@ -0,0 +1,8 @@
+rawsz sha1:20
+rawsz sha256:32
+
+hexsz sha1:40
+hexsz sha256:64
+
+zero sha1:0000000000000000000000000000000000000000
+zero sha256:0000000000000000000000000000000000000000000000000000000000000000
diff --git a/t/oid-info/oid b/t/oid-info/oid
new file mode 100644
index 0000000000..a754970523
--- /dev/null
+++ b/t/oid-info/oid
@@ -0,0 +1,29 @@
+# These are some common invalid and partial object IDs used in tests.
+001	sha1:0000000000000000000000000000000000000001
+001	sha256:0000000000000000000000000000000000000000000000000000000000000001
+002	sha1:0000000000000000000000000000000000000002
+002	sha256:0000000000000000000000000000000000000000000000000000000000000002
+003	sha1:0000000000000000000000000000000000000003
+003	sha256:0000000000000000000000000000000000000000000000000000000000000003
+004	sha1:0000000000000000000000000000000000000004
+004	sha256:0000000000000000000000000000000000000000000000000000000000000004
+005	sha1:0000000000000000000000000000000000000005
+005	sha256:0000000000000000000000000000000000000000000000000000000000000005
+006	sha1:0000000000000000000000000000000000000006
+006	sha256:0000000000000000000000000000000000000000000000000000000000000006
+007	sha1:0000000000000000000000000000000000000007
+007	sha256:0000000000000000000000000000000000000000000000000000000000000007
+# All zeros or Fs missing one or two hex segments.
+zero_1		sha1:000000000000000000000000000000000000000
+zero_1		sha256:000000000000000000000000000000000000000000000000000000000000000
+zero_2		sha1:00000000000000000000000000000000000000
+zero_2		sha256:00000000000000000000000000000000000000000000000000000000000000
+ff_1		sha1:fffffffffffffffffffffffffffffffffffffff
+ff_1		sha256:fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
+ff_2		sha1:ffffffffffffffffffffffffffffffffffffff
+ff_2		sha256:ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
+# More various invalid OIDs.
+numeric		sha1:0123456789012345678901234567890123456789
+numeric		sha256:0123456789012345678901234567890123456789012345678901234567890123
+deadbeef	sha1:deadbeefdeadbeefdeadbeefdeadbeefdeadbeef
+deadbeef	sha256:deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index 850f651e4e..e3cace299e 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -821,6 +821,41 @@ test_expect_success 'tests clean up even on failures' "
 	EOF
 "
 
+test_oid_init
+
+test_expect_success 'test_oid provides sane info by default' '
+	test_oid zero >actual &&
+	grep "^00*$" actual &&
+	rawsz="$(test_oid rawsz)" &&
+	hexsz="$(test_oid hexsz)" &&
+	test "$hexsz" -eq $(wc -c <actual) &&
+	test $(( $rawsz * 2)) -eq "$hexsz"
+'
+
+test_expect_success 'test_oid can look up data for SHA-1' '
+	test_when_finished "test_detect_hash" &&
+	test_set_hash sha1 &&
+	test_oid zero >actual &&
+	grep "^00*$" actual &&
+	rawsz="$(test_oid rawsz)" &&
+	hexsz="$(test_oid hexsz)" &&
+	test $(wc -c <actual) -eq 40 &&
+	test "$rawsz" -eq 20 &&
+	test "$hexsz" -eq 40
+'
+
+test_expect_success 'test_oid can look up data for SHA-256' '
+	test_when_finished "test_detect_hash" &&
+	test_set_hash sha256 &&
+	test_oid zero >actual &&
+	grep "^00*$" actual &&
+	rawsz="$(test_oid rawsz)" &&
+	hexsz="$(test_oid hexsz)" &&
+	test $(wc -c <actual) -eq 64 &&
+	test "$rawsz" -eq 32 &&
+	test "$hexsz" -eq 64
+'
+
 ################################################################
 # Basics of the basics
 
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 4207af4077..2300ec49dd 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -1155,3 +1155,47 @@ depacketize () {
 		}
 	'
 }
+
+test_set_hash () {
+	test_hash_algo="$1"
+}
+
+test_detect_hash () {
+	test_hash_algo='sha1'
+}
+
+test_oid_init () {
+	test_oid_cache <"$TEST_DIRECTORY/oid-info/hash-info" &&
+	test_oid_cache <"$TEST_DIRECTORY/oid-info/oid"
+}
+
+test_oid_cache () {
+	test -n "$test_hash_algo" || test_detect_hash
+	while read _tag _rest
+	do
+		case $_tag in
+		\#*)
+			continue;;
+		?*)
+			# non-empty
+			;;
+		*)
+			# blank line
+			continue;;
+
+		esac &&
+
+		_k="${_rest%:*}" &&
+		_v="${_rest#*:}" &&
+		{ echo "$_k" | egrep '^[a-z0-9]+$' >/dev/null ||
+			error 'bug in the test script: bad hash algorithm'; } &&
+		eval "test_oid_${_k}_$_tag=\"\$_v\"" || return 1
+	done
+}
+
+test_oid () {
+	eval "
+		test -n \"\${test_oid_${test_hash_algo}_$1+set}\" &&
+		printf '%s' \"\${test_oid_${test_hash_algo}_$1}\"
+	"
+}

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 02/11] t0000: use hash translation table
  2018-08-29  0:56 [PATCH v3 00/11] Hash-independent tests (part 3) brian m. carlson
  2018-08-29  0:56 ` [PATCH v3 01/11] t: add tool to translate hash-related values brian m. carlson
@ 2018-08-29  0:56 ` brian m. carlson
  2018-08-29  0:56 ` [PATCH v3 03/11] t0000: update tests for SHA-256 brian m. carlson
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: brian m. carlson @ 2018-08-29  0:56 UTC (permalink / raw)
  To: git
  Cc: Jeff King, Eric Sunshine, Nguyễn Thái Ngọc Duy,
	Torsten Bögershausen

If the hash we're using is 32 bytes in size, attempting to insert a
20-byte object name won't work.  Since these are synthesized objects
that are almost all zeros, look them up in a translation table.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t0000-basic.sh | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index e3cace299e..b7f57ea052 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -1013,12 +1013,13 @@ test_expect_success SHA1 'validate object ID for a known tree' '
 
 test_expect_success 'put invalid objects into the index' '
 	rm -f .git/index &&
-	cat >badobjects <<-\EOF &&
-	100644 blob 1000000000000000000000000000000000000000	dir/file1
-	100644 blob 2000000000000000000000000000000000000000	dir/file2
-	100644 blob 3000000000000000000000000000000000000000	dir/file3
-	100644 blob 4000000000000000000000000000000000000000	dir/file4
-	100644 blob 5000000000000000000000000000000000000000	dir/file5
+	suffix=$(echo $ZERO_OID | sed -e "s/^.//") &&
+	cat >badobjects <<-EOF &&
+	100644 blob $(test_oid 001)	dir/file1
+	100644 blob $(test_oid 002)	dir/file2
+	100644 blob $(test_oid 003)	dir/file3
+	100644 blob $(test_oid 004)	dir/file4
+	100644 blob $(test_oid 005)	dir/file5
 	EOF
 	git update-index --index-info <badobjects
 '

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 03/11] t0000: update tests for SHA-256
  2018-08-29  0:56 [PATCH v3 00/11] Hash-independent tests (part 3) brian m. carlson
  2018-08-29  0:56 ` [PATCH v3 01/11] t: add tool to translate hash-related values brian m. carlson
  2018-08-29  0:56 ` [PATCH v3 02/11] t0000: use hash translation table brian m. carlson
@ 2018-08-29  0:56 ` brian m. carlson
  2018-08-29  0:56 ` [PATCH v3 04/11] t0002: abstract away SHA-1 specific constants brian m. carlson
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: brian m. carlson @ 2018-08-29  0:56 UTC (permalink / raw)
  To: git
  Cc: Jeff King, Eric Sunshine, Nguyễn Thái Ngọc Duy,
	Torsten Bögershausen

Test t0000 tests the "basics of the basics" and as such, checks that we
have various fixed hard-coded object IDs.  The tests relying on these
assertions have been marked with the SHA1 prerequisite, as they will
obviously not function in their current form with SHA-256.

Use the test_oid helper to update these assertions and provide values
for both SHA-1 and SHA-256.

These object IDs were synthesized using a set of scripts that created
the objects for both SHA-1 and SHA-256 using the same method to ensure
that they are indeed the correct values.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t0000-basic.sh | 163 +++++++++++++++++++++++++++++------------------
 1 file changed, 102 insertions(+), 61 deletions(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index b7f57ea052..72cf6bcb12 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -859,6 +859,47 @@ test_expect_success 'test_oid can look up data for SHA-256' '
 ################################################################
 # Basics of the basics
 
+test_oid_cache <<\EOF
+path0f sha1:f87290f8eb2cbbea7857214459a0739927eab154
+path0f sha256:638106af7c38be056f3212cbd7ac65bc1bac74f420ca5a436ff006a9d025d17d
+
+path0s sha1:15a98433ae33114b085f3eb3bb03b832b3180a01
+path0s sha256:3a24cc53cf68edddac490bbf94a418a52932130541361f685df685e41dd6c363
+
+path2f sha1:3feff949ed00a62d9f7af97c15cd8a30595e7ac7
+path2f sha256:2a7f36571c6fdbaf0e3f62751a0b25a3f4c54d2d1137b3f4af9cb794bb498e5f
+
+path2s sha1:d8ce161addc5173867a3c3c730924388daedbc38
+path2s sha256:18fd611b787c2e938ddcc248fabe4d66a150f9364763e9ec133dd01d5bb7c65a
+
+path2d sha1:58a09c23e2ca152193f2786e06986b7b6712bdbe
+path2d sha256:00e4b32b96e7e3d65d79112dcbea53238a22715f896933a62b811377e2650c17
+
+path3f sha1:0aa34cae68d0878578ad119c86ca2b5ed5b28376
+path3f sha256:09f58616b951bd571b8cb9dc76d372fbb09ab99db2393f5ab3189d26c45099ad
+
+path3s sha1:8599103969b43aff7e430efea79ca4636466794f
+path3s sha256:fce1aed087c053306f3f74c32c1a838c662bbc4551a7ac2420f5d6eb061374d0
+
+path3d sha1:21ae8269cacbe57ae09138dcc3a2887f904d02b3
+path3d sha256:9b60497be959cb830bf3f0dc82bcc9ad9e925a24e480837ade46b2295e47efe1
+
+subp3f sha1:00fb5908cb97c2564a9783c0c64087333b3b464f
+subp3f sha256:a1a9e16998c988453f18313d10375ee1d0ddefe757e710dcae0d66aa1e0c58b3
+
+subp3s sha1:6649a1ebe9e9f1c553b66f5a6e74136a07ccc57c
+subp3s sha256:81759d9f5e93c6546ecfcadb560c1ff057314b09f93fe8ec06e2d8610d34ef10
+
+subp3d sha1:3c5e5399f3a333eddecce7a9b9465b63f65f51e2
+subp3d sha256:76b4ef482d4fa1c754390344cf3851c7f883b27cf9bc999c6547928c46aeafb7
+
+root sha1:087704a96baf1c2d1c869a8b084481e121c88b5b
+root sha256:9481b52abab1b2ffeedbf9de63ce422b929f179c1b98ff7bee5f8f1bc0710751
+
+simpletree sha1:7bb943559a305bdd6bdee2cef6e5df2413c3d30a
+simpletree sha256:1710c07a6c86f9a3c7376364df04c47ee39e5a5e221fcdd84b743bc9bb7e2bc5
+EOF
+
 # updating a new file without --add should fail.
 test_expect_success 'git update-index without --add should fail adding' '
 	test_must_fail git update-index should-be-empty
@@ -874,8 +915,8 @@ test_expect_success 'writing tree out with git write-tree' '
 '
 
 # we know the shape and contents of the tree and know the object ID for it.
-test_expect_success SHA1 'validate object ID of a known tree' '
-	test "$tree" = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a
+test_expect_success 'validate object ID of a known tree' '
+	test "$tree" = "$(test_oid simpletree)"
     '
 
 # Removing paths.
@@ -917,16 +958,16 @@ test_expect_success 'showing stage with git ls-files --stage' '
 	git ls-files --stage >current
 '
 
-test_expect_success SHA1 'validate git ls-files output for a known tree' '
-	cat >expected <<-\EOF &&
-	100644 f87290f8eb2cbbea7857214459a0739927eab154 0	path0
-	120000 15a98433ae33114b085f3eb3bb03b832b3180a01 0	path0sym
-	100644 3feff949ed00a62d9f7af97c15cd8a30595e7ac7 0	path2/file2
-	120000 d8ce161addc5173867a3c3c730924388daedbc38 0	path2/file2sym
-	100644 0aa34cae68d0878578ad119c86ca2b5ed5b28376 0	path3/file3
-	120000 8599103969b43aff7e430efea79ca4636466794f 0	path3/file3sym
-	100644 00fb5908cb97c2564a9783c0c64087333b3b464f 0	path3/subp3/file3
-	120000 6649a1ebe9e9f1c553b66f5a6e74136a07ccc57c 0	path3/subp3/file3sym
+test_expect_success 'validate git ls-files output for a known tree' '
+	cat >expected <<-EOF &&
+	100644 $(test_oid path0f) 0	path0
+	120000 $(test_oid path0s) 0	path0sym
+	100644 $(test_oid path2f) 0	path2/file2
+	120000 $(test_oid path2s) 0	path2/file2sym
+	100644 $(test_oid path3f) 0	path3/file3
+	120000 $(test_oid path3s) 0	path3/file3sym
+	100644 $(test_oid subp3f) 0	path3/subp3/file3
+	120000 $(test_oid subp3s) 0	path3/subp3/file3sym
 	EOF
 	test_cmp expected current
 '
@@ -935,20 +976,20 @@ test_expect_success 'writing tree out with git write-tree' '
 	tree=$(git write-tree)
 '
 
-test_expect_success SHA1 'validate object ID for a known tree' '
-	test "$tree" = 087704a96baf1c2d1c869a8b084481e121c88b5b
+test_expect_success 'validate object ID for a known tree' '
+	test "$tree" = "$(test_oid root)"
 '
 
 test_expect_success 'showing tree with git ls-tree' '
     git ls-tree $tree >current
 '
 
-test_expect_success SHA1 'git ls-tree output for a known tree' '
-	cat >expected <<-\EOF &&
-	100644 blob f87290f8eb2cbbea7857214459a0739927eab154	path0
-	120000 blob 15a98433ae33114b085f3eb3bb03b832b3180a01	path0sym
-	040000 tree 58a09c23e2ca152193f2786e06986b7b6712bdbe	path2
-	040000 tree 21ae8269cacbe57ae09138dcc3a2887f904d02b3	path3
+test_expect_success 'git ls-tree output for a known tree' '
+	cat >expected <<-EOF &&
+	100644 blob $(test_oid path0f)	path0
+	120000 blob $(test_oid path0s)	path0sym
+	040000 tree $(test_oid path2d)	path2
+	040000 tree $(test_oid path3d)	path3
 	EOF
 	test_cmp expected current
 '
@@ -959,16 +1000,16 @@ test_expect_success 'showing tree with git ls-tree -r' '
 	git ls-tree -r $tree >current
 '
 
-test_expect_success SHA1 'git ls-tree -r output for a known tree' '
-	cat >expected <<-\EOF &&
-	100644 blob f87290f8eb2cbbea7857214459a0739927eab154	path0
-	120000 blob 15a98433ae33114b085f3eb3bb03b832b3180a01	path0sym
-	100644 blob 3feff949ed00a62d9f7af97c15cd8a30595e7ac7	path2/file2
-	120000 blob d8ce161addc5173867a3c3c730924388daedbc38	path2/file2sym
-	100644 blob 0aa34cae68d0878578ad119c86ca2b5ed5b28376	path3/file3
-	120000 blob 8599103969b43aff7e430efea79ca4636466794f	path3/file3sym
-	100644 blob 00fb5908cb97c2564a9783c0c64087333b3b464f	path3/subp3/file3
-	120000 blob 6649a1ebe9e9f1c553b66f5a6e74136a07ccc57c	path3/subp3/file3sym
+test_expect_success 'git ls-tree -r output for a known tree' '
+	cat >expected <<-EOF &&
+	100644 blob $(test_oid path0f)	path0
+	120000 blob $(test_oid path0s)	path0sym
+	100644 blob $(test_oid path2f)	path2/file2
+	120000 blob $(test_oid path2s)	path2/file2sym
+	100644 blob $(test_oid path3f)	path3/file3
+	120000 blob $(test_oid path3s)	path3/file3sym
+	100644 blob $(test_oid subp3f)	path3/subp3/file3
+	120000 blob $(test_oid subp3s)	path3/subp3/file3sym
 	EOF
 	test_cmp expected current
 '
@@ -978,19 +1019,19 @@ test_expect_success 'showing tree with git ls-tree -r -t' '
 	git ls-tree -r -t $tree >current
 '
 
-test_expect_success SHA1 'git ls-tree -r output for a known tree' '
-	cat >expected <<-\EOF &&
-	100644 blob f87290f8eb2cbbea7857214459a0739927eab154	path0
-	120000 blob 15a98433ae33114b085f3eb3bb03b832b3180a01	path0sym
-	040000 tree 58a09c23e2ca152193f2786e06986b7b6712bdbe	path2
-	100644 blob 3feff949ed00a62d9f7af97c15cd8a30595e7ac7	path2/file2
-	120000 blob d8ce161addc5173867a3c3c730924388daedbc38	path2/file2sym
-	040000 tree 21ae8269cacbe57ae09138dcc3a2887f904d02b3	path3
-	100644 blob 0aa34cae68d0878578ad119c86ca2b5ed5b28376	path3/file3
-	120000 blob 8599103969b43aff7e430efea79ca4636466794f	path3/file3sym
-	040000 tree 3c5e5399f3a333eddecce7a9b9465b63f65f51e2	path3/subp3
-	100644 blob 00fb5908cb97c2564a9783c0c64087333b3b464f	path3/subp3/file3
-	120000 blob 6649a1ebe9e9f1c553b66f5a6e74136a07ccc57c	path3/subp3/file3sym
+test_expect_success 'git ls-tree -r output for a known tree' '
+	cat >expected <<-EOF &&
+	100644 blob $(test_oid path0f)	path0
+	120000 blob $(test_oid path0s)	path0sym
+	040000 tree $(test_oid path2d)	path2
+	100644 blob $(test_oid path2f)	path2/file2
+	120000 blob $(test_oid path2s)	path2/file2sym
+	040000 tree $(test_oid path3d)	path3
+	100644 blob $(test_oid path3f)	path3/file3
+	120000 blob $(test_oid path3s)	path3/file3sym
+	040000 tree $(test_oid subp3d)	path3/subp3
+	100644 blob $(test_oid subp3f)	path3/subp3/file3
+	120000 blob $(test_oid subp3s)	path3/subp3/file3sym
 	EOF
 	test_cmp expected current
 '
@@ -999,16 +1040,16 @@ test_expect_success 'writing partial tree out with git write-tree --prefix' '
 	ptree=$(git write-tree --prefix=path3)
 '
 
-test_expect_success SHA1 'validate object ID for a known tree' '
-	test "$ptree" = 21ae8269cacbe57ae09138dcc3a2887f904d02b3
+test_expect_success 'validate object ID for a known tree' '
+	test "$ptree" = $(test_oid path3d)
 '
 
 test_expect_success 'writing partial tree out with git write-tree --prefix' '
 	ptree=$(git write-tree --prefix=path3/subp3)
 '
 
-test_expect_success SHA1 'validate object ID for a known tree' '
-	test "$ptree" = 3c5e5399f3a333eddecce7a9b9465b63f65f51e2
+test_expect_success 'validate object ID for a known tree' '
+	test "$ptree" = $(test_oid subp3d)
 '
 
 test_expect_success 'put invalid objects into the index' '
@@ -1042,16 +1083,16 @@ test_expect_success 'git read-tree followed by write-tree should be idempotent'
 	test "$newtree" = "$tree"
 '
 
-test_expect_success SHA1 'validate git diff-files output for a know cache/work tree state' '
-	cat >expected <<\EOF &&
-:100644 100644 f87290f8eb2cbbea7857214459a0739927eab154 0000000000000000000000000000000000000000 M	path0
-:120000 120000 15a98433ae33114b085f3eb3bb03b832b3180a01 0000000000000000000000000000000000000000 M	path0sym
-:100644 100644 3feff949ed00a62d9f7af97c15cd8a30595e7ac7 0000000000000000000000000000000000000000 M	path2/file2
-:120000 120000 d8ce161addc5173867a3c3c730924388daedbc38 0000000000000000000000000000000000000000 M	path2/file2sym
-:100644 100644 0aa34cae68d0878578ad119c86ca2b5ed5b28376 0000000000000000000000000000000000000000 M	path3/file3
-:120000 120000 8599103969b43aff7e430efea79ca4636466794f 0000000000000000000000000000000000000000 M	path3/file3sym
-:100644 100644 00fb5908cb97c2564a9783c0c64087333b3b464f 0000000000000000000000000000000000000000 M	path3/subp3/file3
-:120000 120000 6649a1ebe9e9f1c553b66f5a6e74136a07ccc57c 0000000000000000000000000000000000000000 M	path3/subp3/file3sym
+test_expect_success 'validate git diff-files output for a know cache/work tree state' '
+	cat >expected <<EOF &&
+:100644 100644 $(test_oid path0f) $ZERO_OID M	path0
+:120000 120000 $(test_oid path0s) $ZERO_OID M	path0sym
+:100644 100644 $(test_oid path2f) $ZERO_OID M	path2/file2
+:120000 120000 $(test_oid path2s) $ZERO_OID M	path2/file2sym
+:100644 100644 $(test_oid path3f) $ZERO_OID M	path3/file3
+:120000 120000 $(test_oid path3s) $ZERO_OID M	path3/file3sym
+:100644 100644 $(test_oid subp3f) $ZERO_OID M	path3/subp3/file3
+:120000 120000 $(test_oid subp3s) $ZERO_OID M	path3/subp3/file3sym
 EOF
 	git diff-files >current &&
 	test_cmp current expected
@@ -1067,23 +1108,23 @@ test_expect_success 'no diff after checkout and git update-index --refresh' '
 '
 
 ################################################################
-P=087704a96baf1c2d1c869a8b084481e121c88b5b
+P=$(test_oid root)
 
-test_expect_success SHA1 'git commit-tree records the correct tree in a commit' '
+test_expect_success 'git commit-tree records the correct tree in a commit' '
 	commit0=$(echo NO | git commit-tree $P) &&
 	tree=$(git show --pretty=raw $commit0 |
 		 sed -n -e "s/^tree //p" -e "/^author /q") &&
 	test "z$tree" = "z$P"
 '
 
-test_expect_success SHA1 'git commit-tree records the correct parent in a commit' '
+test_expect_success 'git commit-tree records the correct parent in a commit' '
 	commit1=$(echo NO | git commit-tree $P -p $commit0) &&
 	parent=$(git show --pretty=raw $commit1 |
 		sed -n -e "s/^parent //p" -e "/^author /q") &&
 	test "z$commit0" = "z$parent"
 '
 
-test_expect_success SHA1 'git commit-tree omits duplicated parent in a commit' '
+test_expect_success 'git commit-tree omits duplicated parent in a commit' '
 	commit2=$(echo NO | git commit-tree $P -p $commit0 -p $commit0) &&
 	     parent=$(git show --pretty=raw $commit2 |
 		sed -n -e "s/^parent //p" -e "/^author /q" |

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 04/11] t0002: abstract away SHA-1 specific constants
  2018-08-29  0:56 [PATCH v3 00/11] Hash-independent tests (part 3) brian m. carlson
                   ` (2 preceding siblings ...)
  2018-08-29  0:56 ` [PATCH v3 03/11] t0000: update tests for SHA-256 brian m. carlson
@ 2018-08-29  0:56 ` brian m. carlson
  2018-08-29  0:56 ` [PATCH v3 05/11] t0027: make hash size independent brian m. carlson
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: brian m. carlson @ 2018-08-29  0:56 UTC (permalink / raw)
  To: git
  Cc: Jeff King, Eric Sunshine, Nguyễn Thái Ngọc Duy,
	Torsten Bögershausen

Adjust the test so that it computes variables for object IDs instead of
using hard-coded hashes.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t0002-gitfile.sh | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/t/t0002-gitfile.sh b/t/t0002-gitfile.sh
index 3691023d51..0aa9908ea1 100755
--- a/t/t0002-gitfile.sh
+++ b/t/t0002-gitfile.sh
@@ -92,11 +92,12 @@ test_expect_success 'enter_repo non-strict mode' '
 		mv .git .realgit &&
 		echo "gitdir: .realgit" >.git
 	) &&
+	head=$(git -C enter_repo rev-parse HEAD) &&
 	git ls-remote enter_repo >actual &&
-	cat >expected <<-\EOF &&
-	946e985ab20de757ca5b872b16d64e92ff3803a9	HEAD
-	946e985ab20de757ca5b872b16d64e92ff3803a9	refs/heads/master
-	946e985ab20de757ca5b872b16d64e92ff3803a9	refs/tags/foo
+	cat >expected <<-EOF &&
+	$head	HEAD
+	$head	refs/heads/master
+	$head	refs/tags/foo
 	EOF
 	test_cmp expected actual
 '
@@ -106,21 +107,23 @@ test_expect_success 'enter_repo linked checkout' '
 		cd enter_repo &&
 		git worktree add  ../foo refs/tags/foo
 	) &&
+	head=$(git -C enter_repo rev-parse HEAD) &&
 	git ls-remote foo >actual &&
-	cat >expected <<-\EOF &&
-	946e985ab20de757ca5b872b16d64e92ff3803a9	HEAD
-	946e985ab20de757ca5b872b16d64e92ff3803a9	refs/heads/master
-	946e985ab20de757ca5b872b16d64e92ff3803a9	refs/tags/foo
+	cat >expected <<-EOF &&
+	$head	HEAD
+	$head	refs/heads/master
+	$head	refs/tags/foo
 	EOF
 	test_cmp expected actual
 '
 
 test_expect_success 'enter_repo strict mode' '
+	head=$(git -C enter_repo rev-parse HEAD) &&
 	git ls-remote --upload-pack="git upload-pack --strict" foo/.git >actual &&
-	cat >expected <<-\EOF &&
-	946e985ab20de757ca5b872b16d64e92ff3803a9	HEAD
-	946e985ab20de757ca5b872b16d64e92ff3803a9	refs/heads/master
-	946e985ab20de757ca5b872b16d64e92ff3803a9	refs/tags/foo
+	cat >expected <<-EOF &&
+	$head	HEAD
+	$head	refs/heads/master
+	$head	refs/tags/foo
 	EOF
 	test_cmp expected actual
 '

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 05/11] t0027: make hash size independent
  2018-08-29  0:56 [PATCH v3 00/11] Hash-independent tests (part 3) brian m. carlson
                   ` (3 preceding siblings ...)
  2018-08-29  0:56 ` [PATCH v3 04/11] t0002: abstract away SHA-1 specific constants brian m. carlson
@ 2018-08-29  0:56 ` brian m. carlson
  2018-08-31 18:21   ` Torsten Bögershausen
  2018-08-29  0:56 ` [PATCH v3 06/11] t0064: " brian m. carlson
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 19+ messages in thread
From: brian m. carlson @ 2018-08-29  0:56 UTC (permalink / raw)
  To: git
  Cc: Jeff King, Eric Sunshine, Nguyễn Thái Ngọc Duy,
	Torsten Bögershausen

We transform various object IDs into all-zero object IDs for comparison.
Adjust the length as well so that this works for all hash algorithms.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t0027-auto-crlf.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
index beb5927f77..0f1235d9d1 100755
--- a/t/t0027-auto-crlf.sh
+++ b/t/t0027-auto-crlf.sh
@@ -14,11 +14,13 @@ compare_files () {
 compare_ws_file () {
 	pfx=$1
 	exp=$2.expect
+	tmp=$2.tmp
 	act=$pfx.actual.$3
-	tr '\015\000abcdef0123456789' QN00000000000000000 <"$2" >"$exp" &&
+	tr '\015\000abcdef0123456789' QN00000000000000000 <"$2" >"$tmp" &&
 	tr '\015\000abcdef0123456789' QN00000000000000000 <"$3" >"$act" &&
+	sed -e "s/0000*/$ZERO_OID/" "$tmp" >"$exp" &&
 	test_cmp "$exp" "$act" &&
-	rm "$exp" "$act"
+	rm "$exp" "$act" "$tmp"
 }
 
 create_gitattributes () {

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 06/11] t0064: make hash size independent
  2018-08-29  0:56 [PATCH v3 00/11] Hash-independent tests (part 3) brian m. carlson
                   ` (4 preceding siblings ...)
  2018-08-29  0:56 ` [PATCH v3 05/11] t0027: make hash size independent brian m. carlson
@ 2018-08-29  0:56 ` brian m. carlson
  2018-08-29  0:56 ` [PATCH v3 07/11] t1006: " brian m. carlson
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: brian m. carlson @ 2018-08-29  0:56 UTC (permalink / raw)
  To: git
  Cc: Jeff King, Eric Sunshine, Nguyễn Thái Ngọc Duy,
	Torsten Bögershausen

Compute test values of the appropriate size instead of hard-coding
40-character values.  Rename the echo20 function to echoid, since the
values may be of varying sizes.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t0064-sha1-array.sh | 49 ++++++++++++++++++++++++-------------------
 1 file changed, 27 insertions(+), 22 deletions(-)

diff --git a/t/t0064-sha1-array.sh b/t/t0064-sha1-array.sh
index 67484502a0..5dda570b9a 100755
--- a/t/t0064-sha1-array.sh
+++ b/t/t0064-sha1-array.sh
@@ -3,30 +3,30 @@
 test_description='basic tests for the SHA1 array implementation'
 . ./test-lib.sh
 
-echo20 () {
+echoid () {
 	prefix="${1:+$1 }"
 	shift
 	while test $# -gt 0
 	do
-		echo "$prefix$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1$1"
+		echo "$prefix$ZERO_OID" | sed -e "s/00/$1/g"
 		shift
 	done
 }
 
 test_expect_success 'ordered enumeration' '
-	echo20 "" 44 55 88 aa >expect &&
+	echoid "" 44 55 88 aa >expect &&
 	{
-		echo20 append 88 44 aa 55 &&
+		echoid append 88 44 aa 55 &&
 		echo for_each_unique
 	} | test-tool sha1-array >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'ordered enumeration with duplicate suppression' '
-	echo20 "" 44 55 88 aa >expect &&
+	echoid "" 44 55 88 aa >expect &&
 	{
-		echo20 append 88 44 aa 55 &&
-		echo20 append 88 44 aa 55 &&
+		echoid append 88 44 aa 55 &&
+		echoid append 88 44 aa 55 &&
 		echo for_each_unique
 	} | test-tool sha1-array >actual &&
 	test_cmp expect actual
@@ -34,8 +34,8 @@ test_expect_success 'ordered enumeration with duplicate suppression' '
 
 test_expect_success 'lookup' '
 	{
-		echo20 append 88 44 aa 55 &&
-		echo20 lookup 55
+		echoid append 88 44 aa 55 &&
+		echoid lookup 55
 	} | test-tool sha1-array >actual &&
 	n=$(cat actual) &&
 	test "$n" -eq 1
@@ -43,8 +43,8 @@ test_expect_success 'lookup' '
 
 test_expect_success 'lookup non-existing entry' '
 	{
-		echo20 append 88 44 aa 55 &&
-		echo20 lookup 33
+		echoid append 88 44 aa 55 &&
+		echoid lookup 33
 	} | test-tool sha1-array >actual &&
 	n=$(cat actual) &&
 	test "$n" -lt 0
@@ -52,9 +52,9 @@ test_expect_success 'lookup non-existing entry' '
 
 test_expect_success 'lookup with duplicates' '
 	{
-		echo20 append 88 44 aa 55 &&
-		echo20 append 88 44 aa 55 &&
-		echo20 lookup 55
+		echoid append 88 44 aa 55 &&
+		echoid append 88 44 aa 55 &&
+		echoid lookup 55
 	} | test-tool sha1-array >actual &&
 	n=$(cat actual) &&
 	test "$n" -ge 2 &&
@@ -63,19 +63,24 @@ test_expect_success 'lookup with duplicates' '
 
 test_expect_success 'lookup non-existing entry with duplicates' '
 	{
-		echo20 append 88 44 aa 55 &&
-		echo20 append 88 44 aa 55 &&
-		echo20 lookup 66
+		echoid append 88 44 aa 55 &&
+		echoid append 88 44 aa 55 &&
+		echoid lookup 66
 	} | test-tool sha1-array >actual &&
 	n=$(cat actual) &&
 	test "$n" -lt 0
 '
 
 test_expect_success 'lookup with almost duplicate values' '
+	# n-1 5s
+	root=$(echoid "" 55) &&
+	root=${root%5} &&
 	{
-		echo "append 5555555555555555555555555555555555555555" &&
-		echo "append 555555555555555555555555555555555555555f" &&
-		echo20 lookup 55
+		id1="${root}5" &&
+		id2="${root}f" &&
+		echo "append $id1" &&
+		echo "append $id2" &&
+		echoid lookup 55
 	} | test-tool sha1-array >actual &&
 	n=$(cat actual) &&
 	test "$n" -eq 0
@@ -83,8 +88,8 @@ test_expect_success 'lookup with almost duplicate values' '
 
 test_expect_success 'lookup with single duplicate value' '
 	{
-		echo20 append 55 55 &&
-		echo20 lookup 55
+		echoid append 55 55 &&
+		echoid lookup 55
 	} | test-tool sha1-array >actual &&
 	n=$(cat actual) &&
 	test "$n" -ge 0 &&

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 07/11] t1006: make hash size independent
  2018-08-29  0:56 [PATCH v3 00/11] Hash-independent tests (part 3) brian m. carlson
                   ` (5 preceding siblings ...)
  2018-08-29  0:56 ` [PATCH v3 06/11] t0064: " brian m. carlson
@ 2018-08-29  0:56 ` brian m. carlson
  2018-08-29  0:56 ` [PATCH v3 08/11] t1400: switch hard-coded object ID to variable brian m. carlson
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: brian m. carlson @ 2018-08-29  0:56 UTC (permalink / raw)
  To: git
  Cc: Jeff King, Eric Sunshine, Nguyễn Thái Ngọc Duy,
	Torsten Bögershausen

Compute the size of the tree and commit objects we're creating by
checking for the size of an object ID and computing the resulting sizes
accordingly.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t1006-cat-file.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index 7f19d591f2..a7c95bb785 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -140,15 +140,17 @@ test_expect_success '--batch-check without %(rest) considers whole line' '
 	test_cmp expect actual
 '
 
+test_oid_init
+
 tree_sha1=$(git write-tree)
-tree_size=33
+tree_size=$(($(test_oid rawsz) + 13))
 tree_pretty_content="100644 blob $hello_sha1	hello"
 
 run_tests 'tree' $tree_sha1 $tree_size "" "$tree_pretty_content"
 
 commit_message="Initial commit"
 commit_sha1=$(echo_without_newline "$commit_message" | git commit-tree $tree_sha1)
-commit_size=177
+commit_size=$(($(test_oid hexsz) + 137))
 commit_content="tree $tree_sha1
 author $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> 0000000000 +0000
 committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 0000000000 +0000

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 08/11] t1400: switch hard-coded object ID to variable
  2018-08-29  0:56 [PATCH v3 00/11] Hash-independent tests (part 3) brian m. carlson
                   ` (6 preceding siblings ...)
  2018-08-29  0:56 ` [PATCH v3 07/11] t1006: " brian m. carlson
@ 2018-08-29  0:56 ` brian m. carlson
  2018-08-29  0:56 ` [PATCH v3 09/11] t1405: make hash size independent brian m. carlson
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 19+ messages in thread
From: brian m. carlson @ 2018-08-29  0:56 UTC (permalink / raw)
  To: git
  Cc: Jeff King, Eric Sunshine, Nguyễn Thái Ngọc Duy,
	Torsten Bögershausen

Switch a hard-coded all-zeros object ID to use a variable instead.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t1400-update-ref.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 7c8df20955..6072650686 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -346,7 +346,7 @@ test_expect_success "verifying $m's log (logged by config)" '
 
 git update-ref $m $D
 cat >.git/logs/$m <<EOF
-0000000000000000000000000000000000000000 $C $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150320 -0500
+$Z $C $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150320 -0500
 $C $A $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150350 -0500
 $A $B $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150380 -0500
 $F $Z $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150680 -0500

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 09/11] t1405: make hash size independent
  2018-08-29  0:56 [PATCH v3 00/11] Hash-independent tests (part 3) brian m. carlson
                   ` (7 preceding siblings ...)
  2018-08-29  0:56 ` [PATCH v3 08/11] t1400: switch hard-coded object ID to variable brian m. carlson
@ 2018-08-29  0:56 ` brian m. carlson
  2018-08-29  0:56 ` [PATCH v3 10/11] t1406: make hash-size independent brian m. carlson
  2018-08-29  0:56 ` [PATCH v3 11/11] t1407: make hash size independent brian m. carlson
  10 siblings, 0 replies; 19+ messages in thread
From: brian m. carlson @ 2018-08-29  0:56 UTC (permalink / raw)
  To: git
  Cc: Jeff King, Eric Sunshine, Nguyễn Thái Ngọc Duy,
	Torsten Bögershausen

Instead of hard-coding a 40-based constant, split the output of
for-each-ref and for-each-reflog by field.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t1405-main-ref-store.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t1405-main-ref-store.sh b/t/t1405-main-ref-store.sh
index a74c38b5fb..331899ddc4 100755
--- a/t/t1405-main-ref-store.sh
+++ b/t/t1405-main-ref-store.sh
@@ -54,7 +54,7 @@ test_expect_success 'for_each_ref(refs/heads/)' '
 '
 
 test_expect_success 'for_each_ref() is sorted' '
-	$RUN for-each-ref refs/heads/ | cut -c 42- >actual &&
+	$RUN for-each-ref refs/heads/ | cut -d" " -f 2- >actual &&
 	sort actual > expected &&
 	test_cmp expected actual
 '
@@ -71,7 +71,7 @@ test_expect_success 'verify_ref(new-master)' '
 '
 
 test_expect_success 'for_each_reflog()' '
-	$RUN for-each-reflog | sort -k2 | cut -c 42- >actual &&
+	$RUN for-each-reflog | sort -k2 | cut -d" " -f 2- >actual &&
 	cat >expected <<-\EOF &&
 	HEAD 0x1
 	refs/heads/master 0x0

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 10/11] t1406: make hash-size independent
  2018-08-29  0:56 [PATCH v3 00/11] Hash-independent tests (part 3) brian m. carlson
                   ` (8 preceding siblings ...)
  2018-08-29  0:56 ` [PATCH v3 09/11] t1405: make hash size independent brian m. carlson
@ 2018-08-29  0:56 ` brian m. carlson
  2018-08-29  0:56 ` [PATCH v3 11/11] t1407: make hash size independent brian m. carlson
  10 siblings, 0 replies; 19+ messages in thread
From: brian m. carlson @ 2018-08-29  0:56 UTC (permalink / raw)
  To: git
  Cc: Jeff King, Eric Sunshine, Nguyễn Thái Ngọc Duy,
	Torsten Bögershausen

Instead of hard-coding a 40-based constant, split the output of
for-each-ref and for-each-reflog by field.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t1406-submodule-ref-store.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t1406-submodule-ref-store.sh b/t/t1406-submodule-ref-store.sh
index e093782cc3..d199d872fb 100755
--- a/t/t1406-submodule-ref-store.sh
+++ b/t/t1406-submodule-ref-store.sh
@@ -39,7 +39,7 @@ test_expect_success 'rename_refs() not allowed' '
 '
 
 test_expect_success 'for_each_ref(refs/heads/)' '
-	$RUN for-each-ref refs/heads/ | cut -c 42- >actual &&
+	$RUN for-each-ref refs/heads/ | cut -d" " -f 2- >actual &&
 	cat >expected <<-\EOF &&
 	master 0x0
 	new-master 0x0
@@ -48,7 +48,7 @@ test_expect_success 'for_each_ref(refs/heads/)' '
 '
 
 test_expect_success 'for_each_ref() is sorted' '
-	$RUN for-each-ref refs/heads/ | cut -c 42- >actual &&
+	$RUN for-each-ref refs/heads/ | cut -d" " -f 2- >actual &&
 	sort actual > expected &&
 	test_cmp expected actual
 '
@@ -65,7 +65,7 @@ test_expect_success 'verify_ref(new-master)' '
 '
 
 test_expect_success 'for_each_reflog()' '
-	$RUN for-each-reflog | sort | cut -c 42- >actual &&
+	$RUN for-each-reflog | sort | cut -d" " -f 2- >actual &&
 	cat >expected <<-\EOF &&
 	HEAD 0x1
 	refs/heads/master 0x0

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 11/11] t1407: make hash size independent
  2018-08-29  0:56 [PATCH v3 00/11] Hash-independent tests (part 3) brian m. carlson
                   ` (9 preceding siblings ...)
  2018-08-29  0:56 ` [PATCH v3 10/11] t1406: make hash-size independent brian m. carlson
@ 2018-08-29  0:56 ` brian m. carlson
  10 siblings, 0 replies; 19+ messages in thread
From: brian m. carlson @ 2018-08-29  0:56 UTC (permalink / raw)
  To: git
  Cc: Jeff King, Eric Sunshine, Nguyễn Thái Ngọc Duy,
	Torsten Bögershausen

Instead of hard-coding a 40-based constant, split the output of
for-each-ref and for-each-reflog by field.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/t1407-worktree-ref-store.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t1407-worktree-ref-store.sh b/t/t1407-worktree-ref-store.sh
index 4623ae15c4..9a84858118 100755
--- a/t/t1407-worktree-ref-store.sh
+++ b/t/t1407-worktree-ref-store.sh
@@ -58,7 +58,7 @@ test_expect_success 'for_each_reflog()' '
 	mkdir -p     .git/worktrees/wt/logs/refs/bisect &&
 	echo $ZERO_OID > .git/worktrees/wt/logs/refs/bisect/wt-random &&
 
-	$RWT for-each-reflog | cut -c 42- | sort >actual &&
+	$RWT for-each-reflog | cut -d" " -f 2- | sort >actual &&
 	cat >expected <<-\EOF &&
 	HEAD 0x1
 	PSEUDO-WT 0x0
@@ -68,7 +68,7 @@ test_expect_success 'for_each_reflog()' '
 	EOF
 	test_cmp expected actual &&
 
-	$RMAIN for-each-reflog | cut -c 42- | sort >actual &&
+	$RMAIN for-each-reflog | cut -d" " -f 2- | sort >actual &&
 	cat >expected <<-\EOF &&
 	HEAD 0x1
 	PSEUDO-MAIN 0x0

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 01/11] t: add tool to translate hash-related values
  2018-08-29  0:56 ` [PATCH v3 01/11] t: add tool to translate hash-related values brian m. carlson
@ 2018-08-29  4:05   ` Jonathan Nieder
  2018-08-29 23:22     ` brian m. carlson
  2018-08-29 12:37   ` Derrick Stolee
  1 sibling, 1 reply; 19+ messages in thread
From: Jonathan Nieder @ 2018-08-29  4:05 UTC (permalink / raw)
  To: brian m. carlson
  Cc: git, Jeff King, Eric Sunshine,
	Nguyễn Thái Ngọc Duy, Torsten Bögershausen

Hi,

brian m. carlson wrote:

> Add a test function helper, test_oid, that produces output that varies
> depending on the hash in use.

Cool!

>                                Add two additional helpers,
> test_oid_cache, which can be used to load data for test_oid from
> standard input, and test_oid_init, which can be used to load certain
> fixed values from lookup charts.  Check that these functions work in
> t0000, as the rest of the testsuite will soon come to depend on them.
>
> Implement two basic lookup charts, one for common invalid or synthesized
> object IDs, and one for various facts about the hash function in use.
> Provide versions for both SHA-1 and SHA-256.

What do test_oid_cache and test_oid_init do?  How can I use them?

Judging from t0000-basic.sh, the idea looks something like

	Add a test function helper, test_oid, that ...

	test_oid allows looking up arbitrary information about an object format:
	the length of object ids, values of well known object ids, etc.  Before
	calling it, a test script must invoke test_oid_cache (either directly
	or indirectly through test_oid_init) to load the lookup charts.

	See t0000 for an example, which also serves as a sanity-check that
	these functions work in preparation for using them in the rest of the
	test suite.

	There are two basic lookup charts for now: oid-info/oid, with common
	invalid or synthesized object IDs; and oid-info/hash-info, with facts
	such as object id length about the formats in use.  The charts contain
	information about both SHA-1 and SHA-256.

	So now you can update existing tests to be format-independent by (1)
	adding an invocation of test_oid_init to test setup and (2) replacing
	format dependencies with $(test_oid foo).

	Since values are stored as shell variables, names used for lookup can
	only consist of shell identifier characters.  If this is a problem in
	the future, we can hash the names before use.

	Improved-by: Eric Sunshine <sunshine@sunshineco.com>

Do these always use sha1 for now?  Ah, t0000 answers but it might be
worth mentioning in the commit message, too:

	test_set_hash allows setting which object format test_oid should look
	up information for, and test_detect_hash returns to the default format.

[...]
> --- /dev/null
> +++ b/t/oid-info/hash-info
> @@ -0,0 +1,8 @@
> +rawsz sha1:20
> +rawsz sha256:32

Can there be a README in this directory describing the files and format?

[...]
> --- a/t/t0000-basic.sh
> +++ b/t/t0000-basic.sh
> @@ -821,6 +821,41 @@ test_expect_success 'tests clean up even on failures' "
>  	EOF
>  "
>  
> +test_oid_init

Can this be wrapped in test_expect_success?  That way, if it fails or
prints an error message then the usual test machinery would handle it.

> +
> +test_expect_success 'test_oid provides sane info by default' '
> +	test_oid zero >actual &&
> +	grep "^00*$" actual &&

nit: can save the reader some confusion by escaping the $.

> +	rawsz="$(test_oid rawsz)" &&
> +	hexsz="$(test_oid hexsz)" &&

optional: no need for these quotation marks --- a command substitution
assigned to a shell variable is treated as if it were quoted.

> +	test "$hexsz" -eq $(wc -c <actual) &&
> +	test $(( $rawsz * 2)) -eq "$hexsz"

Makes sense.

[...]
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -1155,3 +1155,47 @@ depacketize () {
[...]
> +test_oid_cache () {
> +	test -n "$test_hash_algo" || test_detect_hash

Should this use an uninterrupted &&-chain?

> +	while read _tag _rest

This appears to be the first use of this naming convention.  I wonder
if we can use "local" instead.

> +	do
> +		case $_tag in
> +		\#*)
> +			continue;;
> +		?*)
> +			# non-empty
> +			;;
> +		*)
> +			# blank line
> +			continue;;
> +

unnecessary blank line here

> +		esac &&
> +
> +		_k="${_rest%:*}" &&
> +		_v="${_rest#*:}" &&
> +		{ echo "$_k" | egrep '^[a-z0-9]+$' >/dev/null ||
> +			error 'bug in the test script: bad hash algorithm'; } &&
> +		eval "test_oid_${_k}_$_tag=\"\$_v\"" || return 1

This is dense, so I'm having trouble taking it in at a glance.

I think the idea is

		key=${rest%%:*} &&
		val=${rest#*:} &&

		if ! expr "$key" : '[a-z0-9]*$' >/dev/null
		then
			error ...
		fi &&
		eval "test_oid_${key}_${tag}=\${val}"

> +	done
> +}
> +
> +test_oid () {
> +	eval "
> +		test -n \"\${test_oid_${test_hash_algo}_$1+set}\" &&
> +		printf '%s' \"\${test_oid_${test_hash_algo}_$1}\"
> +	"

I'm also having trouble taking this one in.  Maybe splitting into two
evals would work?

	var=test_oid_${test_hash_algo}_$1 &&

	eval "test -n \"\${$var+set}\"" &&
	eval "printf '%s\n' \"\${$var}\""

What is the initial test meant to do?  Can this function get a
documentation comment?  Are we relying on "test -n" to return a failing
result if the variable is unset, or could the test be omitted (relying
on "\${$var}" to evaluate to "" when the variable is unset)?  Should
this call 'error' when the variable is unset?

Can t/README describe the new test helpers?

Thanks,
Jonathan

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 01/11] t: add tool to translate hash-related values
  2018-08-29  0:56 ` [PATCH v3 01/11] t: add tool to translate hash-related values brian m. carlson
  2018-08-29  4:05   ` Jonathan Nieder
@ 2018-08-29 12:37   ` Derrick Stolee
  2018-08-29 22:52     ` brian m. carlson
  1 sibling, 1 reply; 19+ messages in thread
From: Derrick Stolee @ 2018-08-29 12:37 UTC (permalink / raw)
  To: brian m. carlson, git
  Cc: Jeff King, Eric Sunshine, Nguyễn Thái Ngọc Duy,
	Torsten Bögershausen

On 8/28/2018 8:56 PM, brian m. carlson wrote:
> +	rawsz="$(test_oid rawsz)" &&
> +	hexsz="$(test_oid hexsz)" &&

These are neat helpers! The 'git commit-graph verify' tests in 
t5318-commit-graph.sh should automatically work if we use these for 
HASH_LEN instead of 20. I'll use a similar pattern when writing 'git 
multi-pack-index verify'.

Thanks,

-Stolee


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 01/11] t: add tool to translate hash-related values
  2018-08-29 12:37   ` Derrick Stolee
@ 2018-08-29 22:52     ` brian m. carlson
  0 siblings, 0 replies; 19+ messages in thread
From: brian m. carlson @ 2018-08-29 22:52 UTC (permalink / raw)
  To: Derrick Stolee
  Cc: git, Jeff King, Eric Sunshine,
	Nguyễn Thái Ngọc Duy, Torsten Bögershausen

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

On Wed, Aug 29, 2018 at 08:37:48AM -0400, Derrick Stolee wrote:
> On 8/28/2018 8:56 PM, brian m. carlson wrote:
> > +	rawsz="$(test_oid rawsz)" &&
> > +	hexsz="$(test_oid hexsz)" &&
> 
> These are neat helpers! The 'git commit-graph verify' tests in
> t5318-commit-graph.sh should automatically work if we use these for HASH_LEN
> instead of 20. I'll use a similar pattern when writing 'git multi-pack-index
> verify'.

Thanks.  In the future, test_oid will learn about internal versus
external object names, since we'll have what's printed by the UI (which
might be SHA-1) and what's under the hood (which might be SHA-256).
However, it should be easy enough to update all those pplaces, so if you
use them now, it should be easy enough to update them in them in the
future to refer to the right thing.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 01/11] t: add tool to translate hash-related values
  2018-08-29  4:05   ` Jonathan Nieder
@ 2018-08-29 23:22     ` brian m. carlson
  0 siblings, 0 replies; 19+ messages in thread
From: brian m. carlson @ 2018-08-29 23:22 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: git, Jeff King, Eric Sunshine,
	Nguyễn Thái Ngọc Duy, Torsten Bögershausen

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

On Tue, Aug 28, 2018 at 09:05:48PM -0700, Jonathan Nieder wrote:
> >                                Add two additional helpers,
> > test_oid_cache, which can be used to load data for test_oid from
> > standard input, and test_oid_init, which can be used to load certain
> > fixed values from lookup charts.  Check that these functions work in
> > t0000, as the rest of the testsuite will soon come to depend on them.
> >
> > Implement two basic lookup charts, one for common invalid or synthesized
> > object IDs, and one for various facts about the hash function in use.
> > Provide versions for both SHA-1 and SHA-256.
> 
> What do test_oid_cache and test_oid_init do?  How can I use them?
> 
> Judging from t0000-basic.sh, the idea looks something like
> 
> 	Add a test function helper, test_oid, that ...
> 
> 	test_oid allows looking up arbitrary information about an object format:
> 	the length of object ids, values of well known object ids, etc.  Before
> 	calling it, a test script must invoke test_oid_cache (either directly
> 	or indirectly through test_oid_init) to load the lookup charts.
> 
> 	See t0000 for an example, which also serves as a sanity-check that
> 	these functions work in preparation for using them in the rest of the
> 	test suite.
> 
> 	There are two basic lookup charts for now: oid-info/oid, with common
> 	invalid or synthesized object IDs; and oid-info/hash-info, with facts
> 	such as object id length about the formats in use.  The charts contain
> 	information about both SHA-1 and SHA-256.
> 
> 	So now you can update existing tests to be format-independent by (1)
> 	adding an invocation of test_oid_init to test setup and (2) replacing
> 	format dependencies with $(test_oid foo).
> 
> 	Since values are stored as shell variables, names used for lookup can
> 	only consist of shell identifier characters.  If this is a problem in
> 	the future, we can hash the names before use.
> 
> 	Improved-by: Eric Sunshine <sunshine@sunshineco.com>
> 
> Do these always use sha1 for now?  Ah, t0000 answers but it might be
> worth mentioning in the commit message, too:
> 
> 	test_set_hash allows setting which object format test_oid should look
> 	up information for, and test_detect_hash returns to the default format.

I'll expand the commit message.  They do use SHA-1 for now, but I have a
branch that makes them use SHA-256 instead.

> [...]
> > --- /dev/null
> > +++ b/t/oid-info/hash-info
> > @@ -0,0 +1,8 @@
> > +rawsz sha1:20
> > +rawsz sha256:32
> 
> Can there be a README in this directory describing the files and format?

Sure, if you like.

> [...]
> > --- a/t/t0000-basic.sh
> > +++ b/t/t0000-basic.sh
> > @@ -821,6 +821,41 @@ test_expect_success 'tests clean up even on failures' "
> >  	EOF
> >  "
> >  
> > +test_oid_init
> 
> Can this be wrapped in test_expect_success?  That way, if it fails or
> prints an error message then the usual test machinery would handle it.

Sure.

> > +
> > +test_expect_success 'test_oid provides sane info by default' '
> > +	test_oid zero >actual &&
> > +	grep "^00*$" actual &&
> 
> nit: can save the reader some confusion by escaping the $.

Good point.

> > +	rawsz="$(test_oid rawsz)" &&
> > +	hexsz="$(test_oid hexsz)" &&
> 
> optional: no need for these quotation marks --- a command substitution
> assigned to a shell variable is treated as if it were quoted.

That's good to know.  I will honestly say that looking through the Git
codebase and getting reviews on the list has taught me huge amounts
about the intricacies of shell.

> [...]
> > --- a/t/test-lib-functions.sh
> > +++ b/t/test-lib-functions.sh
> > @@ -1155,3 +1155,47 @@ depacketize () {
> [...]
> > +test_oid_cache () {
> > +	test -n "$test_hash_algo" || test_detect_hash
> 
> Should this use an uninterrupted &&-chain?

Yes.  Will fix.

> > +	while read _tag _rest
> 
> This appears to be the first use of this naming convention.  I wonder
> if we can use "local" instead.

We probably can.  There was a discussion about this elsewhere, and we
determined that it's probably safe, and if it's not, it should be
relatively easy to back out.

> > +		esac &&
> > +
> > +		_k="${_rest%:*}" &&
> > +		_v="${_rest#*:}" &&
> > +		{ echo "$_k" | egrep '^[a-z0-9]+$' >/dev/null ||
> > +			error 'bug in the test script: bad hash algorithm'; } &&
> > +		eval "test_oid_${_k}_$_tag=\"\$_v\"" || return 1
> 
> This is dense, so I'm having trouble taking it in at a glance.
> 
> I think the idea is
> 
> 		key=${rest%%:*} &&
> 		val=${rest#*:} &&
> 
> 		if ! expr "$key" : '[a-z0-9]*$' >/dev/null
> 		then
> 			error ...
> 		fi &&
> 		eval "test_oid_${key}_${tag}=\${val}"

Yes.  I take it that you think that's easier to read, so I'll rewrite it
that way.  I will admit a tendency to write code that is more compact,
sometimes (unintentionally) at the cost of readability.  Thanks for
providing a sanity check.

I agree that expr is probably better than the echo and egrep.

> > +	done
> > +}
> > +
> > +test_oid () {
> > +	eval "
> > +		test -n \"\${test_oid_${test_hash_algo}_$1+set}\" &&
> > +		printf '%s' \"\${test_oid_${test_hash_algo}_$1}\"
> > +	"
> 
> I'm also having trouble taking this one in.  Maybe splitting into two
> evals would work?
> 
> 	var=test_oid_${test_hash_algo}_$1 &&
> 
> 	eval "test -n \"\${$var+set}\"" &&
> 	eval "printf '%s\n' \"\${$var}\""
> 
> What is the initial test meant to do?  Can this function get a
> documentation comment?  Are we relying on "test -n" to return a failing
> result if the variable is unset, or could the test be omitted (relying
> on "\${$var}" to evaluate to "" when the variable is unset)?  Should
> this call 'error' when the variable is unset?

Yes.  The test -n will return false if the variable is unset, since
${$var+set} evaluates to nothing if the variable is unset and "set" if
it is set.  I will admit that I had to look this up in the shell
documentation, so I'm not surprised that this is confusing at first
glance.

Switching to error is probably better.

> Can t/README describe the new test helpers?

Sure.  I wasn't aware that there was one to add this to, but now that
you've pointed it out, it makes sense to add them.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 05/11] t0027: make hash size independent
  2018-08-29  0:56 ` [PATCH v3 05/11] t0027: make hash size independent brian m. carlson
@ 2018-08-31 18:21   ` Torsten Bögershausen
  2018-08-31 18:40     ` Eric Sunshine
  0 siblings, 1 reply; 19+ messages in thread
From: Torsten Bögershausen @ 2018-08-31 18:21 UTC (permalink / raw)
  To: brian m. carlson
  Cc: git, Jeff King, Eric Sunshine,
	Nguyễn Thái Ngọc Duy

On Wed, Aug 29, 2018 at 12:56:36AM +0000, brian m. carlson wrote:
> We transform various object IDs into all-zero object IDs for comparison.
> Adjust the length as well so that this works for all hash algorithms.
> 
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  t/t0027-auto-crlf.sh | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
> index beb5927f77..0f1235d9d1 100755
> --- a/t/t0027-auto-crlf.sh
> +++ b/t/t0027-auto-crlf.sh
> @@ -14,11 +14,13 @@ compare_files () {
>  compare_ws_file () {
>  	pfx=$1
>  	exp=$2.expect
> +	tmp=$2.tmp
>  	act=$pfx.actual.$3
> -	tr '\015\000abcdef0123456789' QN00000000000000000 <"$2" >"$exp" &&
> +	tr '\015\000abcdef0123456789' QN00000000000000000 <"$2" >"$tmp" &&
>  	tr '\015\000abcdef0123456789' QN00000000000000000 <"$3" >"$act" &&
> +	sed -e "s/0000*/$ZERO_OID/" "$tmp" >"$exp" &&
>  	test_cmp "$exp" "$act" &&
> -	rm "$exp" "$act"
> +	rm "$exp" "$act" "$tmp"
>  }
>  
>  create_gitattributes () {

I only managed to review the changes in t0027.
Out of interest: why do we use a "tmp" file here?
Would it make more sense  to chain the 'tr' with 'sed' and skip the
tmp file ?

 	tr '\015\000abcdef0123456789' QN00000000000000000 <"$2" |
	sed -e "s/0000*/$ZERO_OID/"  >"$exp" &&


Yes, we will loose the exit status of 'tr', I think.
How important is the exit status ?

I don't know, hopefully someone with more experience/knowledge
about shell scripting can help me out here.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 05/11] t0027: make hash size independent
  2018-08-31 18:21   ` Torsten Bögershausen
@ 2018-08-31 18:40     ` Eric Sunshine
  2018-09-01 15:33       ` brian m. carlson
  0 siblings, 1 reply; 19+ messages in thread
From: Eric Sunshine @ 2018-08-31 18:40 UTC (permalink / raw)
  To: Torsten Bögershausen
  Cc: brian m. carlson, Git List, Jeff King,
	Nguyễn Thái Ngọc Duy

On Fri, Aug 31, 2018 at 2:21 PM Torsten Bögershausen <tboegi@web.de> wrote:
> > diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh
> > @@ -14,11 +14,13 @@ compare_files () {
> >  compare_ws_file () {
> > +     tmp=$2.tmp
> >       act=$pfx.actual.$3
> > -     tr '\015\000abcdef0123456789' QN00000000000000000 <"$2" >"$exp" &&
> > +     tr '\015\000abcdef0123456789' QN00000000000000000 <"$2" >"$tmp" &&
> >       tr '\015\000abcdef0123456789' QN00000000000000000 <"$3" >"$act" &&
> > +     sed -e "s/0000*/$ZERO_OID/" "$tmp" >"$exp" &&
>
> Out of interest: why do we use a "tmp" file here?
> Would it make more sense  to chain the 'tr' with 'sed' and skip the
> tmp file ?
>
>         tr '\015\000abcdef0123456789' QN00000000000000000 <"$2" |
>         sed -e "s/0000*/$ZERO_OID/"  >"$exp" &&
>
> Yes, we will loose the exit status of 'tr', I think.
> How important is the exit status ?

As far as I understand, it is only Git commands for which we worry
about losing the exit status upstream in pipes. System utilities, on
the other hand, are presumed to be bug-free, thus we don't mind having
them upstream.

A different question is why does this need to run both 'tr' and 'sed'
when 'sed itself could do the entire job since 'sed' has 'tr'
functionality built in (see sed's "y" command)?

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v3 05/11] t0027: make hash size independent
  2018-08-31 18:40     ` Eric Sunshine
@ 2018-09-01 15:33       ` brian m. carlson
  0 siblings, 0 replies; 19+ messages in thread
From: brian m. carlson @ 2018-09-01 15:33 UTC (permalink / raw)
  To: Eric Sunshine
  Cc: Torsten Bögershausen, Git List, Jeff King,
	Nguyễn Thái Ngọc Duy

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

On Fri, Aug 31, 2018 at 02:40:12PM -0400, Eric Sunshine wrote:
> On Fri, Aug 31, 2018 at 2:21 PM Torsten Bögershausen <tboegi@web.de> wrote:
> > Out of interest: why do we use a "tmp" file here?
> > Would it make more sense  to chain the 'tr' with 'sed' and skip the
> > tmp file ?
> >
> >         tr '\015\000abcdef0123456789' QN00000000000000000 <"$2" |
> >         sed -e "s/0000*/$ZERO_OID/"  >"$exp" &&
> >
> > Yes, we will loose the exit status of 'tr', I think.
> > How important is the exit status ?
> 
> As far as I understand, it is only Git commands for which we worry
> about losing the exit status upstream in pipes. System utilities, on
> the other hand, are presumed to be bug-free, thus we don't mind having
> them upstream.

If that's the case, that's fine, and I can make that change.  I know
that we do often want to preserve the exit status of a system command,
but presumably the tr and sed here would exit 0, so I'm happy to assume
that for the test.

> A different question is why does this need to run both 'tr' and 'sed'
> when 'sed itself could do the entire job since 'sed' has 'tr'
> functionality built in (see sed's "y" command)?

It doesn't.  I went for a minimal change, but I could switch to using
both s/// and y/// in sed instead.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2018-09-01 15:33 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-29  0:56 [PATCH v3 00/11] Hash-independent tests (part 3) brian m. carlson
2018-08-29  0:56 ` [PATCH v3 01/11] t: add tool to translate hash-related values brian m. carlson
2018-08-29  4:05   ` Jonathan Nieder
2018-08-29 23:22     ` brian m. carlson
2018-08-29 12:37   ` Derrick Stolee
2018-08-29 22:52     ` brian m. carlson
2018-08-29  0:56 ` [PATCH v3 02/11] t0000: use hash translation table brian m. carlson
2018-08-29  0:56 ` [PATCH v3 03/11] t0000: update tests for SHA-256 brian m. carlson
2018-08-29  0:56 ` [PATCH v3 04/11] t0002: abstract away SHA-1 specific constants brian m. carlson
2018-08-29  0:56 ` [PATCH v3 05/11] t0027: make hash size independent brian m. carlson
2018-08-31 18:21   ` Torsten Bögershausen
2018-08-31 18:40     ` Eric Sunshine
2018-09-01 15:33       ` brian m. carlson
2018-08-29  0:56 ` [PATCH v3 06/11] t0064: " brian m. carlson
2018-08-29  0:56 ` [PATCH v3 07/11] t1006: " brian m. carlson
2018-08-29  0:56 ` [PATCH v3 08/11] t1400: switch hard-coded object ID to variable brian m. carlson
2018-08-29  0:56 ` [PATCH v3 09/11] t1405: make hash size independent brian m. carlson
2018-08-29  0:56 ` [PATCH v3 10/11] t1406: make hash-size independent brian m. carlson
2018-08-29  0:56 ` [PATCH v3 11/11] t1407: make hash size independent brian m. carlson

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).