Discussion:
[PATCH] Testbeds for libudev/gudev clients
Martin Pitt
2012-07-06 05:01:31 UTC
Permalink
Hello friends of udev,

I would like to make it vastly easier, and for some aspects possible
at all, to write automatic tests for libudev/gudev client software.
For example, upower's test suite builds a temporary mock sysfs tree:

http://cgit.freedesktop.org/upower/tree/src/linux/integration-test

(line 106 ff.) so that we can test its logic with having various
combinations of empty or full batteries, UPSes, ACs, power supplies
that power the whole platform vs. only particular devices, and so on.
Often developers do not have that hardware, or it's not easy to set up
a situation to reproduce a bug (waiting a couple of hours for your
battery to drain, etc.), so it is much more convenient to reproduce
the sysfs attributes in a small test bed.

Another example is the test suite for Ubuntu's driver package
detection which implements the corresponding PackageKit API; it also
uses a "fake sys" module largely derived from upower's:

https://github.com/tseliot/ubuntu-drivers-common/blob/master/tests/fakesysfs.py

There are several problems with these approaches:

* Copy&paste code is obviously bad. It would be much better if the
convenience API to set up a libudev/gudev test bed would be
provided by libudev/gudev itself. Also, it should be in C and
available through introspection, so that you can use it from a
variety of languages; this means it should be in gudev.

I am currently working on a first patch for this, and will post it
for discussion here when I have something working.

* upower's test suite is not working any more with current
systemd/udev versions because the support for $SYSFS_PATH was
removed.

I have a patch attached to generalize this to use
$UDEV_TEST_PREFIX, replacing the current compile-time TEST_PREFIX
macro. This will also allow us in the future to create mock dev and
run/rules.d files without having to change the environment variable
and temporary directory structure again. Once that (or a variant of
it) is in, I'll adapt upower's test suite to also set
UDEV_TEST_PREFIX so that it works with both old and new libudev
versions.

With that patch you can now e. g. call

UDEV_TEST_PREFIX=test ./udevadm info --export-db
UDEV_TEST_PREFIX=test ./udevadm info --query=all --path=/devices/virtual/block/md0

in the built systemd tree and it will DTRT. I tested with
./test-libudev, which also works fine.

* You can currently only control the coldplug part. That's why
upower's test suite starts the daemon again in each test case. But
you cannot test the dynamic parts with that, such as updating the
properties correctly when changing power sources. What's missing is
to emulate corresponding uevents for the $UDEV_TEST_PREFIX tree.

My initial idea about this was to make udev_monitor_send_device()
public API and relax the uid/pid checks to allow non-root messages
when the corresponding sysfs dir is writable for the calling user
(or something like that). But as non-root users are not allowed to
send NETLINK_KOBJECT_UEVENT messages, this is moot anyway as we do
not want to require root privileges for tests, at least not in all
cases.

So my current idea about this to have gudev read events from a
simple GIO socket in $UDEV_TEST_PREFIX/uevent instead from
libudev_monitor_* if $UDEV_TEST_PREFIX is given, as in that case
the real uevents won't coincide with the sysfs anyway. The "uevent"
signal would then be raised on messages from that socket. Do you
think that's too crazy?

Thanks,

Martin
--
Martin Pitt | http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
Martin Pitt
2012-07-09 07:25:21 UTC
Permalink
Post by Martin Pitt
* Copy&paste code is obviously bad. It would be much better if the
convenience API to set up a libudev/gudev test bed would be
provided by libudev/gudev itself. Also, it should be in C and
available through introspection, so that you can use it from a
variety of languages; this means it should be in gudev.
I am currently working on a first patch for this, and will post it
for discussion here when I have something working.
I have that working now. The remaining TODO is to remove the temporary
tree again in the destructor (if only GLib had a function for that..)
This provides a simple to use API for building a sandbox with mock
devices, including a new automatic test (covering the sandbox as well
as parts of gudev itself), and gtk-doc.

What do you think about that?

Thanks,

Martin
--
Martin Pitt | http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
Kay Sievers
2012-07-09 13:50:44 UTC
Permalink
Post by Martin Pitt
Post by Martin Pitt
* Copy&paste code is obviously bad. It would be much better if the
convenience API to set up a libudev/gudev test bed would be
provided by libudev/gudev itself. Also, it should be in C and
available through introspection, so that you can use it from a
variety of languages; this means it should be in gudev.
I am currently working on a first patch for this, and will post it
for discussion here when I have something working.
I have that working now. The remaining TODO is to remove the temporary
tree again in the destructor (if only GLib had a function for that..)
This provides a simple to use API for building a sandbox with mock
devices, including a new automatic test (covering the sandbox as well
as parts of gudev itself), and gtk-doc.
What do you think about that?
Urks, the patch isn't really pretty. It was actually nice to get rid
of all these otherwise totally useless knobs.

Can't the tests just use a fs namespace and run the test in it with a
bind-mount of /sys in it? Maybe check the unshare(1) tool to get the
idea, or it might already be able to do that.

If that works, it would not require any patching of tools and could
possibly offer even more useful options for testing in general.

Thanks,
Kay
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Martin Pitt
2012-07-09 14:22:33 UTC
Permalink
Post by Kay Sievers
Can't the tests just use a fs namespace and run the test in it with a
bind-mount of /sys in it? Maybe check the unshare(1) tool to get the
idea, or it might already be able to do that.
Most alternatives would require root privileges, which make tests a
lot less useful: You cannot run them in environments like jhbuild or
"make distcheck", and are also both inconvenient and potentially
ruining your system when you run them during development.

Another alternative I can think of that avoids root privs is a
fakechroot like LD_PRELOAD wrapper which intercepts all gazillion
variants of open and stat-like calls and redirects them to the
testbed. But given how fakechroot breaks with every other new glibc
release this is not something I'm very keen to do.

The third option that comes to my mind is to change the build system
to build a libudev-test.so with a hardcoded TEST_PREFIX of '.', so
that upower and friends can preload/link to that instead of the real
udev, and run the daemon in the directory of the test bed. Daemons
must not chdir() then (chdir('/') actually used to be best practice
for daemons), but that would not be a too hard limitation.

Other ideas greatly appreciated.

If you veto all those, we'll need to live with tests needing root
privileges. It still allows continuous integration test servers to run
stuff in VMs, but that's a lot harder for developers to set up.

Thanks,

Martin
--
Martin Pitt | http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Lucas De Marchi
2012-07-09 19:54:25 UTC
Permalink
Post by Martin Pitt
Post by Kay Sievers
Can't the tests just use a fs namespace and run the test in it with a
bind-mount of /sys in it? Maybe check the unshare(1) tool to get the
idea, or it might already be able to do that.
Most alternatives would require root privileges, which make tests a
lot less useful: You cannot run them in environments like jhbuild or
"make distcheck", and are also both inconvenient and potentially
ruining your system when you run them during development.
Another alternative I can think of that avoids root privs is a
fakechroot like LD_PRELOAD wrapper which intercepts all gazillion
variants of open and stat-like calls and redirects them to the
testbed. But given how fakechroot breaks with every other new glibc
release this is not something I'm very keen to do.
You might want to look into kmod's testsuite. We do exactly that and
until there's a better alternative I plan to support this for newer
glibcs. There's a path.so that you can copy and paste to your project
- there are even other traps like the ones to uname(), init_module(),
delete_module() etc.

When I implemented that I tried what you are trying now and it didn't
look right and it's ugly to touch all the calls with path strings and
also very error prone.

It's not perfect, it could be simplified I think, but it doesn't touch
any libkmod/tools code like your approach, it doesn't require root
privileges and it's working quite well for all needs of kmod until
now, which I think are more than the ones for testing udev.

Lucas De Marchi
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Martin Pitt
2012-07-09 20:09:04 UTC
Permalink
Hello Lucas,
Post by Lucas De Marchi
You might want to look into kmod's testsuite. We do exactly that and
until there's a better alternative I plan to support this for newer
glibcs.
Thanks for pointing out! I'll do that. When it fails with a newer
glibc, we should get test case failures and thus it should be rather
obvious where things need fixing.

I guess that still means we'd either need a libudev-test.so or a shell
wrapper and the libpath.so thing around all tests, and thus
build/ship the two as part of a libudev install. That's something
which I considered to be more ugly, but if Kay prefers that, I'll look
into this.
Post by Lucas De Marchi
When I implemented that I tried what you are trying now and it didn't
look right and it's ugly to touch all the calls with path strings and
also very error prone.
libudev already has something like that, it has the TEST_PREFIX macro
everywhere. So I don't think it's actually getting much worse, but
with a preloaded library we wouldn't need the TEST_PREFIX thing any
more either.

So the trade is "add the get_prefix() calls to the path name build
calls" vs. "maintain/build/install a preload library".

Thanks,

Martin
--
Martin Pitt | http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Lucas De Marchi
2012-07-09 20:21:22 UTC
Permalink
Post by Martin Pitt
Hello Lucas,
Post by Lucas De Marchi
You might want to look into kmod's testsuite. We do exactly that and
until there's a better alternative I plan to support this for newer
glibcs.
Thanks for pointing out! I'll do that. When it fails with a newer
glibc, we should get test case failures and thus it should be rather
obvious where things need fixing.
I guess that still means we'd either need a libudev-test.so or a shell
wrapper and the libpath.so thing around all tests, and thus
build/ship the two as part of a libudev install. That's something
which I considered to be more ugly, but if Kay prefers that, I'll look
into this.
It's part of my "make check" running the testsuite.
Post by Martin Pitt
Post by Lucas De Marchi
When I implemented that I tried what you are trying now and it didn't
look right and it's ugly to touch all the calls with path strings and
also very error prone.
libudev already has something like that, it has the TEST_PREFIX macro
everywhere. So I don't think it's actually getting much worse, but
with a preloaded library we wouldn't need the TEST_PREFIX thing any
more either.
So the trade is "add the get_prefix() calls to the path name build
calls" vs. "maintain/build/install a preload library".
I never ever install that, and doing so is just wrong IMO. Just make
it part of the "make check". The final user is not supposed to be
running a testsuite.


Lucas De Marchi
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Martin Pitt
2012-07-10 03:40:04 UTC
Permalink
Post by Lucas De Marchi
Post by Martin Pitt
So the trade is "add the get_prefix() calls to the path name build
calls" vs. "maintain/build/install a preload library".
I never ever install that, and doing so is just wrong IMO. Just make
it part of the "make check". The final user is not supposed to be
running a testsuite.
This is not (primarily) for testing libudev/gudev itself. The whole
point of this is to make the functionality available to users of
libudev/gudev so that you can write tests for _those_ easily. I
already pointed out two existing examples (upower and
ubuntu-drivers-common) in my initial mail. This is also why I wrote
that entire GUdevTestbed; testing gudev itself would be a lot simpler,
but that's not the reason why we need it.

So we do have to ship it, and document how upower and friends have to
use it (presumably an "udevtestbed" wrapper script which finds and
preloads the test library).

Thanks,

Martin
--
Martin Pitt | http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Martin Pitt
2012-07-10 09:11:30 UTC
Permalink
Hello again,
Post by Martin Pitt
I have a patch attached to generalize this to use
$UDEV_TEST_PREFIX, replacing the current compile-time TEST_PREFIX
macro. This will also allow us in the future to create mock dev and
run/rules.d files without having to change the environment variable
and temporary directory structure again. Once that (or a variant of
it) is in, I'll adapt upower's test suite to also set
UDEV_TEST_PREFIX so that it works with both old and new libudev
versions.
As the original patch that changed libudev to respect
$UDEV_TEST_PREFIX was not very palatable, I now changed the approach
to use an external LD_PRELOAD wrapper, as discussed with Lucas. This
is the "no privileges" and "do not do any permanent changes to the
system" variant of Kay's namespacing proposal, and does not touch the
existing code at all. I based libudev-testbed.so on kmod's path.c,
simplified it a bit, added some missing functions, and integrated it
into the build system together with a simple "libudev-testbed" wrapper
script that you need to run test suites under.

I actually like this approach, as it allows us to extend the
functionality later on. E. g. this can easily intercept access to
/dev/ and sysctls as well.
Post by Martin Pitt
With that patch you can now e. g. call
UDEV_TEST_PREFIX=test ./udevadm info --export-db
UDEV_TEST_PREFIX=test ./udevadm info --query=all --path=/devices/virtual/block/md0
This now becomes

LD_LIBRARY_PATH=.libs UDEV_TEST_PREFIX=test src/test/libudev-testbed ./udevadm info --export-db

The LD_LIBRARY_PATH is just for the build tree; when using the
installed version (e. g. upower's test suite), you would use

UDEV_TEST_PREFIX=/path/to/testbed libudev-testbed /udevadm info --export-db

I also adjusted the GUdevTestbed patch a bit for the new approach.

TODOs that I am aware of which need to be fixed before committing:

- libudev-testbed needs a manpage
- GUdevTestbed needs to remove its temporary directory in the
destructor
- Write some documentation how to use this stuff (in
libudev-testbed's manpage, and some general "how to write tests for
hardware handling" wiki page on fd.o)

I'll do these once we have a general agreement about the approach in
these patches.

Thanks,

Martin
--
Martin Pitt | http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
Loading...