tests: rework test macros to not take code as parameters

C macros are nasty. We use them, but we try to be conservative with
them. In particular passing literal, complex code blocks as argument is
icky, because of "," handling of C, and also because it's quite a
challange for most code highlighters and similar. Hence, let's avoid
that. Using macros for genreating functions is OK but if so, the
parameters should be simple words, not full code blocks.

hence, rework DEFINE_CUSTOM_TEST_MAIN() to take a function name instead
of code block as argument.

As side-effect this also fixes a bunch of cases where we might end up
returning a negative value from main().

Some uses of DEFINE_CUSTOM_TEST_MAIN() inserted local variables into the
main() functions, these are replaced by static variables, and their
destructors by the static destructor logic.

This doesn't fix any bugs or so, it's just supposed to make the code
easier to work with and improve it easthetically.

Or in other words: let's use macros where it really makes sense, but
let's not go overboard with it.

(And yes, FOREACH_DIRENT() is another one of those macros that take
code, and I dislike that too and regret I ever added that.)

src/shared/tests.h

@@ -6,6 +6,7 @@
 #include "sd-daemon.h"
 
 #include "macro.h"
+#include "static-destruct.h"
 #include "util.h"
 
 static inline bool manager_errno_skip_test(int r) {
@@ -109,15 +110,27 @@ static inline int run_test_table(void) {
         return r;
 }
 
+static inline int test_nop(void) {
+        return EXIT_SUCCESS;
+}
+
 #define DEFINE_CUSTOM_TEST_MAIN(log_level, intro, outro) \
         int main(int argc, char *argv[]) {               \
-                int _r = EXIT_SUCCESS;                   \
+                int _r, _q;                              \
                 test_setup_logging(log_level);           \
                 save_argc_argv(argc, argv);              \
-                intro;                                   \
-                _r = run_test_table();                   \
-                outro;                                   \
-                return _r;                               \
+                _r = intro();                            \
+                if (_r == EXIT_SUCCESS)                  \
+                        _r = run_test_table();           \
+                _q = outro();                            \
+                static_destruct();                       \
+                if (_r < 0)                              \
+                        return EXIT_FAILURE;             \
+                if (_r != EXIT_SUCCESS)                  \
+                        return _r;                       \
+                if (_q < 0)                              \
+                        return EXIT_FAILURE;             \
+                return _q;                               \
         }
 
-#define DEFINE_TEST_MAIN(log_level) DEFINE_CUSTOM_TEST_MAIN(log_level, , )
+#define DEFINE_TEST_MAIN(log_level) DEFINE_CUSTOM_TEST_MAIN(log_level, test_nop, test_nop)

src/test/test-barrier.c

@@ -421,25 +421,27 @@ TEST_BARRIER(barrier_pending_exit,
         }),
         TEST_BARRIER_WAIT_SUCCESS(pid2));
 
-DEFINE_CUSTOM_TEST_MAIN(
-        LOG_INFO,
-        ({
-                if (!slow_tests_enabled())
-                        return log_tests_skipped("slow tests are disabled");
 
-                /*
-                * This test uses real-time alarms and sleeps to test for CPU races
-                * explicitly. This is highly fragile if your system is under load. We
-                * already increased the BASE_TIME value to make the tests more robust,
-                * but that just makes the test take significantly longer. Given the recent
-                * issues when running the test in a virtualized environments, limit it
-                * to bare metal machines only, to minimize false-positives in CIs.
-                */
-                int v = detect_virtualization();
-                if (IN_SET(v, -EPERM, -EACCES))
-                        return log_tests_skipped("Cannot detect virtualization");
+static int intro(void) {
+        if (!slow_tests_enabled())
+                return log_tests_skipped("slow tests are disabled");
 
-                if (v != VIRTUALIZATION_NONE)
-                        return log_tests_skipped("This test requires a baremetal machine");
-        }),
-        /* no outro */);
+        /*
+         * This test uses real-time alarms and sleeps to test for CPU races explicitly. This is highly
+         * fragile if your system is under load. We already increased the BASE_TIME value to make the tests
+         * more robust, but that just makes the test take significantly longer. Given the recent issues when
+         * running the test in a virtualized environments, limit it to bare metal machines only, to minimize
+         * false-positives in CIs.
+         */
+
+        int v = detect_virtualization();
+        if (IN_SET(v, -EPERM, -EACCES))
+                return log_tests_skipped("Cannot detect virtualization");
+
+        if (v != VIRTUALIZATION_NONE)
+                return log_tests_skipped("This test requires a baremetal machine");
+
+        return EXIT_SUCCESS;
+ }
+
+DEFINE_CUSTOM_TEST_MAIN(LOG_INFO, intro, test_nop);

src/test/test-cgroup-setup.c

@@ -64,10 +64,11 @@ TEST(is_wanted) {
         test_is_wanted_print_one(false);
 }
 
-DEFINE_CUSTOM_TEST_MAIN(
-        LOG_DEBUG,
-        ({
-                if (access("/proc/cmdline", R_OK) < 0 && ERRNO_IS_PRIVILEGE(errno))
-                        return log_tests_skipped("can't read /proc/cmdline");
-        }),
-        /* no outro */);
+static int intro(void) {
+        if (access("/proc/cmdline", R_OK) < 0 && ERRNO_IS_PRIVILEGE(errno))
+                return log_tests_skipped("can't read /proc/cmdline");
+
+        return EXIT_SUCCESS;
+}
+
+DEFINE_CUSTOM_TEST_MAIN(LOG_DEBUG, intro, test_nop);

src/test/test-chown-rec.c

@@ -149,10 +149,11 @@ TEST(chown_recursive) {
         assert_se(!has_xattr(p));
 }
 
-DEFINE_CUSTOM_TEST_MAIN(
-        LOG_DEBUG,
-        ({
-                if (geteuid() != 0)
-                        return log_tests_skipped("not running as root");
-        }),
-        /* no outro */);
+static int intro(void) {
+        if (geteuid() != 0)
+                return log_tests_skipped("not running as root");
+
+        return EXIT_SUCCESS;
+}
+
+DEFINE_CUSTOM_TEST_MAIN(LOG_DEBUG, intro, test_nop);

src/test/test-format-table.c

@@ -529,10 +529,10 @@ TEST(table) {
                                 "5min              5min              \n"));
 }
 
-DEFINE_CUSTOM_TEST_MAIN(
-        LOG_INFO,
-        ({
-                assert_se(setenv("SYSTEMD_COLORS", "0", 1) >= 0);
-                assert_se(setenv("COLUMNS", "40", 1) >= 0);
-        }),
-        /* no outro */);
+static int intro(void) {
+        assert_se(setenv("SYSTEMD_COLORS", "0", 1) >= 0);
+        assert_se(setenv("COLUMNS", "40", 1) >= 0);
+        return EXIT_SUCCESS;
+}
+
+DEFINE_CUSTOM_TEST_MAIN(LOG_INFO, intro, test_nop);

src/test/test-fs-util.c

@@ -968,4 +968,9 @@ TEST(open_mkdir_at) {
         assert_se(subsubdir_fd >= 0);
 }
 
-DEFINE_CUSTOM_TEST_MAIN(LOG_INFO, arg_test_dir = argv[1], /* no outro */);
+static int intro(void) {
+        arg_test_dir = saved_argv[1];
+        return EXIT_SUCCESS;
+}
+
+DEFINE_CUSTOM_TEST_MAIN(LOG_INFO, intro, test_nop);

src/test/test-hashmap.c

@@ -158,7 +158,15 @@ TEST(hashmap_put_strdup_null) {
 /* This variable allows us to assert that the tests from different compilation units were actually run. */
 int n_extern_tests_run = 0;
 
-DEFINE_CUSTOM_TEST_MAIN(
-        LOG_INFO,
-        assert_se(n_extern_tests_run == 0),
-        assert_se(n_extern_tests_run == 2)); /* Ensure hashmap and ordered_hashmap were tested. */
+static int intro(void) {
+        assert_se(n_extern_tests_run == 0);
+        return EXIT_SUCCESS;
+}
+
+static int outro(void) {
+        /* Ensure hashmap and ordered_hashmap were tested. */
+        assert_se(n_extern_tests_run == 2);
+        return EXIT_SUCCESS;
+}
+
+DEFINE_CUSTOM_TEST_MAIN(LOG_INFO, intro, outro);

src/test/test-install-root.c

@@ -11,8 +11,11 @@
 #include "special.h"
 #include "string-util.h"
 #include "tests.h"
+#include "tmpfile-util.h"
 
-static char root[] = "/tmp/rootXXXXXX";
+static char *root = NULL;
+
+STATIC_DESTRUCTOR_REGISTER(root, rm_rf_physical_and_freep);
 
 TEST(basic_mask_and_enable) {
         const char *p;
@@ -1239,10 +1242,10 @@ TEST(verify_alias) {
         verify_one(&di_inst_template, "goo.target.conf/plain.service", -EXDEV, NULL);
 }
 
-static void setup_root(void) {
+static int intro(void) {
         const char *p;
 
-        assert_se(mkdtemp(root));
+        assert_se(mkdtemp_malloc("/tmp/rootXXXXXX", &root) >= 0);
 
         p = strjoina(root, "/usr/lib/systemd/system/");
         assert_se(mkdir_p(p, 0755) >= 0);
@@ -1264,6 +1267,9 @@ static void setup_root(void) {
 
         p = strjoina(root, "/usr/lib/systemd/system/graphical.target");
         assert_se(write_string_file(p, "# pretty much empty", WRITE_STRING_FILE_CREATE) >= 0);
+
+        return EXIT_SUCCESS;
 }
 
-DEFINE_CUSTOM_TEST_MAIN(LOG_INFO, setup_root(), assert_se(rm_rf(root, REMOVE_ROOT|REMOVE_PHYSICAL) >= 0));
+
+DEFINE_CUSTOM_TEST_MAIN(LOG_INFO, intro, test_nop);

src/test/test-load-fragment.c

@@ -30,6 +30,10 @@
 /* Nontrivial value serves as a placeholder to check that parsing function (didn't) change it */
 #define CGROUP_LIMIT_DUMMY      3
 
+static char *runtime_dir = NULL;
+
+STATIC_DESTRUCTOR_REGISTER(runtime_dir, rm_rf_physical_and_freep);
+
 TEST_RET(unit_file_get_set) {
         int r;
         Hashmap *h;
@@ -958,15 +962,12 @@ TEST(unit_is_recursive_template_dependency) {
         assert_se(unit_is_likely_recursive_template_dependency(u, "foobar@foobar@123.mount", "foobar@%n.mount") == 0);
 }
 
-DEFINE_CUSTOM_TEST_MAIN(
-        LOG_INFO,
+static int intro(void) {
+        if (enter_cgroup_subroot(NULL) == -ENOMEDIUM)
+                return log_tests_skipped("cgroupfs not available");
 
-        _cleanup_(rm_rf_physical_and_freep) char *runtime_dir = NULL;
-        ({
-                if (enter_cgroup_subroot(NULL) == -ENOMEDIUM)
-                        return log_tests_skipped("cgroupfs not available");
+        assert_se(runtime_dir = setup_fake_runtime_dir());
+        return EXIT_SUCCESS;
+}
 
-                assert_se(runtime_dir = setup_fake_runtime_dir());
-        }),
-
-        /* no outro */);
+DEFINE_CUSTOM_TEST_MAIN(LOG_INFO, intro, test_nop);

src/test/test-mountpoint-util.c

@@ -294,17 +294,19 @@ TEST(fd_is_mount_point) {
         assert_se(IN_SET(fd_is_mount_point(fd, "root/", 0), -ENOENT, 0));
 }
 
-DEFINE_CUSTOM_TEST_MAIN(
-        LOG_DEBUG,
-        ({
-                /* let's move into our own mount namespace with all propagation from the host turned off, so
-                 * that /proc/self/mountinfo is static and constant for the whole time our test runs. */
-                if (unshare(CLONE_NEWNS) < 0) {
-                        if (!ERRNO_IS_PRIVILEGE(errno))
-                                return log_error_errno(errno, "Failed to detach mount namespace: %m");
+static int intro(void) {
+        /* let's move into our own mount namespace with all propagation from the host turned off, so
+         * that /proc/self/mountinfo is static and constant for the whole time our test runs. */
 
-                        log_notice("Lacking privilege to create separate mount namespace, proceeding in originating mount namespace.");
-                } else
-                        assert_se(mount(NULL, "/", NULL, MS_PRIVATE | MS_REC, NULL) >= 0);
-        }),
-        /* no outro */);
+        if (unshare(CLONE_NEWNS) < 0) {
+                if (!ERRNO_IS_PRIVILEGE(errno))
+                        return log_error_errno(errno, "Failed to detach mount namespace: %m");
+
+                log_notice("Lacking privilege to create separate mount namespace, proceeding in originating mount namespace.");
+        } else
+                assert_se(mount(NULL, "/", NULL, MS_PRIVATE | MS_REC, NULL) >= 0);
+
+        return EXIT_SUCCESS;
+}
+
+DEFINE_CUSTOM_TEST_MAIN(LOG_DEBUG, intro, test_nop);

src/test/test-namespace.c

@@ -221,10 +221,11 @@ TEST(protect_kernel_logs) {
         assert_se(wait_for_terminate_and_check("ns-kernellogs", pid, WAIT_LOG) == EXIT_SUCCESS);
 }
 
-DEFINE_CUSTOM_TEST_MAIN(
-        LOG_INFO,
-        ({
-                if (!have_namespaces())
-                        return log_tests_skipped("Don't have namespace support");
-        }),
-        /* no outro */);
+static int intro(void) {
+        if (!have_namespaces())
+                return log_tests_skipped("Don't have namespace support");
+
+        return EXIT_SUCCESS;
+}
+
+DEFINE_CUSTOM_TEST_MAIN(LOG_INFO, intro, test_nop);

src/test/test-proc-cmdline.c

@@ -247,10 +247,11 @@ TEST(proc_cmdline_key_startswith) {
         assert_se(!proc_cmdline_key_startswith("foo-bar", "foo_xx"));
 }
 
-DEFINE_CUSTOM_TEST_MAIN(
-        LOG_INFO,
-        ({
-                if (access("/proc/cmdline", R_OK) < 0 && ERRNO_IS_PRIVILEGE(errno))
-                        return log_tests_skipped("can't read /proc/cmdline");
-        }),
-        /* no outro */);
+static int intro(void) {
+        if (access("/proc/cmdline", R_OK) < 0 && ERRNO_IS_PRIVILEGE(errno))
+                return log_tests_skipped("can't read /proc/cmdline");
+
+        return EXIT_SUCCESS;
+}
+
+DEFINE_CUSTOM_TEST_MAIN(LOG_INFO, intro, test_nop);

src/test/test-process-util.c

@@ -895,4 +895,9 @@ TEST(set_oom_score_adjust) {
         assert_se(b == a);
 }
 
-DEFINE_CUSTOM_TEST_MAIN(LOG_INFO, log_show_color(true), /* no outro */);
+static int intro(void) {
+        log_show_color(true);
+        return EXIT_SUCCESS;
+}
+
+DEFINE_CUSTOM_TEST_MAIN(LOG_INFO, intro, test_nop);

src/test/test-sd-hwdb.c

@@ -52,12 +52,15 @@ TEST(basic_enumerate) {
         assert_se(len1 == len2);
 }
 
-DEFINE_CUSTOM_TEST_MAIN(
-        LOG_DEBUG,
-        ({
-                _cleanup_(sd_hwdb_unrefp) sd_hwdb *hwdb = NULL;
-                int r = sd_hwdb_new(&hwdb);
-                if (r == -ENOENT || ERRNO_IS_PRIVILEGE(r))
-                        return log_tests_skipped_errno(r, "cannot open hwdb");
-        }),
-        /* no outro */);
+static int intro(void) {
+        _cleanup_(sd_hwdb_unrefp) sd_hwdb *hwdb = NULL;
+        int r;
+
+        r = sd_hwdb_new(&hwdb);
+        if (r == -ENOENT || ERRNO_IS_PRIVILEGE(r))
+                return log_tests_skipped_errno(r, "cannot open hwdb");
+
+        return EXIT_SUCCESS;
+}
+
+DEFINE_CUSTOM_TEST_MAIN(LOG_DEBUG, intro, test_nop);

src/test/test-serialize.c

@@ -10,7 +10,7 @@
 #include "tests.h"
 #include "tmpfile-util.h"
 
-char long_string[LONG_LINE_MAX+1];
+static char long_string[LONG_LINE_MAX+1];
 
 TEST(serialize_item) {
         _cleanup_(unlink_tempfilep) char fn[] = "/tmp/test-serialize.XXXXXX";
@@ -189,10 +189,10 @@ TEST(serialize_environment) {
         assert_se(strv_equal(env, env2));
 }
 
-DEFINE_CUSTOM_TEST_MAIN(
-        LOG_INFO,
-        ({
-                memset(long_string, 'x', sizeof(long_string)-1);
-                char_array_0(long_string);
-        }),
-        /* no outro */);
+static int intro(void) {
+        memset(long_string, 'x', sizeof(long_string)-1);
+        char_array_0(long_string);
+        return EXIT_SUCCESS;
+}
+
+DEFINE_CUSTOM_TEST_MAIN(LOG_INFO, intro, test_nop);

src/test/test-sleep.c

@@ -118,10 +118,11 @@ TEST(sleep) {
         log_info("Suspend-then-Hibernate configured and possible: %s", r >= 0 ? yes_no(r) : strerror_safe(r));
 }
 
-DEFINE_CUSTOM_TEST_MAIN(
-        LOG_DEBUG,
-        ({
-                if (getuid() != 0)
-                        log_warning("This program is unlikely to work for unprivileged users");
-        }),
-        /* no outro */);
+static int intro(void) {
+        if (getuid() != 0)
+                log_warning("This program is unlikely to work for unprivileged users");
+
+        return EXIT_SUCCESS;
+}
+
+DEFINE_CUSTOM_TEST_MAIN(LOG_DEBUG, intro, test_nop);

src/test/test-stat-util.c

@@ -236,4 +236,9 @@ TEST(dir_is_empty) {
         assert_se(dir_is_empty_at(AT_FDCWD, empty_dir) > 0);
 }
 
-DEFINE_CUSTOM_TEST_MAIN(LOG_INFO, log_show_color(true), /* no outro */);
+static int intro(void) {
+        log_show_color(true);
+        return EXIT_SUCCESS;
+}
+
+DEFINE_CUSTOM_TEST_MAIN(LOG_INFO, intro, test_nop);

src/test/test-time-util.c

@@ -588,7 +588,7 @@ TEST(map_clock_usec) {
         }
 }
 
-static void setup_test(void) {
+static int intro(void) {
         log_info("realtime=" USEC_FMT "\n"
                  "monotonic=" USEC_FMT "\n"
                  "boottime=" USEC_FMT "\n",
@@ -603,6 +603,8 @@ static void setup_test(void) {
         uintmax_t x = TIME_T_MAX;
         x++;
         assert_se((time_t) x < 0);
+
+        return EXIT_SUCCESS;
 }
 
-DEFINE_CUSTOM_TEST_MAIN(LOG_INFO, setup_test(), /* no outro */);
+DEFINE_CUSTOM_TEST_MAIN(LOG_INFO, intro, test_nop);

src/test/test-unit-file.c

@@ -102,4 +102,9 @@ TEST(runlevel_to_target) {
         assert_se(streq_ptr(runlevel_to_target("rd.rescue"), SPECIAL_RESCUE_TARGET));
 }
 
-DEFINE_CUSTOM_TEST_MAIN(LOG_DEBUG, log_show_color(true), /* no outro */);
+static int intro(void) {
+        log_show_color(true);
+        return EXIT_SUCCESS;
+}
+
+DEFINE_CUSTOM_TEST_MAIN(LOG_DEBUG, intro, test_nop);

src/test/test-unit-name.c

@@ -23,6 +23,10 @@
 #include "user-util.h"
 #include "util.h"
 
+static char *runtime_dir = NULL;
+
+STATIC_DESTRUCTOR_REGISTER(runtime_dir, rm_rf_physical_and_freep);
+
 static void test_unit_name_is_valid_one(const char *name, UnitNameFlags flags, bool expected) {
         log_info("%s ( %s%s%s ): %s",
                  name,
@@ -844,15 +848,12 @@ TEST(unit_name_prefix_equal) {
         assert_se(!unit_name_prefix_equal("a", "a"));
 }
 
-DEFINE_CUSTOM_TEST_MAIN(
-        LOG_INFO,
+static int intro(void) {
+        if (enter_cgroup_subroot(NULL) == -ENOMEDIUM)
+                return log_tests_skipped("cgroupfs not available");
 
-        _cleanup_(rm_rf_physical_and_freep) char *runtime_dir = NULL;
-        ({
-                if (enter_cgroup_subroot(NULL) == -ENOMEDIUM)
-                        return log_tests_skipped("cgroupfs not available");
+        assert_se(runtime_dir = setup_fake_runtime_dir());
+        return EXIT_SUCCESS;
+}
 
-                assert_se(runtime_dir = setup_fake_runtime_dir());
-        }),
-
-        /* no outro */);
+DEFINE_CUSTOM_TEST_MAIN(LOG_INFO, intro, test_nop);

src/test/test-unit-serialize.c

@@ -4,6 +4,10 @@
 #include "service.h"
 #include "tests.h"
 
+static char *runtime_dir = NULL;
+
+STATIC_DESTRUCTOR_REGISTER(runtime_dir, rm_rf_physical_and_freep);
+
 #define EXEC_START_ABSOLUTE \
         "ExecStart 0 /bin/sh \"sh\" \"-e\" \"-x\" \"-c\" \"systemctl --state=failed --no-legend --no-pager >/failed ; systemctl daemon-reload ; echo OK >/testok\""
 #define EXEC_START_RELATIVE \
@@ -48,15 +52,12 @@ TEST(deserialize_exec_command) {
         test_deserialize_exec_command_one(m, "control-command", "ExecWhat 11 /a/b c d e", -EINVAL);
 }
 
-DEFINE_CUSTOM_TEST_MAIN(
-        LOG_DEBUG,
+static int intro(void) {
+        if (enter_cgroup_subroot(NULL) == -ENOMEDIUM)
+                return log_tests_skipped("cgroupfs not available");
 
-        _cleanup_(rm_rf_physical_and_freep) char *runtime_dir = NULL;
-        ({
-                if (enter_cgroup_subroot(NULL) == -ENOMEDIUM)
-                        return log_tests_skipped("cgroupfs not available");
+        assert_se(runtime_dir = setup_fake_runtime_dir());
+        return EXIT_SUCCESS;
+}
 
-                assert_se(runtime_dir = setup_fake_runtime_dir());
-        }),
-
-        /* no outro */);
+DEFINE_CUSTOM_TEST_MAIN(LOG_DEBUG, intro, test_nop);

src/test/test-utf8.c

@@ -231,4 +231,9 @@ TEST(utf8_to_utf16) {
         }
 }
 
-DEFINE_CUSTOM_TEST_MAIN(LOG_INFO, log_show_color(true), /* no outro */);
+static int intro(void) {
+        log_show_color(true);
+        return EXIT_SUCCESS;
+}
+
+DEFINE_CUSTOM_TEST_MAIN(LOG_INFO, intro, test_nop);