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

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