[libvirt] [PATCH 1/2] Add tests for virUSBDeviceFind functions

Michal Privoznik mprivozn at redhat.com
Thu Feb 27 13:54:51 UTC 2014


On 27.02.2014 14:27, Michal Privoznik wrote:
> On 26.02.2014 17:54, Ján Tomko wrote:
>> Mock the /sys/bus/usb directory and test the finding
>> (and not finding) of some USB devices.
>> ---
>>   .gitignore                                         |   1 +
>>   cfg.mk                                             |   3 +-
>>   tests/Makefile.am                                  |  12 +
>>   tests/virusbtest.c                                 | 268
>> +++++++++++++++++++++
>>   .../sys_bus_usb/devices/1-1.5.3.1/devnum           |   1 +
>>   .../sys_bus_usb/devices/1-1.5.3.1/idProduct        |   1 +
>>   .../sys_bus_usb/devices/1-1.5.3.1/idVendor         |   1 +
>>   .../sys_bus_usb/devices/1-1.5.3.1/serial           |   1 +
>>   .../sys_bus_usb/devices/1-1.5.3.3/devnum           |   1 +
>>   .../sys_bus_usb/devices/1-1.5.3.3/idProduct        |   1 +
>>   .../sys_bus_usb/devices/1-1.5.3.3/idVendor         |   1 +
>>   .../sys_bus_usb/devices/1-1.5.3.3/serial           |   1 +
>>   .../sys_bus_usb/devices/1-1.5.3/devnum             |   1 +
>>   .../sys_bus_usb/devices/1-1.5.3/idProduct          |   1 +
>>   .../sys_bus_usb/devices/1-1.5.3/idVendor           |   1 +
>>   .../sys_bus_usb/devices/1-1.5.3/serial             |   1 +
>>   .../sys_bus_usb/devices/1-1.5.4/devnum             |   1 +
>>   .../sys_bus_usb/devices/1-1.5.4/idProduct          |   1 +
>>   .../sys_bus_usb/devices/1-1.5.4/idVendor           |   1 +
>>   .../sys_bus_usb/devices/1-1.5.4/serial             |   1 +
>>   .../sys_bus_usb/devices/1-1.5.5/devnum             |   1 +
>>   .../sys_bus_usb/devices/1-1.5.5/idProduct          |   1 +
>>   .../sys_bus_usb/devices/1-1.5.5/idVendor           |   1 +
>>   .../sys_bus_usb/devices/1-1.5.5/serial             |   1 +
>>   .../sys_bus_usb/devices/1-1.5.6/devnum             |   1 +
>>   .../sys_bus_usb/devices/1-1.5.6/idProduct          |   1 +
>>   .../sys_bus_usb/devices/1-1.5.6/idVendor           |   1 +
>>   .../sys_bus_usb/devices/1-1.5.6/serial             |   1 +
>>   .../sys_bus_usb/devices/1-1.5/devnum               |   1 +
>>   .../sys_bus_usb/devices/1-1.5/idProduct            |   1 +
>>   .../sys_bus_usb/devices/1-1.5/idVendor             |   1 +
>>   .../sys_bus_usb/devices/1-1.5/serial               |   1 +
>>   .../sys_bus_usb/devices/1-1.6/devnum               |   1 +
>>   .../sys_bus_usb/devices/1-1.6/idProduct            |   1 +
>>   .../sys_bus_usb/devices/1-1.6/idVendor             |   1 +
>>   .../sys_bus_usb/devices/1-1.6/serial               |   1 +
>>   .../virusbtestdata/sys_bus_usb/devices/1-1/devnum  |   1 +
>>   .../sys_bus_usb/devices/1-1/idProduct              |   1 +
>>   .../sys_bus_usb/devices/1-1/idVendor               |   1 +
>>   .../virusbtestdata/sys_bus_usb/devices/1-1/serial  |   1 +
>>   .../sys_bus_usb/devices/2-1.2/devnum               |   1 +
>>   .../sys_bus_usb/devices/2-1.2/idProduct            |   1 +
>>   .../sys_bus_usb/devices/2-1.2/idVendor             |   1 +
>>   .../sys_bus_usb/devices/2-1.2/serial               |   1 +
>>   .../virusbtestdata/sys_bus_usb/devices/2-1/devnum  |   1 +
>>   .../sys_bus_usb/devices/2-1/idProduct              |   1 +
>>   .../sys_bus_usb/devices/2-1/idVendor               |   1 +
>>   .../virusbtestdata/sys_bus_usb/devices/2-1/serial  |   1 +
>>   .../virusbtestdata/sys_bus_usb/devices/usb1/devnum |   1 +
>>   .../sys_bus_usb/devices/usb1/idProduct             |   1 +
>>   .../sys_bus_usb/devices/usb1/idVendor              |   1 +
>>   .../virusbtestdata/sys_bus_usb/devices/usb1/serial |   1 +
>>   .../virusbtestdata/sys_bus_usb/devices/usb2/devnum |   1 +
>>   .../sys_bus_usb/devices/usb2/idProduct             |   1 +
>>   .../sys_bus_usb/devices/usb2/idVendor              |   1 +
>>   .../virusbtestdata/sys_bus_usb/devices/usb2/serial |   1 +
>>   .../virusbtestdata/sys_bus_usb/devices/usb3/devnum |   1 +
>>   .../sys_bus_usb/devices/usb3/idProduct             |   1 +
>>   .../sys_bus_usb/devices/usb3/idVendor              |   1 +
>>   .../virusbtestdata/sys_bus_usb/devices/usb3/serial |   1 +
>>   .../virusbtestdata/sys_bus_usb/devices/usb4/devnum |   1 +
>>   .../sys_bus_usb/devices/usb4/idProduct             |   1 +
>>   .../sys_bus_usb/devices/usb4/idVendor              |   1 +
>>   .../virusbtestdata/sys_bus_usb/devices/usb4/serial |   1 +
>>   64 files changed, 343 insertions(+), 1 deletion(-)
>>   create mode 100644 tests/virusbtest.c
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/devnum
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/idProduct
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/idVendor
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/serial
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/devnum
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/idProduct
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/idVendor
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.3/serial
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/devnum
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/idProduct
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/idVendor
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3/serial
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/devnum
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/idProduct
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/idVendor
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5.4/serial
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/devnum
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/idProduct
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/idVendor
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5.5/serial
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/devnum
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/idProduct
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/idVendor
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5.6/serial
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5/devnum
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5/idProduct
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5/idVendor
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.5/serial
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.6/devnum
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.6/idProduct
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.6/idVendor
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1.6/serial
>>   create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1/devnum
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1/idProduct
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/1-1/idVendor
>>   create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/1-1/serial
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/2-1.2/devnum
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/2-1.2/idProduct
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/2-1.2/idVendor
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/2-1.2/serial
>>   create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/2-1/devnum
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/2-1/idProduct
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/2-1/idVendor
>>   create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/2-1/serial
>>   create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb1/devnum
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/usb1/idProduct
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/usb1/idVendor
>>   create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb1/serial
>>   create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb2/devnum
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/usb2/idProduct
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/usb2/idVendor
>>   create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb2/serial
>>   create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb3/devnum
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/usb3/idProduct
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/usb3/idVendor
>>   create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb3/serial
>>   create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb4/devnum
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/usb4/idProduct
>>   create mode 100644
>> tests/virusbtestdata/sys_bus_usb/devices/usb4/idVendor
>>   create mode 100644 tests/virusbtestdata/sys_bus_usb/devices/usb4/serial
>>
>> diff --git a/.gitignore b/.gitignore
>> index 69c81df..f6dc440 100644
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -218,6 +218,7 @@
>>   /tests/virsystemdtest
>>   /tests/virtimetest
>>   /tests/viruritest
>> +/tests/virusbtest
>>   /tests/vmwarevertest
>>   /tests/vmx2xmltest
>>   /tests/xencapstest
>> diff --git a/cfg.mk b/cfg.mk
>> index fa2638f..ff889b9 100644
>> --- a/cfg.mk
>> +++ b/cfg.mk
>> @@ -941,7 +941,8 @@ exclude_file_name_regexp--sc_bindtextdomain =
>> ^(tests|examples)/
>>   exclude_file_name_regexp--sc_copyright_usage = \
>>     ^COPYING(|\.LESSER)$$
>>
>> -exclude_file_name_regexp--sc_flags_usage =
>> ^(docs/|src/util/virnetdevtap\.c$$|tests/vir(cgroup|pci)mock\.c$$)
>> +exclude_file_name_regexp--sc_flags_usage = \
>> +
>> ^(docs/|src/util/virnetdevtap\.c$$|tests/vir(cgroup|pci)mock\.c$$|tests/virusbtest\.c$$)
>>
>>
>>   exclude_file_name_regexp--sc_libvirt_unmarked_diagnostics = \
>>     ^(src/rpc/gendispatch\.pl$$|tests/)
>> diff --git a/tests/Makefile.am b/tests/Makefile.am
>> index c374f14..3ce1c2c 100644
>> --- a/tests/Makefile.am
>> +++ b/tests/Makefile.am
>> @@ -149,6 +149,7 @@ test_programs = virshtest sockettest \
>>       virkmodtest \
>>       vircapstest \
>>       domainconftest \
>> +    virusbtest \
>>       $(NULL)
>>
>>   if WITH_REMOTE
>> @@ -330,6 +331,7 @@ EXTRA_DIST += $(test_scripts)
>>
>>   test_libraries = libshunload.la \
>>           libvirportallocatormock.la \
>> +        virusbmock.la \
>>           virnetserverclientmock.la \
>>           vircgroupmock.la \
>>           virpcimock.la \
>> @@ -815,6 +817,16 @@ virpcimock_la_LIBADD = $(GNULIB_LIBS) \
>>   virpcimock_la_LDFLAGS = -module -avoid-version \
>>           -rpath /evil/libtool/hack/to/force/shared/lib/creation
>>
>> +virusbtest_SOURCES = \
>> +    virusbtest.c testutils.h testutils.c
>> +virusbtest_LDADD = $(LDADDS)
>> +
>> +virusbmock_la_SOURCES = \
>> +    virusbtest.c
>> +virusbmock_la_CFLAGS = $(AM_CFLAGS) -DMOCK_HELPER=1
>> +virusbmock_la_LDFLAGS = -module -avoid-version \
>> +        -rpath /evil/libtool/hack/to/force/shared/lib/creation
>
> Is there any specific reason why the mock functions are not in a
> separate file? We have that pattern already.
>
>> +
>>   if WITH_DBUS
>>   virdbustest_SOURCES = \
>>       virdbustest.c testutils.h testutils.c
>> diff --git a/tests/virusbtest.c b/tests/virusbtest.c
>> new file mode 100644
>> index 0000000..f9104bf
>> --- /dev/null
>> +++ b/tests/virusbtest.c
>
>> +static int testDeviceFind(const void *opaque)
>> +{
>> +    const struct findTestInfo *info = opaque;
>> +    int ret = -1;
>> +    virUSBDevicePtr dev = NULL;
>> +    virUSBDeviceListPtr devs = NULL;
>> +    int rv = 0;
>> +    size_t i, ndevs = 0;
>> +
>> +    switch (info->how) {
>> +    case FIND_BY_ALL:
>> +        rv = virUSBDeviceFind(info->vendor, info->product,
>> +                              info->bus, info->devno,
>> +                              info->vroot, info->mandatory, &dev);
>> +        break;
>> +    case FIND_BY_VENDOR:
>> +        rv = virUSBDeviceFindByVendor(info->vendor, info->product,
>> +                                      info->vroot, info->mandatory,
>> &devs);
>> +        break;
>> +    case FIND_BY_BUS:
>> +        rv = virUSBDeviceFindByBus(info->bus, info->devno,
>> +                                   info->vroot, info->mandatory, &dev);
>> +        break;
>> +    }
>> +
>> +    if (info->expectFailure) {
>> +        if (rv == 0) {
>> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                           "unexpected success");
>> +            goto cleanup;
>> +        }
>
> Would it make sense to:
>       ret = 0;
>       goto cleanup;
>
> since tesing either an empty device list  or NULL dev doesn't make much
> sense? I know you're calling FileIterate only when there's something to
> call it on, but sill.
>
>> +    } else if (rv < 0) {
>> +        goto cleanup;
>> +    }
>> +
>> +    if (dev && virUSBDeviceFileIterate(dev, testDeviceFileActor,
>> NULL) < 0)
>> +        goto cleanup;
>> +
>> +    if (devs)
>> +        ndevs = virUSBDeviceListCount(devs);
>> +
>> +    for (i = 0; i < ndevs; i++) {
>> +        virUSBDevicePtr device = virUSBDeviceListGet(devs, i);
>> +        if (virUSBDeviceFileIterate(device, testDeviceFileActor,
>> NULL) < 0)
>> +            goto cleanup;
>> +    }
>> +
>> +    ret = 0;
>> +
>> +cleanup:
>> +    virObjectUnref(devs);
>> +    virUSBDeviceFree(dev);
>> +    return ret;
>> +}
>> +
>
>> diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/devnum
>> b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/devnum
>> new file mode 100644
>> index 0000000..98d9bcb
>> --- /dev/null
>> +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/devnum
>> @@ -0,0 +1 @@
>> +17
>> diff --git
>> a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/idProduct
>> b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/idProduct
>> new file mode 100644
>> index 0000000..211990d
>> --- /dev/null
>> +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/idProduct
>> @@ -0,0 +1 @@
>> +301b
>> diff --git
>> a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/idVendor
>> b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/idVendor
>> new file mode 100644
>> index 0000000..489f206
>> --- /dev/null
>> +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/idVendor
>> @@ -0,0 +1 @@
>> +04b3
>> diff --git a/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/serial
>> b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/serial
>> new file mode 100644
>> index 0000000..8b13789
>> --- /dev/null
>> +++ b/tests/virusbtestdata/sys_bus_usb/devices/1-1.5.3.1/serial
>> @@ -0,0 +1 @@
>> +
>
> Why are you creating these /serial files? They don't seem to be used
> anywhere, moreover, they seem to not exists even in real sysfs (at least
> on my system):
>
> # find /sys/bus/usb/ -name serial | wc -l
> 0
>

As usual, my command was wrong. I need to allow find to follow symlinks. 
Sysfs basically consists of symlinks. More accurate command would be:

# find -L /sys/bus/usb/ -name serial -maxdepth 10 2>/dev/null | wc -l
544

Anyway, they are not used anywhere within our code right now. So I don't 
see much point in adding them (esp. when the test passes even after I 
removed them).

Michal




More information about the libvir-list mailing list