[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] tests: Unit tests for internal hash APIs



On 04/15/2011 02:04 PM, Jiri Denemark wrote:
> This is a basic set of tests for testing removals of hash entries during
> iteration.
> ---
> More tests for all other hash APIs will come on Monday.

Such as a test that gets 8 collisions into a single bucket to force hash
table growth, or a test of custom hasher/comparator functions?

> 
>  tests/Makefile.am |    8 +++-
>  tests/hashdata.h  |   33 +++++++++++
>  tests/hashtest.c  |  157 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 197 insertions(+), 1 deletions(-)
>  create mode 100644 tests/hashdata.h
>  create mode 100644 tests/hashtest.c
> 
> +++ b/tests/hashdata.h
> @@ -0,0 +1,33 @@
> +const char *uuids[] = {
> +/* [ 46] */ "f17494ba-2353-4af0-b1ba-13680858edcc",
> +            "64ab4e78-1b6e-4b88-b47f-2def46c79a86",
> +            "f99b0d59-ecff-4cc6-a9d3-20159536edc8",
> +/* [ 75] */ "e1bfa30f-bc0b-4a24-99ae-bed7f3f21a7b",
> +            "acda5fa0-58de-4e1e-aa65-2124d1e29c54",
> +/* [ 76] */ "5f476c28-8f26-48e0-98de-85745fe2f304",

Looks suspiciously like you used gdb to dump an existing hash structure
on a machine with lots of VMs :)  Works for me.

> +static virHashTablePtr
> +testHashInit(int size)
> +{
> +    virHashTablePtr hash;
> +    int i;
> +
> +    if (!(hash = virHashCreate(size, NULL)))
> +        return NULL;
> +
> +    /* entires are added in reverse order so that they will be linked in
> +     * collision list in the same order as in the uuids array

We're abusing an an internal detail.  So good to have the comment - if
the test breaks because of another rewrite, but the only breakage is due
to a different link order in each bucket, then I'm okay modifying the
test at that point, and meanwhile it justifies our abuse (that is, no
change needed).

> +#define DO_TEST_DATA(name, cmd, data)                               \
> +    DO_TEST_FULL(name "(" #data ")",                                \
> +                 cmd,                                               \
> +                 testHash ## cmd ## data,                           \
> +                 testHashCount ## cmd ## data)
> +
> +    DO_TEST_DATA("Remove in ForEach", RemoveForEach, Some);
> +    DO_TEST_DATA("Remove in ForEach", RemoveForEach, All);
> +
> +    return (ret == 0) ? EXIT_SUCCESS : EXIT_FAILURE;

Looks like a good test; it certainly would have caught the previous bugs
before the most recent hash.c fixes.

ACK.

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

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]