Hi,
brian m. carlson wrote:
> Add a test function helper, test_oid, that produces output that varies
> depending on the hash in use.
Cool!
> Add two additional helpers,
> test_oid_cache, which can be used to load data for test_oid from
> standard input, and test_oid_init, which can be used to load certain
> fixed values from lookup charts. Check that these functions work in
> t0000, as the rest of the testsuite will soon come to depend on them.
>
> Implement two basic lookup charts, one for common invalid or synthesized
> object IDs, and one for various facts about the hash function in use.
> Provide versions for both SHA-1 and SHA-256.
What do test_oid_cache and test_oid_init do? How can I use them?
Judging from t0000-basic.sh, the idea looks something like
Add a test function helper, test_oid, that ...
test_oid allows looking up arbitrary information about an object format:
the length of object ids, values of well known object ids, etc. Before
calling it, a test script must invoke test_oid_cache (either directly
or indirectly through test_oid_init) to load the lookup charts.
See t0000 for an example, which also serves as a sanity-check that
these functions work in preparation for using them in the rest of the
test suite.
There are two basic lookup charts for now: oid-info/oid, with common
invalid or synthesized object IDs; and oid-info/hash-info, with facts
such as object id length about the formats in use. The charts contain
information about both SHA-1 and SHA-256.
So now you can update existing tests to be format-independent by (1)
adding an invocation of test_oid_init to test setup and (2) replacing
format dependencies with $(test_oid foo).
Since values are stored as shell variables, names used for lookup can
only consist of shell identifier characters. If this is a problem in
the future, we can hash the names before use.
Improved-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
Do these always use sha1 for now? Ah, t0000 answers but it might be
worth mentioning in the commit message, too:
test_set_hash allows setting which object format test_oid should look
up information for, and test_detect_hash returns to the default format.
[...]
> --- /dev/null
> +++ b/t/oid-info/hash-info
> @@ -0,0 +1,8 @@
> +rawsz sha1:20
> +rawsz sha256:32
Can there be a README in this directory describing the files and format?
[...]
> --- a/t/t0000-basic.sh
> +++ b/t/t0000-basic.sh
> @@ -821,6 +821,41 @@ test_expect_success 'tests clean up even on failures' "
> EOF
> "
>
> +test_oid_init
Can this be wrapped in test_expect_success? That way, if it fails or
prints an error message then the usual test machinery would handle it.
> +
> +test_expect_success 'test_oid provides sane info by default' '
> + test_oid zero >actual &&
> + grep "^00*$" actual &&
nit: can save the reader some confusion by escaping the $.
> + rawsz="$(test_oid rawsz)" &&
> + hexsz="$(test_oid hexsz)" &&
optional: no need for these quotation marks --- a command substitution
assigned to a shell variable is treated as if it were quoted.
> + test "$hexsz" -eq $(wc -c <actual) &&
> + test $(( $rawsz * 2)) -eq "$hexsz"
Makes sense.
[...]
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -1155,3 +1155,47 @@ depacketize () {
[...]
> +test_oid_cache () {
> + test -n "$test_hash_algo" || test_detect_hash
Should this use an uninterrupted &&-chain?
> + while read _tag _rest
This appears to be the first use of this naming convention. I wonder
if we can use "local" instead.
> + do
> + case $_tag in
> + \#*)
> + continue;;
> + ?*)
> + # non-empty
> + ;;
> + *)
> + # blank line
> + continue;;
> +
unnecessary blank line here
> + esac &&
> +
> + _k="${_rest%:*}" &&
> + _v="${_rest#*:}" &&
> + { echo "$_k" | egrep '^[a-z0-9]+$' >/dev/null ||
> + error 'bug in the test script: bad hash algorithm'; } &&
> + eval "test_oid_${_k}_$_tag=\"\$_v\"" || return 1
This is dense, so I'm having trouble taking it in at a glance.
I think the idea is
key=${rest%%:*} &&
val=${rest#*:} &&
if ! expr "$key" : '[a-z0-9]*$' >/dev/null
then
error ...
fi &&
eval "test_oid_${key}_${tag}=\${val}"
> + done
> +}
> +
> +test_oid () {
> + eval "
> + test -n \"\${test_oid_${test_hash_algo}_$1+set}\" &&
> + printf '%s' \"\${test_oid_${test_hash_algo}_$1}\"
> + "
I'm also having trouble taking this one in. Maybe splitting into two
evals would work?
var=test_oid_${test_hash_algo}_$1 &&
eval "test -n \"\${$var+set}\"" &&
eval "printf '%s\n' \"\${$var}\""
What is the initial test meant to do? Can this function get a
documentation comment? Are we relying on "test -n" to return a failing
result if the variable is unset, or could the test be omitted (relying
on "\${$var}" to evaluate to "" when the variable is unset)? Should
this call 'error' when the variable is unset?
Can t/README describe the new test helpers?
Thanks,
Jonathan
