[v2,07/24] t6300: abstract away SHA-1-specific constants
diff mbox series

Message ID 20200222201749.937983-8-sandals@crustytoothpaste.net
State New
Headers show
Series
  • SHA-256 stage 4 implementation, part 1/3
Related show

Commit Message

brian m. carlson Feb. 22, 2020, 8:17 p.m. UTC
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/t6300-for-each-ref.sh | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

Comments

Junio C Hamano Feb. 24, 2020, 6:01 p.m. UTC | #1
"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> 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/t6300-for-each-ref.sh | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
> index 9c910ce746..2406b93d35 100755
> --- a/t/t6300-for-each-ref.sh
> +++ b/t/t6300-for-each-ref.sh
> @@ -20,6 +20,10 @@ setdate_and_increment () {
>  }
>  
>  test_expect_success setup '
> +	test_oid_cache <<-EOF &&
> +	disklen sha1:138
> +	disklen sha256:154
> +	EOF
>  	setdate_and_increment &&
>  	echo "Using $datestamp" > one &&
>  	git add one &&
> @@ -50,6 +54,9 @@ test_atom() {
>  	"
>  }
>  
> +hexlen=$(test_oid hexsz)
> +disklen=$(test_oid disklen)
> +
>  test_atom head refname refs/heads/master
>  test_atom head refname: refs/heads/master
>  test_atom head refname:short master
> @@ -82,9 +89,9 @@ test_atom head push:rstrip=-1 refs
>  test_atom head push:strip=1 remotes/myfork/master
>  test_atom head push:strip=-1 master
>  test_atom head objecttype commit
> -test_atom head objectsize 171
> -test_atom head objectsize:disk 138
> -test_atom head deltabase 0000000000000000000000000000000000000000
> +test_atom head objectsize $((131 + hexlen))

171 == 131 + 40 and that is because we are looking at the initial
commit, whose contents has a single object name (i.e. its tree).

Makes sense.

> +test_atom head objectsize:disk $disklen
> +test_atom head deltabase $ZERO_OID
>  test_atom head objectname $(git rev-parse refs/heads/master)
>  test_atom head objectname:short $(git rev-parse --short refs/heads/master)
>  test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)
> @@ -125,11 +132,11 @@ test_atom tag refname:short testtag
>  test_atom tag upstream ''
>  test_atom tag push ''
>  test_atom tag objecttype tag
> -test_atom tag objectsize 154
> -test_atom tag objectsize:disk 138
> -test_atom tag '*objectsize:disk' 138
> -test_atom tag deltabase 0000000000000000000000000000000000000000
> -test_atom tag '*deltabase' 0000000000000000000000000000000000000000
> +test_atom tag objectsize $((114 + hexlen))

Likewise, 154 == 114 + 40 because an annotated tag has an object
pointer to a single object (i.e. its pointee).

Makes sense, too.

> +test_atom tag objectsize:disk $disklen
> +test_atom tag '*objectsize:disk' $disklen
> +test_atom tag deltabase $ZERO_OID
> +test_atom tag '*deltabase' $ZERO_OID
>  test_atom tag objectname $(git rev-parse refs/tags/testtag)
>  test_atom tag objectname:short $(git rev-parse --short refs/tags/testtag)
>  test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)
> @@ -139,7 +146,7 @@ test_atom tag parent ''
>  test_atom tag numparent ''
>  test_atom tag object $(git rev-parse refs/tags/testtag^0)
>  test_atom tag type 'commit'
> -test_atom tag '*objectname' 'ea122842f48be4afb2d1fc6a4b96c05885ab7463'
> +test_atom tag '*objectname' $(git rev-parse refs/tags/testtag^{})
>  test_atom tag '*objecttype' 'commit'
>  test_atom tag author ''
>  test_atom tag authorname ''
Jeff King Feb. 24, 2020, 6:12 p.m. UTC | #2
On Mon, Feb 24, 2020 at 10:01:08AM -0800, Junio C Hamano wrote:

> > -test_atom head objectsize 171
> > -test_atom head objectsize:disk 138
> > -test_atom head deltabase 0000000000000000000000000000000000000000
> > +test_atom head objectsize $((131 + hexlen))
> 
> 171 == 131 + 40 and that is because we are looking at the initial
> commit, whose contents has a single object name (i.e. its tree).

I wonder if it would be more readable to just pipe "cat-file" through
"wc -c", rather than hard-coding. Then there's no magic number at all.

> > +test_atom head objectsize:disk $disklen

Likewise for $disklen, if it's a loose object we could just get the
information from the filesystem. That would stop us caring about the
hash, _and_ it would make us robust to random changes in the zlib
compression.

I'm not sure if we also check packed objects. If so, you can compute the
size from the output of show-index, which gives the offsets of each
object. That's also how Git does it internally, though, so I'm not sure
if that is getting too close to just testing nothing (but IMHO the thing
we're really covering here is the format routines).

-Peff
Junio C Hamano Feb. 24, 2020, 8:41 p.m. UTC | #3
Jeff King <peff@peff.net> writes:

> On Mon, Feb 24, 2020 at 10:01:08AM -0800, Junio C Hamano wrote:
>
>> > -test_atom head objectsize 171
>> > -test_atom head objectsize:disk 138
>> > -test_atom head deltabase 0000000000000000000000000000000000000000
>> > +test_atom head objectsize $((131 + hexlen))
>> 
>> 171 == 131 + 40 and that is because we are looking at the initial
>> commit, whose contents has a single object name (i.e. its tree).
>
> I wonder if it would be more readable to just pipe "cat-file" through
> "wc -c", rather than hard-coding. Then there's no magic number at all.

After seeing nearby tests use output from $(git rev-parse) as the
expected output, I had a similar thought.

>> > +test_atom head objectsize:disk $disklen
>
> Likewise for $disklen, if it's a loose object we could just get the
> information from the filesystem. That would stop us caring about the
> hash, _and_ it would make us robust to random changes in the zlib
> compression.
>
> I'm not sure if we also check packed objects. If so, you can compute the
> size from the output of show-index, which gives the offsets of each
> object. That's also how Git does it internally, though, so I'm not sure
> if that is getting too close to just testing nothing (but IMHO the thing
> we're really covering here is the format routines).

Somebody may find it tempting to use "cat-file --batch-check=<format>"
and at that point it would really become fuzzy what we are checking ;-)

Patch
diff mbox series

diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh
index 9c910ce746..2406b93d35 100755
--- a/t/t6300-for-each-ref.sh
+++ b/t/t6300-for-each-ref.sh
@@ -20,6 +20,10 @@  setdate_and_increment () {
 }
 
 test_expect_success setup '
+	test_oid_cache <<-EOF &&
+	disklen sha1:138
+	disklen sha256:154
+	EOF
 	setdate_and_increment &&
 	echo "Using $datestamp" > one &&
 	git add one &&
@@ -50,6 +54,9 @@  test_atom() {
 	"
 }
 
+hexlen=$(test_oid hexsz)
+disklen=$(test_oid disklen)
+
 test_atom head refname refs/heads/master
 test_atom head refname: refs/heads/master
 test_atom head refname:short master
@@ -82,9 +89,9 @@  test_atom head push:rstrip=-1 refs
 test_atom head push:strip=1 remotes/myfork/master
 test_atom head push:strip=-1 master
 test_atom head objecttype commit
-test_atom head objectsize 171
-test_atom head objectsize:disk 138
-test_atom head deltabase 0000000000000000000000000000000000000000
+test_atom head objectsize $((131 + hexlen))
+test_atom head objectsize:disk $disklen
+test_atom head deltabase $ZERO_OID
 test_atom head objectname $(git rev-parse refs/heads/master)
 test_atom head objectname:short $(git rev-parse --short refs/heads/master)
 test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)
@@ -125,11 +132,11 @@  test_atom tag refname:short testtag
 test_atom tag upstream ''
 test_atom tag push ''
 test_atom tag objecttype tag
-test_atom tag objectsize 154
-test_atom tag objectsize:disk 138
-test_atom tag '*objectsize:disk' 138
-test_atom tag deltabase 0000000000000000000000000000000000000000
-test_atom tag '*deltabase' 0000000000000000000000000000000000000000
+test_atom tag objectsize $((114 + hexlen))
+test_atom tag objectsize:disk $disklen
+test_atom tag '*objectsize:disk' $disklen
+test_atom tag deltabase $ZERO_OID
+test_atom tag '*deltabase' $ZERO_OID
 test_atom tag objectname $(git rev-parse refs/tags/testtag)
 test_atom tag objectname:short $(git rev-parse --short refs/tags/testtag)
 test_atom head objectname:short=1 $(git rev-parse --short=1 refs/heads/master)
@@ -139,7 +146,7 @@  test_atom tag parent ''
 test_atom tag numparent ''
 test_atom tag object $(git rev-parse refs/tags/testtag^0)
 test_atom tag type 'commit'
-test_atom tag '*objectname' 'ea122842f48be4afb2d1fc6a4b96c05885ab7463'
+test_atom tag '*objectname' $(git rev-parse refs/tags/testtag^{})
 test_atom tag '*objecttype' 'commit'
 test_atom tag author ''
 test_atom tag authorname ''