[libvirt] [PATCH] tests: More unit tests for internal hash APIs

[Eric Blake]

On 04/19/2011 07:22 AM, Jiri Denemark wrote:
> This adds several tests for remaining hash APIs (custom
> hasher/comparator functions are not covered yet, though).
> 
> All tests pass both before and after the "Simplify hash implementation".
> ---
>  src/util/hash.c  |   18 +++
>  src/util/hash.h  |    1 +
>  tests/hashdata.h |  237 +++++++++++++++++++++++++++++++++++-
>  tests/hashtest.c |  361 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 615 insertions(+), 2 deletions(-)
> 
> diff --git a/src/util/hash.c b/src/util/hash.c
> index fc7652d..5dbc7f1 100644
> --- a/src/util/hash.c
> +++ b/src/util/hash.c
> @@ -499,6 +499,24 @@ virHashSize(virHashTablePtr table)
>  }
>  
>  /**
> + * virHashTableSize:
> + * @table: the hash table
> + *
> + * Query the size of the hash @table, i.e., number of keys in the table.

This is confusing.  I think virHashSize is already the number of keys
(or elements) in the table; whereas virHashTableSize is the number of
buckets (> keys if table is not full, < keys if table is using lots of
collision chains but hasn't had a reason to grow).  I'd rather use the
term "buckets" than "keys" for this documentation.

> + *
> + * Returns the number of keys in the hash table or
> + * -1 in case of error
> + */
> +int
> +virHashTableSize(virHashTablePtr table)
> +{
> +    if (table == NULL)
> +        return -1;
> +    return table->size;
> +}

Nice, but does it also need an addition to src/libvirt_private.syms?

>              "d1a564b2-c7f3-4b76-8712-3b8f5aae6ded",
> -            "0e614f33-c1da-4cfe-b6d5-65ecd2d066f2"
> +            "0e614f33-c1da-4cfe-b6d5-65ecd2d066f2",
> +/* [250] */ "334fdaba-f373-42ff-8546-219c1463a7c5",
> +            "d4ff408e-8d43-46ff-94a4-bcfa6c994776",
> +/* [253] */ "d1abd887-d5de-46b0-88d6-f71f31a61305",
> +/* [254] */ "1d76af65-64d6-4211-b1b5-f5b799595e4d",
> +/* [255] */ "b3ad4257-29b0-4c44-b7a7-95f1d102769c"

Put a trailing comma on this one (if you had done that last time, we
wouldn't have the - vs. + line for 0e614f33... a few lines earlier).

> +const char *uuids_new[] = {
> +    "a103cc42-d0e5-40fb-8f7f-3c1bee93e327",
> +    "01e13dc5-e65b-4988-a0cc-0d2f1f1e10fe",
> +    "71f3678a-a8c6-4176-a26e-c34779067135",
> +    "4f054508-22d5-49e1-9962-7508225c8b16",
> +    "e572116b-5b7b-45fd-bbe9-296029ce16b5",
> +    "695c8cfe-9830-4d9a-be67-50a124cefb76",
> +    "ea244996-645b-4a19-ad4a-48f3022b8e94",
> +    "0fd98758-9cc4-4779-b403-95ae3500f138",
> +    "b86772b4-0728-46ae-99e8-027799697838",
> +    "1c0cd559-81cd-4c27-8e24-6aef6f5af7f1",
> +    "2a37fe4a-8825-4fd6-9284-e1606967548a",
> +    "5920cc9d-62a3-4772-9e73-eb97f0bc483c",
> +    "53c215dd-bdba-4fdc-887a-86ab6f860df4"

Trailing comma here, too.

> +static int
> +testHashRemoveSetIter(const void *payload ATTRIBUTE_UNUSED,
> +                      const void *name,
> +                      const void *data)
> +{
> +    int *count = (int *) data;
> +    bool rem = false;
> +    int i;
> +
> +    for (i = 0; i < ARRAY_CARDINALITY(uuids_subset); i++) {
> +        if (STREQ(uuids_subset[i], name)) {
> +            rem = true;
> +            break;
> +        }
> +    }
> +
> +    if (rem || rand() % 2) {

Do we need to seed the random number pseudo-generator, so that the test
is reproducible?  I guess I'm okay if it's not seeded, though.

Overall, this looks like a good patch; the fact that it passes 'make
check' speaks highly, and it is some good additional coverage.

ACK with nits fixed.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110426/f597873d/attachment-0001.sig>