Re: [PATCH 01/10] t: add tool to translate hash-related values

["Torsten Bögershausen" <tboegi@web.de>]

Some style nite inline

On Mon, Jun 04, 2018 at 11:52:20PM +0000, brian m. carlson wrote:
> Add a test function helper, test_translate, that will produce its first
> argument if the hash in use is SHA-1 and the second if its argument is
> NewHash.  Implement a mode that can read entries from a file as well for
> reusability across tests.
> 
> For the moment, use the length of the empty blob to determine the hash
> in use.  In the future, we can change this code so that it can use the
> configuration and learn about the difference in input, output, and
> on-disk formats.
> 
> Implement two basic lookup charts, one for common invalid or synthesized
> object IDs, and one for various facts about the hash function in use.
> 
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  t/test-lib-functions.sh | 40 ++++++++++++++++++++++++++++++++++++++++
>  t/translate/hash-info   |  9 +++++++++
>  t/translate/oid         | 15 +++++++++++++++
>  3 files changed, 64 insertions(+)
>  create mode 100644 t/translate/hash-info
>  create mode 100644 t/translate/oid
> 
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 2b2181dca0..0e7067460b 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -1147,3 +1147,43 @@ depacketize () {
>  		}
>  	'
>  }
> +
> +test_translate_f_ () {
> +	local file="$TEST_DIRECTORY/translate/$2" &&

Unless I'm wrong, we don't use the "local" keyword ?

> +	perl -e '

The bare "perl" is better spelled as "$PERL_PATH"


> +		$delim = "\t";
> +		($hoidlen, $file, $arg) = @ARGV;
> +		open($fh, "<", $file) or die "open: $!";
> +		while (<$fh>) {
> +			# Allow specifying other delimiters.
> +			$delim = $1 if /^#!\sdelimiter\s(.)/;
> +			next if /^#/;
> +			@fields = split /$delim/, $_, 3;
> +			if ($fields[0] eq $arg) {
> +				print($hoidlen == 40 ? $fields[1] : $fields[2]);
> +				last;
> +			}
> +		}
> +	' "$1" "$file" "$3"
> +}
> +
> +# Without -f, print the first argument if we are using SHA-1 and the second if
> +# we're using NewHash.
> +# With -f FILE ARG, read the (by default) tab-delimited file from
> +# t/translate/FILE, finding the first field matching ARG and printing either the
> +# second or third field depending on the hash in use.
> +test_translate () {
> +	local hoidlen=$(printf "%s" "$EMPTY_BLOB" | wc -c) &&
> +	if [ "$1" = "-f" ]

Style nit, please avoid [] and use test:
	if test "$1" = "-f"

And more [] below

> +	then
> +		shift &&
> +		test_translate_f_ "$hoidlen" "$@"
> +	else
> +		if [ "$hoidlen" -eq 40 ]
> +		then
> +			printf "%s" "$1"
> +		else
> +			printf "%s" "$2"
> +		fi
> +	fi
> +}
> diff --git a/t/translate/hash-info b/t/translate/hash-info
> new file mode 100644
> index 0000000000..36cbd9a8eb
> --- /dev/null
> +++ b/t/translate/hash-info
> @@ -0,0 +1,9 @@
> +# Various facts about the hash algorithm in use for easy access in tests.
> +#
> +# Several aliases are provided for easy recall.
> +rawsz	20	32
> +oidlen	20	32
> +hexsz	40	64
> +hexoidlen	40	64
> +zero	0000000000000000000000000000000000000000	0000000000000000000000000000000000000000000000000000000000000000
> +zero-oid	0000000000000000000000000000000000000000	0000000000000000000000000000000000000000000000000000000000000000
> diff --git a/t/translate/oid b/t/translate/oid
> new file mode 100644
> index 0000000000..8de0fd64af
> --- /dev/null
> +++ b/t/translate/oid
> @@ -0,0 +1,15 @@
> +# These are some common invalid and partial object IDs used in tests.
> +001	0000000000000000000000000000000000000001	0000000000000000000000000000000000000000000000000000000000000001
> +002	0000000000000000000000000000000000000002	0000000000000000000000000000000000000000000000000000000000000002
> +003	0000000000000000000000000000000000000003	0000000000000000000000000000000000000000000000000000000000000003
> +004	0000000000000000000000000000000000000004	0000000000000000000000000000000000000000000000000000000000000004
> +005	0000000000000000000000000000000000000005	0000000000000000000000000000000000000000000000000000000000000005
> +006	0000000000000000000000000000000000000006	0000000000000000000000000000000000000000000000000000000000000006
> +007	0000000000000000000000000000000000000007	0000000000000000000000000000000000000000000000000000000000000007
> +# All zeros or Fs missing one or two hex segments.
> +zero-1	000000000000000000000000000000000000000	000000000000000000000000000000000000000000000000000000000000000
> +zero-2	00000000000000000000000000000000000000	00000000000000000000000000000000000000000000000000000000000000
> +ff-1	fffffffffffffffffffffffffffffffffffffff	fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> +ff-2	ffffffffffffffffffffffffffffffffffffff	ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> +numeric	0123456789012345678901234567890123456789	0123456789012345678901234567890123456789012345678901234567890123
> +deadbeef	deadbeefdeadbeefdeadbeefdeadbeefdeadbeef	deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef