On Mon, Jun 11, 2018 at 03:47:43AM -0400, Eric Sunshine wrote:
On Mon, Jun 04, 2018 at 11:52:20PM +0000, brian m. carlson wrote:
quoted
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.
The word "translate" is very generic and is (at least in my mind)
strongly associated with i18n/l10n, so the name test_translate() may
be confusing for readers. Perhaps test_oid_lookup() or test_oid_get()
or even just test_oid()?
test_oid would be fine. One note is that this doesn't always produce
OIDs; sometimes it will produce other values, but as long as you don't
think that's too confusing, I'm fine with it.
This is a very expensive lookup since it invokes a heavyweight command
(perl, in this case) for *every* OID it needs to retrieve from the
file. Windows users, especially, will likely not be happy about this.
See below for an alternative.
I agree perl would be expensive if it were invoked frequently, but
excepting SHA1-prerequisite tests, this function is invoked 32 times in
the entire testsuite.
One of the reasons I chose perl was because we have a variety of cases
where we'll need spaces in values, and those tend to be complex in
shell.
This is less flexible than I had expected, allowing for only SHA1 and
NewHash. When you had written about OID lookup table functionality in
email previously, my impression was that the tables would allow values
for arbitrary hash algorithms. Such flexibility would allow people to
experiment with hash algorithms without having to once again retrofit
the test suite machinery.
I wasn't thinking of that as a goal, but that would be a nice
improvement.
quoted hunk
Here's what I had envisioned when reading your emails about OID lookup
table functionality:
test_detect_hash () {
test_hash_algo=...
}
test_oid_cache () {
while read tag rest
do
case $tag in \#*) continue ;; esac
for x in $rest
do
k=${x%:*}
v=${x#*:}
if test "$k" = $test_hash_algo
then
eval "test_oid_$tag=$v"
break
fi
done
done
}
test_oid () {
if test $# -gt 1
then
test_oid_cache <<-EOF
$*
EOF
fi
eval "echo \$test_oid_$1"
}
Using shell variables like this does have the downside that we're
restricted to only characters allowed in shell variables. That was
something I was trying to avoid, but it certainly isn't fatal.
test_detect_hash() would detect the hash algorithm and record it
instead of having to determine it each time an OID needs to be
"translated". It probably would be called by test-lib.sh.
We'll probably have to deal with multiple hashes in the future,
including for input and output, but this could probably be coerced to
handle that case.
And, when specifying values from which to choose based upon hash
algorithm:
$(test_oid bored sha1:deadbeef NewHash:feedface)
This syntax won't exactly be usable, because we have to deal with spaces
in values. It shouldn't be too much of a problem to just use
test_oid_cache at the top of the file, though.
A nice property of how this implementation caches values is that you
don't need test_oid() for really simple cases. You can just access the
variable directly. For instance: $test_oid_hexsz
Because we're going to need multiple hash support in the future, for
input, output, and on-disk, I feel like this is not a good direction for
us to go in the testsuite. Internally, those variable names are likely
to change.
Another nice property of how caching is implemented is that someone
testing a new hash algorithm doesn't have edit the existing tables to
tack the value for the new algorithm onto the end of each line. It
works equally well to place those values in a new file or new here-doc
or simply append new lines to existing files or here-docs. For
instance, someone testing algorithm "NewShiny" can just add those
lines without having to modify existing lines:
That would be convenient.
I like a lot of things about your implementation. This has been helpful
feedback; let me think about it some and reroll.
--
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204