Re: [PATCH 1/2] Add tests for virUSBDeviceFind functions

Michal Privoznik <mprivozn@xxxxxxxxxx> · Thu, 27 Feb 2014 14:54:51 +0100

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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list








