Testbeds for libudev/gudev clients
PATCH 1/1 Testbeds for libudev/gudev clients
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/md0in 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
Signature · 2 lines
Martin Pitt | http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From eebd02e708cc73e08f6ff13b49ac4f99716c6777 Mon Sep 17 00:00:00 2001
From: Martin Pitt <[email protected]>
Date: Thu, 5 Jul 2012 13:34:23 +0200
Subject: [PATCH] udev: Replace compile-time TEST_PREFIX with runtime $UDEV_TEST_PREFIX
The removal of $SYSFS_PATH and configurable paths made it impossible for test suites to set up a local sysfs tree and point libudev to that. Restore and generalize the functionality with checking the $UDEV_TEST_PREFIX environment variable which replaces the compile-time TEST_PREFIX macro.
Also adjust udevadm to respect the test prefix.
--- Makefile.am | 7 +++-- src/libudev/libudev-device-private.c | 11 +++++--- src/libudev/libudev-device.c | 50 ++++++++++++++++++++-------------- src/libudev/libudev-enumerate.c | 19 +++++++++---- src/libudev/libudev-private.h | 4 --- src/libudev/libudev.c | 23 ++++++++++++++++ src/libudev/libudev.h | 1 + src/test/test-udev.c | 2 +- src/udev/udev-event.c | 14 ++++++---- src/udev/udev-node.c | 11 +++++--- src/udev/udev-rules.c | 26 ++++++++++++------ src/udev/udevadm-info.c | 17 +++++++++--- 12 files changed, 125 insertions(+), 60 deletions(-)
Makefile.am +4 -3
@@ -1630,6 +1630,10 @@ TESTS += \ test/udev-test.pl \ test/rules-test.sh +# The test-udev program should run with relative path names for /sys, /dev, +# /run prefixed, pointing to our test/ directory. +TESTS_ENVIRONMENT = UDEV_TEST_PREFIX=test + noinst_PROGRAMS += \ test-libudev \ test-udev
@@ -1647,10 +1651,7 @@ test_udev_SOURCES = \ $(libudev_core_la_SOURCES) \ $(libudev_private_la_SOURCES) -# The test-udev program needs everything compiled with relative path -# names for /sys, /dev, /run prefixed, pointing to our test/ directory. test_udev_CFLAGS = \ - -DTEST_PREFIX=\"test\" \ $(libudev_core_la_CFLAGS) \ $(libudev_private_la_CFLAGS)
src/libudev/libudev-device-private.c +7 -4
@@ -30,7 +30,8 @@ static void udev_device_tag(struct udev_device *dev, const char *tag, bool add) id =udev_device_get_id_filename (dev);if (id ==NULL )return ; -util_strscpyl (filename,sizeof (filename),TEST_PREFIX "/run/udev/tags/" , tag,"/" , id,NULL ); +util_strscpyl (filename,sizeof (filename),udev_get_test_prefix (udev_device_get_udev (dev)), +"/run/udev/tags/" , tag,"/" , id,NULL );if (add) {int fd;
@@ -107,7 +108,8 @@ int udev_device_update_db(struct udev_device *udev_device)return -1 ; has_info =device_has_info (udev_device); -util_strscpyl (filename,sizeof (filename),TEST_PREFIX "/run/udev/data/" , id,NULL ); +util_strscpyl (filename,sizeof (filename),udev_get_test_prefix (udev_device_get_udev (udev_device)), +"/run/udev/data/" , id,NULL );/* do not store anything for otherwise empty devices */ if (!has_info &&
@@ -138,7 +140,7 @@ int udev_device_update_db(struct udev_device *udev_device)if (major (udev_device_get_devnum (udev_device)) >0 ) { udev_list_entry_foreach(list_entry, udev_device_get_devlinks_list_entry(udev_device)) -fprintf (f,"S:%s\n" ,udev_list_entry_get_name (list_entry) +strlen (TEST_PREFIX "/dev/" )); +fprintf (f,"S:%s\n" ,udev_list_entry_get_name (list_entry) +strlen (udev_get_test_prefix (udev)) +strlen ("/dev/" ));if (udev_device_get_devlink_priority (udev_device) !=0 )fprintf (f,"L:%i\n" ,udev_device_get_devlink_priority (udev_device));if (udev_device_get_watch_handle (udev_device) >=0 )
@@ -175,7 +177,8 @@ int udev_device_delete_db(struct udev_device *udev_device) id =udev_device_get_id_filename (udev_device);if (id ==NULL )return -1 ; -util_strscpyl (filename,sizeof (filename),TEST_PREFIX "/run/udev/data/" , id,NULL ); +util_strscpyl (filename,sizeof (filename),udev_get_test_prefix (udev_device_get_udev (udev_device)), +"/run/udev/data/" , id,NULL );unlink (filename);return 0 ; }
src/libudev/libudev-device.c +30 -20
160 lines not on this page · raw message
src/libudev/libudev-enumerate.c +14 -5
@@ -214,7 +214,7 @@ static bool devices_delay_end(struct udev *udev, const char *syspath)int i;for (i =0 ; delay_device_list[i] !=NULL ; i++) { -if (strstr (syspath +strlen ("/sys" ), delay_device_list[i]) !=NULL ) +if (strstr (syspath +strlen (udev_get_test_prefix (udev)) +strlen ("/sys" ), delay_device_list[i]) !=NULL )return true; }return false;
@@ -649,7 +649,8 @@ static int scan_dir_and_add_devices(struct udev_enumerate *udev_enumerate,struct dirent *dent; s = path; - l =util_strpcpyl (&s,sizeof (path),"/sys/" , basedir,NULL ); + l =util_strpcpyl (&s,sizeof (path),udev_get_test_prefix (udev_enumerate_get_udev (udev_enumerate)), +"/sys/" , basedir,NULL );if (subdir1 !=NULL ) l =util_strpcpyl (&s, l,"/" , subdir1,NULL );if (subdir2 !=NULL )
@@ -728,7 +729,8 @@ static int scan_dir(struct udev_enumerate *udev_enumerate, const char *basedir,DIR *dir;struct dirent *dent; -util_strscpyl (path,sizeof (path),"/sys/" , basedir,NULL ); +util_strscpyl (path,sizeof (path),udev_get_test_prefix (udev_enumerate_get_udev (udev_enumerate)), +"/sys/" , basedir,NULL ); dir =opendir (path);if (dir ==NULL )return -1 ;
@@ -875,8 +877,12 @@ static int scan_devices_children(struct udev_enumerate *enumerate)static int scan_devices_all (struct udev_enumerate *udev_enumerate) {struct stat statbuf; +char path[UTIL_PATH_SIZE ]; + +util_strscpyl (path,sizeof (path),udev_get_test_prefix (udev_enumerate_get_udev (udev_enumerate)), +"/sys/subsystem" ,NULL ); -if (stat ("/sys/subsystem" , &statbuf) ==0 ) { +if (stat (path, &statbuf) ==0 ) {/* we have /subsystem/, forget all the old stuff */ scan_dir (udev_enumerate,"subsystem" ,"devices" ,NULL ); }else {
@@ -924,6 +930,7 @@ _public_ int udev_enumerate_scan_subsystems(struct udev_enumerate *udev_enumerat {struct stat statbuf;const char *subsysdir; +char path[UTIL_PATH_SIZE ];if (udev_enumerate ==NULL )return -EINVAL ;
@@ -932,7 +939,9 @@ _public_ int udev_enumerate_scan_subsystems(struct udev_enumerate *udev_enumeratif (match_subsystem (udev_enumerate,"module" ))scan_dir_and_add_devices (udev_enumerate,"module" ,NULL ,NULL ); -if (stat ("/sys/subsystem" , &statbuf) ==0 ) +util_strscpyl (path,sizeof (path),udev_get_test_prefix (udev_enumerate_get_udev (udev_enumerate)), +"/sys/subsystem" ,NULL ); +if (stat (path, &statbuf) ==0 ) subsysdir ="subsystem" ;else subsysdir ="bus" ;
src/libudev/libudev-private.h +0 -4
@@ -25,10 +25,6 @@#define READ_END 0#define WRITE_END 1 -#ifndef TEST_PREFIX -#define TEST_PREFIX "" -#endif -/* avoid (sometimes expensive) calculations of parameters for debug output */ #define udev_log_cond(udev, prio, arg...) \ do { \
src/libudev/libudev.c +23 -0
@@ -43,6 +43,7 @@ struct udev {void *userdata;struct udev_list properties_list;int log_priority; +char *test_prefix; }; void udev_log(struct udev *udev,
@@ -95,6 +96,23 @@ _public_ void udev_set_userdata(struct udev *udev, void *userdata) }/** +* udev_get_test_prefix: +* @udev: udev library context +* +* Retrieve the file system root prefix for accessing /sys, /dev, etc. The +* default is empty, meaning the normal file system root. For testing purposes, +* it can be overridden with the environment variable $UDEV_TEST_PREFIX. +* +* Returns: the test prefix +**/ +_public_ const char *udev_get_test_prefix(struct udev *udev) +{ +if (udev ==NULL ) +return NULL ; +return udev->test_prefix; +} + +/** * udev_new: * * Create udev library context. This reads the udev configuration
@@ -190,6 +208,10 @@ _public_ struct udev *udev_new(void)fclose (f); } +/* test prefix in environment? */ + env =getenv ("UDEV_TEST_PREFIX" ); + udev->test_prefix =strdup (env ? env :"" ); +/* environment overrides config */ env =getenv ("UDEV_LOG" );if (env !=NULL )
@@ -231,6 +253,7 @@ _public_ struct udev *udev_unref(struct udev *udev)if (udev->refcount >0 )return udev;udev_list_cleanup (&udev->properties_list); +free (udev->test_prefix);free (udev);return NULL ; }
src/libudev/libudev.h +1 -0
@@ -38,6 +38,7 @@ int udev_get_log_priority(struct udev *udev);void udev_set_log_priority (struct udev *udev,int priority);void *udev_get_userdata (struct udev *udev);void udev_set_userdata (struct udev *udev,void *userdata); +const char *udev_get_test_prefix (struct udev *udev);/* * udev_list
src/test/test-udev.c +1 -1
@@ -68,7 +68,7 @@ int main(int argc, char *argv[]) rules =udev_rules_new (udev,1 ); -util_strscpyl (syspath,sizeof (syspath),TEST_PREFIX "/sys" , devpath,NULL ); +util_strscpyl (syspath,sizeof (syspath),udev_get_test_prefix (udev) ,"/sys" , devpath,NULL ); dev =udev_device_new_from_syspath (udev, syspath);if (dev ==NULL ) {log_debug ("unknown device '%s'\n" , devpath);
src/udev/udev-event.c +8 -6
@@ -112,6 +112,8 @@ size_t udev_event_apply_format(struct udev_event *event, const char *src, char *const char *from;char *s;size_t l; +const char *prefix =udev_get_test_prefix (udev_device_get_udev (dev)); +const int devpath_len =strlen (prefix) +strlen ("/dev/" ); from = src; s = dest;
@@ -319,7 +321,7 @@ subst:break ; devnode =udev_device_get_devnode (dev_parent);if (devnode !=NULL ) - l =util_strpcpy (&s, l, devnode +strlen (TEST_PREFIX "/dev/" )); + l =util_strpcpy (&s, l, devnode + devpath_len);break ; }case SUBST_DEVNODE :
@@ -330,7 +332,7 @@ subst:if (event->name !=NULL ) l =util_strpcpy (&s, l, event->name);else if (udev_device_get_devnode (dev) !=NULL ) - l =util_strpcpy (&s, l,udev_device_get_devnode (dev) +strlen (TEST_PREFIX "/dev/" )); + l =util_strpcpy (&s, l,udev_device_get_devnode (dev) + devpath_len);else l =util_strpcpy (&s, l,udev_device_get_sysname (dev));break ;
@@ -340,16 +342,16 @@ subst: list_entry =udev_device_get_devlinks_list_entry (dev);if (list_entry ==NULL )break ; - l =util_strpcpy (&s, l,udev_list_entry_get_name (list_entry) +strlen (TEST_PREFIX "/dev/" )); + l =util_strpcpy (&s, l,udev_list_entry_get_name (list_entry) + devpath_len);udev_list_entry_foreach (list_entry,udev_list_entry_get_next (list_entry)) - l = util_strpcpyl(&s, l, " ", udev_list_entry_get_name(list_entry) + strlen(TEST_PREFIX "/dev/"), NULL); + l = util_strpcpyl(&s, l, " ", udev_list_entry_get_name(list_entry) + devpath_len, NULL);break ; }case SUBST_ROOT : - l =util_strpcpy (&s, l,TEST_PREFIX "/dev" ); + l =util_strpcpyl (&s, l, prefix,"/dev" ,NULL );break ;case SUBST_SYS : - l =util_strpcpy (&s, l,TEST_PREFIX "/sys" ); + l =util_strpcpyl (&s, l, prefix,"/sys" ,NULL );break ;case SUBST_ENV :if (attr ==NULL ) {
src/udev/udev-node.c +7 -4
@@ -202,9 +202,10 @@ static void link_update(struct udev_device *dev, const char *slink, bool add)char dirname[UTIL_PATH_SIZE ];const char *target;char buf[UTIL_PATH_SIZE ]; +const char *prefix =udev_get_test_prefix (udev_device_get_udev (dev)); -util_path_encode (slink +strlen (TEST_PREFIX "/dev" ), name_enc,sizeof (name_enc)); -util_strscpyl (dirname,sizeof (dirname),TEST_PREFIX "/run/udev/links/" , name_enc,NULL ); +util_path_encode (slink +strlen (prefix) +strlen ("/dev" ), name_enc,sizeof (name_enc)); +util_strscpyl (dirname,sizeof (dirname), prefix,"/run/udev/links/" , name_enc,NULL );util_strscpyl (filename,sizeof (filename), dirname,"/" ,udev_device_get_id_filename (dev),NULL );if (!add &&unlink (filename) ==0 )
@@ -328,7 +329,8 @@ void udev_node_add(struct udev_device *dev, mode_t mode, uid_t uid, gid_t gid)return ;/* always add /dev/{block,char}/$major:$minor */ -snprintf (filename,sizeof (filename),TEST_PREFIX "/dev/%s/%u:%u" , +snprintf (filename,sizeof (filename),"%s/dev/%s/%u:%u" , +udev_get_test_prefix (udev),strcmp (udev_device_get_subsystem (dev),"block" ) ==0 ?"block" :"char" ,major (udev_device_get_devnum (dev)),minor (udev_device_get_devnum (dev)));node_symlink (udev,udev_device_get_devnode (dev), filename);
@@ -353,7 +355,8 @@ void udev_node_remove(struct udev_device *dev)link_update (dev,udev_list_entry_get_name (list_entry),0 );/* remove /dev/{block,char}/$major:$minor */ -snprintf (filename,sizeof (filename),TEST_PREFIX "/dev/%s/%u:%u" , +snprintf (filename,sizeof (filename),"%s/dev/%s/%u:%u" , +udev_get_test_prefix (udev_device_get_udev (dev)),strcmp (udev_device_get_subsystem (dev),"block" ) ==0 ?"block" :"char" ,major (udev_device_get_devnum (dev)),minor (udev_device_get_devnum (dev)));unlink (filename);
src/udev/udev-rules.c +17 -9
@@ -1718,6 +1718,9 @@ struct udev_rules *udev_rules_new(struct udev *udev, int resolve_names)struct token end_token;char **files, **f;int r; +char sysconf_path[UTIL_PATH_SIZE ]; +char run_path[UTIL_PATH_SIZE ]; +char libexec_path[UTIL_PATH_SIZE ]; rules =calloc (1 ,sizeof (struct udev_rules ));if (rules ==NULL )
@@ -1757,10 +1760,14 @@ struct udev_rules *udev_rules_new(struct udev *udev, int resolve_names)memset (rules->trie_nodes,0x00 ,sizeof (struct trie_node )); rules->trie_nodes_cur =1 ; - rules->dirs = strv_new(TEST_PREFIX SYSCONFDIR "/udev/rules.d", -TEST_PREFIX "/run/udev/rules.d" , - TEST_PREFIX UDEVLIBEXECDIR "/rules.d", -NULL ); +util_strscpyl (sysconf_path,sizeof (sysconf_path),udev_get_test_prefix (udev), +SYSCONFDIR "/udev/rules.d" ,NULL ); +util_strscpyl (run_path,sizeof (run_path),udev_get_test_prefix (udev), +"/run/udev/rules.d" ,NULL ); +util_strscpyl (libexec_path,sizeof (libexec_path),udev_get_test_prefix (udev), +UDEVLIBEXECDIR "/rules.d" ,NULL ); + rules->dirs =strv_new (sysconf_path, run_path, libexec_path,NULL ); +if (!rules->dirs) {log_error ("failed to build config directory array" );return NULL ;
@@ -2018,6 +2025,7 @@ int udev_rules_apply_to_event(struct udev_rules *rules, struct udev_event *eventstruct token *rule;enum escape_type esc =ESCAPE_UNSET ;bool can_set_name; +const char *prefix =udev_get_test_prefix (event->udev);if (rules->tokens ==NULL )return -1 ;
@@ -2059,7 +2067,7 @@ int udev_rules_apply_to_event(struct udev_rules *rules, struct udev_event *event udev_list_entry_foreach(list_entry, udev_device_get_devlinks_list_entry(event->dev)) {const char *devlink; - devlink =udev_list_entry_get_name (list_entry) +strlen (TEST_PREFIX "/dev/" ); + devlink =udev_list_entry_get_name (list_entry) +strlen (prefix) +strlen ("/dev/" );if (match_key (rules, cur, devlink) ==0 ) { match = true;break ;
@@ -2533,7 +2541,7 @@ int udev_rules_apply_to_event(struct udev_rules *rules, struct udev_event *eventlog_debug ("%i character(s) replaced\n" , count); }if (major (udev_device_get_devnum (event->dev)) && - (!streq (name_str,udev_device_get_devnode (event->dev) +strlen (TEST_PREFIX "/dev/" )))) { + (!streq (name_str,udev_device_get_devnode (event->dev) +strlen (prefix) +strlen ("/dev/" )))) {log_error ("NAME=\"%s\" ignored, kernel device nodes " "can not be renamed; please fix it in %s:%u\n" , name, &rules->buf[rule->rule.filename_off], rule->rule.filename_line);
@@ -2578,7 +2586,7 @@ int udev_rules_apply_to_event(struct udev_rules *rules, struct udev_event *event next[0 ] ='\0' ;log_debug ("LINK '%s' %s:%u\n" , pos, &rules->buf[rule->rule.filename_off], rule->rule.filename_line); -util_strscpyl (filename,sizeof (filename),TEST_PREFIX "/dev/" , pos,NULL ); +util_strscpyl (filename,sizeof (filename), prefix,"/dev/" , pos,NULL );udev_device_add_devlink (event->dev, filename, cur->key.devlink_unique);while (isspace (next[1 ])) next++;
@@ -2588,7 +2596,7 @@ int udev_rules_apply_to_event(struct udev_rules *rules, struct udev_event *eventif (pos[0 ] !='\0' ) {log_debug ("LINK '%s' %s:%u\n" , pos, &rules->buf[rule->rule.filename_off], rule->rule.filename_line); -util_strscpyl (filename,sizeof (filename),TEST_PREFIX "/dev/" , pos,NULL ); +util_strscpyl (filename,sizeof (filename), prefix,"/dev/" , pos,NULL );udev_device_add_devlink (event->dev, filename, cur->key.devlink_unique); }break ;
@@ -2698,7 +2706,7 @@ void udev_rules_apply_static_dev_perms(struct udev_rules *rules)/* we assure, that the permissions tokens are sorted before the static token */ if (mode ==0 && uid ==0 && gid ==0 ) goto next; -util_strscpyl (filename,sizeof (filename),TEST_PREFIX "/dev/" , +util_strscpyl (filename,sizeof (filename),udev_get_test_prefix (rules->udev),"/dev/" , &rules->buf[cur->key.value_off],NULL );if (stat (filename, &stats) !=0 ) goto next;
src/udev/udevadm-info.c +13 -4
@@ -267,13 +267,22 @@ static void cleanup_db(struct udev *udev)static struct udev_device *find_device (struct udev *udev,const char *id,const char *prefix) {char name[UTIL_PATH_SIZE ]; +char fullprefix[UTIL_PATH_SIZE ]; +char devdir[UTIL_PATH_SIZE ]; +char sysdir[UTIL_PATH_SIZE ]; -if (prefix && !startswith (id, prefix)) { -util_strscpyl (name,sizeof (name), prefix, id,NULL ); +if (prefix) +util_strscpyl (fullprefix,sizeof (fullprefix),udev_get_test_prefix (udev), prefix,NULL ); + +if (prefix && !startswith (id, fullprefix)) { +util_strscpyl (name,sizeof (name), fullprefix, id,NULL ); id = name; } -if (startswith (id,"/dev/" )) { +util_strscpyl (devdir,sizeof (devdir),udev_get_test_prefix (udev),"/dev/" ,NULL ); +util_strscpyl (sysdir,sizeof (sysdir),udev_get_test_prefix (udev),"/sys/" ,NULL ); + +if (startswith (id, devdir)) {struct stat statbuf;char type;
@@ -288,7 +297,7 @@ static struct udev_device *find_device(struct udev *udev, const char *id, constreturn NULL ;return udev_device_new_from_devnum (udev, type, statbuf.st_rdev); - }else if (startswith (id,"/sys/" )) + }else if (startswith (id, sysdir))return udev_device_new_from_syspath (udev, id);else return NULL ;
Signature · 1 line
1.7.10.4
Martin Pitt [2012-07-06 7:01 +0200]:
Quoted 8 lines
* 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
Signature · 2 lines
Martin Pitt | http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
From d8e3de2529b956ac3fc78640ca179fde6e909150 Mon Sep 17 00:00:00 2001
From: Martin Pitt <[email protected]>
Date: Fri, 6 Jul 2012 13:27:51 +0200
Subject: [PATCH 2/2] gudev: Add GUdevTestbed
The GUdevTestbed class is used to build a temporary sysfs file system. You can add a number of devices including arbitrary sysfs attributes and udev properties, and then run a gudev client in that test bed that is independent of the actual hardware it is running on. With this you can simulate particular hardware in virtual environments up to some degree (e. g. sysctls will fail).
Also add a test-gudev check which tests both GUDev itself and GUdevTestbed.
--- .gitignore | 1 + Makefile.am | 25 ++- docs/gudev/gudev-docs.xml | 1 + docs/gudev/gudev-sections.txt | 24 +++ src/gudev/gudev.h | 1 + src/gudev/gudevtestbed.c | 448 +++++++++++++++++++++++++++++++++++++++++ src/gudev/gudevtestbed.h | 103 ++++++++++ src/gudev/gudevtypes.h | 1 + src/test/test-gudev.c | 248 +++++++++++++++++++++++ 9 files changed, 851 insertions(+), 1 deletion(-) create mode 100644 src/gudev/gudevtestbed.c create mode 100644 src/gudev/gudevtestbed.h create mode 100644 src/test/test-gudev.c
.gitignore +1 -0
@@ -123,3 +123,4 @@ stamp-* /v4l_id /test-libudev /test-udev +/test-gudev
Makefile.am +24 -1
@@ -1802,6 +1802,7 @@ libgudev_include_HEADERS = \ src/gudev/gudevtypes.h \ src/gudev/gudevclient.h \ src/gudev/gudevdevice.h \ + src/gudev/gudevtestbed.h \ src/gudev/gudevenumerator.h lib_LTLIBRARIES += libgudev-1.0.la
@@ -1826,6 +1827,8 @@ libgudev_1_0_la_SOURCES = \ src/gudev/gudevdevice.c \ src/gudev/gudevenumerator.h \ src/gudev/gudevenumerator.c \ + src/gudev/gudevtestbed.h \ + src/gudev/gudevtestbed.c \ src/gudev/gudevprivate.h nodist_libgudev_1_0_la_SOURCES = \
@@ -1920,7 +1923,10 @@ src_gudev_GUdev_1_0_gir_FILES = \ $(top_srcdir)/src/gudev/gudevenumerator.h \ $(top_srcdir)/src/gudev/gudevclient.c \ $(top_srcdir)/src/gudev/gudevdevice.c \ - $(top_srcdir)/src/gudev/gudevenumerator.c + $(top_srcdir)/src/gudev/gudevenumerator.c \ + $(top_srcdir)/src/gudev/gudevtestbed.h \ + $(top_srcdir)/src/gudev/gudevtestbed.c \ + $(NULL) INTROSPECTION_GIRS = src/gudev/GUdev-1.0.gir INTROSPECTION_SCANNER_ARGS = --c-include=gudev/gudev.h
@@ -1949,6 +1955,23 @@ libgudev-install-move-hook: libgudev-uninstall-move-hook: rm -f $(DESTDIR)$(rootlibdir)/libgudev-1.0.so* +noinst_PROGRAMS += \ + test-gudev + +test_gudev_SOURCES = \ + src/test/test-gudev.c + +test_gudev_CFLAGS = \ + $(GLIB_CFLAGS) + +test_gudev_LDADD = \ + libgudev-1.0.la \ + $(GLIB_LIBS) + +TESTS += \ + test-gudev + + INSTALL_EXEC_HOOKS += libgudev-install-move-hook UNINSTALL_EXEC_HOOKS += libgudev-uninstall-move-hook endif
docs/gudev/gudev-docs.xml +1 -0
@@ -26,6 +26,7 @@ <xi:include href="xml/gudevclient.xml"/> <xi:include href="xml/gudevdevice.xml"/> <xi:include href="xml/gudevenumerator.xml"/> + <xi:include href="xml/gudevtestbed.xml"/> </chapter> <chapter id="gudev-hierarchy">
docs/gudev/gudev-sections.txt +24 -0
@@ -98,3 +98,27 @@ G_UDEV_ENUMERATOR_GET_CLASS <SUBSECTION Private> GUdevEnumeratorPrivate </SECTION> + +<SECTION> +<FILE>gudevtestbed</FILE> +<TITLE>GUdevTestbed</TITLE> +GUdevTestbed +GUdevTestbedClass +g_udev_testbed_new +g_udev_testbed_add_device +g_udev_testbed_add_devicev +g_udev_testbed_get_root_dir +g_udev_testbed_get_sys_dir +g_udev_testbed_set_attribute +g_udev_testbed_set_property +<SUBSECTION Standard> +g_udev_testbed_get_type +G_UDEV_IS_TESTBED +G_UDEV_IS_TESTBED_CLASS +G_UDEV_TESTBED +G_UDEV_TESTBED_CLASS +G_UDEV_TESTBED_GET_CLASS +G_UDEV_TYPE_TESTBED +<SUBSECTION Private> +GUdevTestbedPrivate +</SECTION>
src/gudev/gudev.h +1 -0
@@ -28,6 +28,7 @@#include <gudev/gudevclient.h> #include <gudev/gudevdevice.h> #include <gudev/gudevenumerator.h> +#include <gudev/gudevtestbed.h> #undef _GUDEV_INSIDE_GUDEV_H#endif /* __G_UDEV_H__ */
src/gudev/gudevtestbed.c new file +448 -0
448 lines not on this page · raw message
src/gudev/gudevtestbed.h new file +103 -0
103 lines not on this page · raw message
src/gudev/gudevtypes.h +1 -0
@@ -33,6 +33,7 @@ G_BEGIN_DECLStypedef struct _GUdevClient GUdevClient ;typedef struct _GUdevDevice GUdevDevice ;typedef struct _GUdevEnumerator GUdevEnumerator ; +typedef struct _GUdevTestbed GUdevTestbed ;/** * GUdevDeviceNumber:
src/test/test-gudev.c new file +248 -0
248 lines not on this page · raw message
Signature · 1 line
1.7.10.4
Quoted 10 lines · Martin Pitt
Martin Pitt [2012-07-06 7:01 +0200]:
Quoted 8 lines
* 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.
Quoted 7 lines
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
Kay Sievers [2012-07-09 15:50 +0200]:
Quoted 3 lines
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
Signature · 2 lines
Martin Pitt | http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
Quoted 10 lines · Martin Pitt
Kay Sievers [2012-07-09 15:50 +0200]:
Quoted 3 lines
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.
Quoted 5 lines
Another alternative I can think of that avoids root privs is a fakechroot like
LD_PRELOADwrapper 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
Hello Lucas,
Lucas De Marchi [2012-07-09 16:54 -0300]:
Quoted 3 lines
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.
Quoted 3 lines
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
Signature · 2 lines
Martin Pitt | http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
Quoted 11 lines · Martin Pitt
Hello Lucas,
Lucas De Marchi [2012-07-09 16:54 -0300]:
Quoted 3 lines
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.
Quoted 5 lines
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.
Quoted 5 lines
Quoted 4 lines
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.
Quoted 7 lines
libudev already has something like that, it has the
TEST_PREFIXmacro everywhere. So I don't think it's actually getting much worse, but with a preloaded library we wouldn't need theTEST_PREFIXthing 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
Lucas De Marchi [2012-07-09 17:21 -0300]:
Quoted 6 lines
Quoted 2 lines
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
Signature · 2 lines
Martin Pitt | http://www.piware.de Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)