Re: [PATCH v3 34/39] t: add test_oid option to select hash algorithm

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jul 23, 2020, 04:51 UTC
Message-ID: <CAPig+cT36rhfMzOTH8L59cyoBKQQoF92nJ-Unccm+yf9RCgXMQ@mail.gmail.com>
In-Reply-To: <20200723010943.2329634-35-sandals@crustytoothpaste.net>

On Wed, Jul 22, 2020 at 9:11 PM brian m. carlson <sandals@crustytoothpaste.net> wrote:

Show 7 quoted lines

> In some tests, we have data files which are written with a particular
> hash algorithm. Instead of keeping two copies of the test files, we can
> keep one, and translate the value on the fly.
>
> In order to do so, we'll need to read both the source algorithm and the
> current algorithm, so add an optional flag to the test_oid helper that
> lets us read look up a value for a specified hash algorithm. This should

Readers trip over confusing grammar: "lets us read look up a value"

Show 8 quoted lines

> not cause any conflicts with existing tests, since key arguments to
> test_oid are allowed to contains only shell identifier characters.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> @@ -928,6 +928,17 @@ test_expect_success 'test_oid can look up data for SHA-256' '
> +test_expect_success 'test_oid can look up data a specified algorithm' '

Readers trip over confusing grammar: "can look up data a specified".

Show 5 quoted lines

> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> @@ -1468,7 +1468,17 @@ test_oid_cache () {
>  # Look up a per-hash value based on a key ($1).  The value must have been loaded
>  # by test_oid_init or test_oid_cache.
>  test_oid () {

Should the function documentation be updated to talk about the new --hash option?

> +       case "$1" in
> +               --hash=*)
> +                       algo="${1#--hash=}" &&

Bikeshedding: I wonder if this should be named "--algo"?