From c745d2bfa00c9732de21a5cf157659b93e9fbec5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 20 Sep 2018 14:25:36 +0200 Subject: [PATCH 1/4] basic/mount-util: reword debug message "because blacklisted" is not grammatical, it should be something like "becuase it is blacklisted", which in turn in too verbose. Let's use a shorter form. --- src/basic/mount-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/mount-util.c b/src/basic/mount-util.c index ae62d3090e..ddf00addff 100644 --- a/src/basic/mount-util.c +++ b/src/basic/mount-util.c @@ -471,7 +471,7 @@ int bind_remount_recursive_with_mountinfo(const char *prefix, bool ro, char **bl if (path_startswith(p, *i)) { blacklisted = true; - log_debug("Not remounting %s, because blacklisted by %s, called for %s", p, *i, cleaned); + log_debug("Not remounting %s blacklisted by %s, called for %s", p, *i, cleaned); break; } } From 465815aaa7e49998e6b71dd56e4b4efc846fe823 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 20 Sep 2018 13:27:04 +0200 Subject: [PATCH 2/4] test-udev: use _cleanup_ and modernize style --- src/test/test-udev.c | 46 +++++++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/src/test/test-udev.c b/src/test/test-udev.c index 0973b2dfe7..03cd99f000 100644 --- a/src/test/test-udev.c +++ b/src/test/test-udev.c @@ -17,6 +17,7 @@ #include "selinux-util.h" #include "signal-util.h" #include "string-util.h" +#include "tests.h" #include "udev.h" static int fake_filesystems(void) { @@ -26,27 +27,26 @@ static int fake_filesystems(void) { const char *error; bool ignore_mount_error; } fakefss[] = { - { "test/tmpfs/sys", "/sys", "failed to mount test /sys", false }, - { "test/tmpfs/dev", "/dev", "failed to mount test /dev", false }, - { "test/run", "/run", "failed to mount test /run", false }, - { "test/run", "/etc/udev/rules.d", "failed to mount empty /etc/udev/rules.d", true }, - { "test/run", UDEVLIBEXECDIR "/rules.d", "failed to mount empty " UDEVLIBEXECDIR "/rules.d", true }, + { "test/tmpfs/sys", "/sys", "Failed to mount test /sys", false }, + { "test/tmpfs/dev", "/dev", "Failed to mount test /dev", false }, + { "test/run", "/run", "Failed to mount test /run", false }, + { "test/run", "/etc/udev/rules.d", "Failed to mount empty /etc/udev/rules.d", true }, + { "test/run", UDEVLIBEXECDIR "/rules.d", "Failed to mount empty " UDEVLIBEXECDIR "/rules.d", true }, }; - unsigned int i; + unsigned i; if (unshare(CLONE_NEWNS) < 0) - return log_error_errno(errno, "failed to call unshare(): %m"); + return log_error_errno(errno, "Failed to call unshare(): %m"); if (mount(NULL, "/", NULL, MS_SLAVE|MS_REC, NULL) < 0) - return log_error_errno(errno, "failed to mount / as private: %m"); + return log_error_errno(errno, "Failed to mount / as private: %m"); - for (i = 0; i < ELEMENTSOF(fakefss); i++) { + for (i = 0; i < ELEMENTSOF(fakefss); i++) if (mount(fakefss[i].src, fakefss[i].target, NULL, MS_BIND, NULL) < 0) { log_full_errno(fakefss[i].ignore_mount_error ? LOG_DEBUG : LOG_ERR, errno, "%s: %m", fakefss[i].error); if (!fakefss[i].ignore_mount_error) return -errno; } - } return 0; } @@ -55,48 +55,46 @@ int main(int argc, char *argv[]) { _cleanup_(udev_event_unrefp) struct udev_event *event = NULL; _cleanup_(udev_device_unrefp) struct udev_device *dev = NULL; _cleanup_(udev_rules_unrefp) struct udev_rules *rules = NULL; - char syspath[UTIL_PATH_SIZE]; - const char *devpath; - const char *action; - int err; + const char *devpath, *action; + int r; - log_parse_environment(); - log_open(); + test_setup_logging(LOG_INFO); - err = fake_filesystems(); - if (err < 0) + r = fake_filesystems(); + if (r < 0) return EXIT_FAILURE; log_debug("version %s", PACKAGE_VERSION); mac_selinux_init(); action = argv[1]; - if (action == NULL) { + if (!action) { log_error("action missing"); goto out; } devpath = argv[2]; - if (devpath == NULL) { + if (!devpath) { log_error("devpath missing"); goto out; } rules = udev_rules_new(1); - strscpyl(syspath, sizeof(syspath), "/sys", devpath, NULL); + const char *syspath = strjoina("/sys", devpath); dev = udev_device_new_from_synthetic_event(NULL, syspath, action); - if (dev == NULL) { + if (!dev) { log_debug("unknown device '%s'", devpath); goto out; } event = udev_event_new(dev); + assert_se(event); assert_se(sigprocmask_many(SIG_BLOCK, NULL, SIGTERM, SIGINT, SIGHUP, SIGCHLD, -1) >= 0); /* do what devtmpfs usually provides us */ - if (udev_device_get_devnode(dev) != NULL) { + if (udev_device_get_devnode(dev)) { mode_t mode = 0600; if (streq(udev_device_get_subsystem(dev), "block")) @@ -122,5 +120,5 @@ int main(int argc, char *argv[]) { out: mac_selinux_finish(); - return err ? EXIT_FAILURE : EXIT_SUCCESS; + return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS; } From 3762f8e316e9cc5fed0ca0dc810bec0db1cb0632 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 20 Sep 2018 16:34:14 +0200 Subject: [PATCH 3/4] tests: add a runner for installed tests We have "installed tests", but don't provide an easy way to run them. The protocol is very simple: each test must return 0 for success, 77 means "skipped", anything else is an error. In addition, we want to print test output only if the test failed. I wrote this simple script. It is pretty basic, but implements the functions listed above. Since it is written in python it should be easy to add option parsing (like running only specific tests, or running unsafe tests, etc.) I looked at the following alternatives: - Ubuntu root-unittests: this works, but just dumps all output to the terminal, has no coloring. - @ssahani's test runner [2] It uses the unittest library and the test suite was implented as a class, and doesn't implement any of the functions listed above. - cram [3,4] cram runs our tests, but does not understand the "ignore the output" part, has not support for our magic skip code (it uses hardcoded 80 instead), and seems dead upstream. - meson test Here the idea would be to provide an almost-empty meson.build file under /usr/lib/systemd/tests/ that would just define all the tests. This would allow us to reuse the test runner we use normally. Unfortunately meson requires a build directory and configuration to be done before running tests. This would be possible, but seems a lot of effort to just run a few binaries. [1] https://salsa.debian.org/systemd-team/systemd/blob/242c96addb06480ec9cd75248a5660f37a17b4b9/debian/tests/root-unittests [2] https://github.com/systemd/systemd-fedora-ci/blob/master/upstream/systemd-upstream-tests.py [3] https://bitheap.org/cram/ [4] https://pypi.org/project/pytest-cram/ Fixes #10069. --- test/meson.build | 8 +++++++ test/run-unit-tests.py | 49 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100755 test/run-unit-tests.py diff --git a/test/meson.build b/test/meson.build index 9750ff22b9..13896aee50 100644 --- a/test/meson.build +++ b/test/meson.build @@ -229,6 +229,14 @@ endif ############################################################ +if install_tests + install_data('run-unit-tests.py', + install_mode : 'rwxr-xr-x', + install_dir : testsdir) +endif + +############################################################ + # prepare test/sys tree sys_script_py = find_program('sys-script.py') custom_target( diff --git a/test/run-unit-tests.py b/test/run-unit-tests.py new file mode 100755 index 0000000000..4bbc3e2c44 --- /dev/null +++ b/test/run-unit-tests.py @@ -0,0 +1,49 @@ +#!/usr/bin/env python3 + +import dataclasses +import glob +import os +import subprocess +import sys +try: + import colorama as c + GREEN = c.Fore.GREEN + YELLOW = c.Fore.YELLOW + RED = c.Fore.RED + RESET_ALL = c.Style.RESET_ALL + BRIGHT = c.Style.BRIGHT +except ImportError: + GREEN = YELLOW = RED = RESET_ALL = BRIGHT = '' + +@dataclasses.dataclass +class Total: + total:int + good:int = 0 + skip:int = 0 + fail:int = 0 + +tests = glob.glob('/usr/lib/systemd/tests/test-*') +total = Total(total=len(tests)) +for test in tests: + name = os.path.basename(test) + + ex = subprocess.run(test, stdin=subprocess.DEVNULL, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + if ex.returncode == 0: + print(f'{GREEN}PASS: {name}{RESET_ALL}') + total.good += 1 + elif ex.returncode == 77: + print(f'{YELLOW}SKIP: {name}{RESET_ALL}') + total.skip += 1 + else: + print(f'{RED}FAIL: {name}{RESET_ALL}') + total.fail += 1 + + # stdout/stderr might not be valid unicode, let's just dump it to the terminal. + # Also let's reset the style afterwards, in case our output sets something. + sys.stdout.buffer.write(ex.stdout) + print(f'{RESET_ALL}{BRIGHT}') + sys.stdout.buffer.write(ex.stderr) + print(f'{RESET_ALL}') + +print(f'{BRIGHT}OK: {total.good} SKIP: {total.skip} FAIL: {total.fail}{RESET_ALL}') +sys.exit(total.fail > 0) From f5acf84dbed6365c484c577ba7245d4054750ef2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 21 Sep 2018 09:28:28 +0200 Subject: [PATCH 4/4] run-unit-tests: add option to run unsafe tests too --- test/run-unit-tests.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/run-unit-tests.py b/test/run-unit-tests.py index 4bbc3e2c44..9a75cd421e 100755 --- a/test/run-unit-tests.py +++ b/test/run-unit-tests.py @@ -1,5 +1,6 @@ #!/usr/bin/env python3 +import argparse import dataclasses import glob import os @@ -22,7 +23,18 @@ class Total: skip:int = 0 fail:int = 0 +def argument_parser(): + p = argparse.ArgumentParser() + p.add_argument('-u', '--unsafe', action='store_true', + help='run "unsafe" tests too') + return p + +opts = argument_parser().parse_args() + tests = glob.glob('/usr/lib/systemd/tests/test-*') +if opts.unsafe: + tests += glob.glob('/usr/lib/systemd/tests/unsafe/test-*') + total = Total(total=len(tests)) for test in tests: name = os.path.basename(test)