Re: [PATCH 2/3] t: add t0016-oidmap.sh

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Jun 9, 2019, 09:22 UTC
Message-ID: <20190609092259.GB24208@szeder.dev>
In-Reply-To: <20190609044907.32477-3-chriscool@tuxfamily.org>

On Sun, Jun 09, 2019 at 06:49:06AM +0200, Christian Couder wrote:

Show 26 quoted lines

> From: Christian Couder <christian.couder@gmail.com>
> 
> Add actual tests for operations using `struct oidmap` from oidmap.{c,h}.
> 
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  t/t0016-oidmap.sh | 100 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 100 insertions(+)
>  create mode 100755 t/t0016-oidmap.sh
> 
> diff --git a/t/t0016-oidmap.sh b/t/t0016-oidmap.sh
> new file mode 100755
> index 0000000000..3a8e8bdb3d
> --- /dev/null
> +++ b/t/t0016-oidmap.sh
> @@ -0,0 +1,100 @@
> +#!/bin/sh
> +
> +test_description='test oidmap'
> +. ./test-lib.sh
> +
> +# This purposefully is very similar to t0011-hashmap.sh
> +
> +test_oidmap() {
> +	echo "$1" | test-tool oidmap $3 > actual &&
> +	echo "$2" > expect &&

Style nit: space between redirection op and filename.

Show 15 quoted lines

> +	test_cmp expect actual
> +}
> +
> +
> +test_expect_success 'setup' '
> +
> +	test_commit one &&
> +	test_commit two &&
> +	test_commit three &&
> +	test_commit four
> +
> +'
> +
> +test_oidhash() {
> +	git rev-parse "$1" | perl -ne 'print hex("$4$3$2$1") . "\n" if m/^(..)(..)(..)(..).*/;'

New Perl dependencies always make Dscho sad... :)

So, 'test oidmap' from the previous patch prints the value we want to check with:

    printf("%u\n", sha1hash(oid.hash));

First, since object ids inherently make more sense as hex values, it would be more appropriate to print that hash with the '%x' format specifier, and then we wouldn't need Perl's hex() anymore, and thus could swap the order of the first four bytes in oidmap's hash without relying on Perl, e.g. with:

  sed -e 's/^\(..\)\(..\)\(..\)\(..\).*/\4\3\2\1/'

Second, and more importantly, the need for swapping the byte order indicates that this test would fail on big-endian systems, I'm afraid. So I think we need an additional bswap32() on the printing side, and then could further simplify 'test_oidhash':

diff --git a/t/helper/test-oidmap.c b/t/helper/test-oidmap.c
index 0ba122a264..4177912f9a 100644
--- a/t/helper/test-oidmap.c
+++ b/t/helper/test-oidmap.c
@@ -51,7 +51,7 @@ int cmd__oidmap(int argc, const char **argv)
 
 			/* print hash of oid */
 			if (!get_oid(p1, &oid))
-				printf("%u\n", sha1hash(oid.hash));
+				printf("%x\n", bswap32(sha1hash(oid.hash)));
 			else
 				printf("Unknown oid: %s\n", p1);
 
diff --git a/t/t0016-oidmap.sh b/t/t0016-oidmap.sh
index 3a8e8bdb3d..9c0d88a316 100755
--- a/t/t0016-oidmap.sh
+++ b/t/t0016-oidmap.sh
@@ -22,10 +22,10 @@ test_expect_success 'setup' '
 '
 
 test_oidhash() {
-	git rev-parse "$1" | perl -ne 'print hex("$4$3$2$1") . "\n" if m/^(..)(..)(..)(..).*/;'
+	git rev-parse "$1" | cut -c1-8
 }
 
-test_expect_success PERL 'hash' '
+test_expect_success 'hash' '
 
 test_oidmap "hash one
 hash two


> +}
> +
> +test_expect_success PERL 'hash' '
> +
> +test_oidmap "hash one
> +hash two
> +hash invalidOid
> +hash three" "$(test_oidhash one)
> +$(test_oidhash two)
> +Unknown oid: invalidOid
> +$(test_oidhash three)"
> +
> +'
> +
> +test_expect_success 'put' '
> +
> +test_oidmap "put one 1
> +put two 2
> +put invalidOid 4
> +put three 3" "NULL
> +NULL
> +Unknown oid: invalidOid
> +NULL"
> +
> +'
> +
> +test_expect_success 'replace' '
> +
> +test_oidmap "put one 1
> +put two 2
> +put three 3
> +put invalidOid 4
> +put two deux
> +put one un" "NULL
> +NULL
> +NULL
> +Unknown oid: invalidOid
> +2
> +1"
> +
> +'
> +
> +test_expect_success 'get' '
> +
> +test_oidmap "put one 1
> +put two 2
> +put three 3
> +get two
> +get four
> +get invalidOid
> +get one" "NULL
> +NULL
> +NULL
> +2
> +NULL
> +Unknown oid: invalidOid
> +1"
> +
> +'
> +
> +test_expect_success 'iterate' '
> +
> +test_oidmap "put one 1
> +put two 2
> +put three 3
> +iterate" "NULL
> +NULL
> +NULL
> +$(git rev-parse two) 2
> +$(git rev-parse one) 1
> +$(git rev-parse three) 3"
> +
> +'
> +
> +test_done
> -- 
> 2.22.0.14.g9023ccb50a
>