| Message ID | 20200723010943.2329634-35-sandals@crustytoothpaste.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | SHA-256, part 3/3 | expand |
On Wed, Jul 22, 2020 at 9:11 PM brian m. carlson <sandals@crustytoothpaste.net> wrote: > 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" > 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". > 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"?
On 2020-07-23 at 04:51:32, Eric Sunshine wrote: > On Wed, Jul 22, 2020 at 9:11 PM brian m. carlson > <sandals@crustytoothpaste.net> wrote: > > 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" > > > 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". Yeah, these are both typos. Will fix. > > 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? Sure. > > + case "$1" in > > + --hash=*) > > + algo="${1#--hash=}" && > > Bikeshedding: I wonder if this should be named "--algo"? We already have a use of this in t5515; I appear to have accidentally sent it in an earlier series. I can, of course, change it, but I don't feel strongly that one or the other is better. If you do feel strongly, or you think it's confusing, I'm happy to change it.
On Thu, Jul 23, 2020 at 7:39 PM brian m. carlson <sandals@crustytoothpaste.net> wrote: > On 2020-07-23 at 04:51:32, Eric Sunshine wrote: > > On Wed, Jul 22, 2020 at 9:11 PM brian m. carlson > > <sandals@crustytoothpaste.net> wrote: > > > + --hash=*) > > > + algo="${1#--hash=}" && > > > > Bikeshedding: I wonder if this should be named "--algo"? > > We already have a use of this in t5515; I appear to have accidentally > sent it in an earlier series. I can, of course, change it, but I don't > feel strongly that one or the other is better. If you do feel strongly, > or you think it's confusing, I'm happy to change it. I don't feel strongly about it and wouldn't insist that it be changed.
Eric Sunshine <sunshine@sunshineco.com> writes: > On Thu, Jul 23, 2020 at 7:39 PM brian m. carlson > <sandals@crustytoothpaste.net> wrote: >> On 2020-07-23 at 04:51:32, Eric Sunshine wrote: >> > On Wed, Jul 22, 2020 at 9:11 PM brian m. carlson >> > <sandals@crustytoothpaste.net> wrote: >> > > + --hash=*) >> > > + algo="${1#--hash=}" && >> > >> > Bikeshedding: I wonder if this should be named "--algo"? >> >> We already have a use of this in t5515; I appear to have accidentally >> sent it in an earlier series. I can, of course, change it, but I don't >> feel strongly that one or the other is better. If you do feel strongly, >> or you think it's confusing, I'm happy to change it. > > I don't feel strongly about it and wouldn't insist that it be changed. I am in favor of "algo", but let's leave the patch noise outside the series. It is something we can do after the dust settles. And when such a clean-up comes, I'll try to remember pointing out the case/arms)/esac indentation rule ;-) Thanks.
diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh index 2ff176cd5d..47d6b502c2 100755 --- 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 "$hexsz" -eq 64 ' +test_expect_success 'test_oid can look up data a specified algorithm' ' + rawsz="$(test_oid --hash=sha1 rawsz)" && + hexsz="$(test_oid --hash=sha1 hexsz)" && + test "$rawsz" -eq 20 && + test "$hexsz" -eq 40 && + rawsz="$(test_oid --hash=sha256 rawsz)" && + hexsz="$(test_oid --hash=sha256 hexsz)" && + test "$rawsz" -eq 32 && + test "$hexsz" -eq 64 +' + test_expect_success 'test_bool_env' ' ( sane_unset envvar && diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 3103be8a32..d243ff43f3 100644 --- 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 () { - local var="test_oid_${test_hash_algo}_$1" && + local algo="${test_hash_algo}" && + + case "$1" in + --hash=*) + algo="${1#--hash=}" && + shift;; + *) + ;; + esac && + + local var="test_oid_${algo}_$1" && # If the variable is unset, we must be missing an entry for this # key-hash pair, so exit with an error.
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 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> --- t/t0000-basic.sh | 11 +++++++++++ t/test-lib-functions.sh | 12 +++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-)