testsuite: remove need_spawn = false support

Our test suite is a little unique in my experience in that the test can
be either a normal (fork) child or a (re)spawned one. All the other test
frameworks I have used opt for only one of the two.

I'm not entirely sure why we have both since the latter is sufficient for
all use-cases that we have. Perhaps the former was kept as
micro-optimisation?

Currently I am exploring a way to provide the results summary and the
need_spawn = false ones, are printed multiple times. At a glance I
couldn't quite find a way to fix it.

In addition I am also looking at removing/reducing the use of exit()
across the test suite. Where the two code-flows makes that process more
convoluted.

So let's remove one of the code-paths, simplify things and fix the
logging output. If needed we can re-introduce it later on.

NOTE: there's a lot going on here, because clang-format insist on
reformatting bunch of the DEFINE_TEST() instances :-\

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
Link: https://github.com/kmod-project/kmod/pull/246
Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>

diff --git a/testsuite/README b/testsuite/README
index 198bce246cafd4d6dd142d2c00946e4225b20944..e1d6d77a7d35bdc7ce16d50bfbfe5943887583fc 100644 (file)
--- a/testsuite/README
+++ b/testsuite/README
@@ -39,27 +39,22 @@ pay attention when writing a test:
 
 4 - Fill in the config vector. Setting any of these configuration will make
     testsuite to export LD_PRELOAD with the necessary override libs before
-    executing the test. If you are not exec'ing an external binary, you need to
-    pass "need_spawn = true" below, otherwise it will not work (LD_PRELOAD is
-    only applied when exec'ing a binary). See each config description in
-    testsuite.h
+    executing the test. See each config description in testsuite.h
 
-5 - need_spawn: if testsuite will exec itself before running a test
-
-6 - expected_fail: if that test is expected to fail, i.e. the return code is
+5 - expected_fail: if that test is expected to fail, i.e. the return code is
     expected not to be 0.
 
-7 - The rootfs is populated by copying the entire contents of rootfs-pristine
+6 - The rootfs is populated by copying the entire contents of rootfs-pristine
     through setup-rootfs.sh then running setup-modules.sh to copy generated
     modules from module-playground. Update the latter script to include any
     modules your test needs.
 
-8 - Tests can be run individually, outside of 'meson test'. strace and gdb
+7 - Tests can be run individually, outside of 'meson test'. strace and gdb
     work too, as long as you tell them to operate on child process.
 
     When running with sanitizers, make sure to 'source scripts/sanitizer-env.sh'.
     Sanitizers are not guaranteed to work well with other tools like strace and gdb.
 
-9 - Make sure test passes when using "default" build flags, i.e. by running
+8 - Make sure test passes when using "default" build flags, i.e. by running
     'meson setup --native-file build-dev.ini ...', which by default enables the
     sanitizers.

diff --git a/testsuite/test-blacklist.c b/testsuite/test-blacklist.c
index 4c741d3c93abe284cf54edd7de592b05f85afd44..580a259e301f63820571ad0a53c41208c86a7401 100644 (file)
--- a/testsuite/test-blacklist.c
+++ b/testsuite/test-blacklist.c
@@ -81,12 +81,9 @@ fail_lookup:
        return EXIT_FAILURE;
 }
 
-DEFINE_TEST(blacklist_1,
-       .description = "check if modules are correctly blacklisted",
-       .config = {
-               [TC_ROOTFS] = TESTSUITE_ROOTFS "test-blacklist/",
-       },
-       .need_spawn = true,
-);
+DEFINE_TEST(blacklist_1, .description = "check if modules are correctly blacklisted",
+           .config = {
+                   [TC_ROOTFS] = TESTSUITE_ROOTFS "test-blacklist/",
+           });
 
 TESTSUITE_MAIN();

diff --git a/testsuite/test-dependencies.c b/testsuite/test-dependencies.c
index 5358f7c24deddc47897c486feef29585a7ce40c2..f315af1b0611424be50ad2be6db3079aa976f3ec 100644 (file)
--- a/testsuite/test-dependencies.c
+++ b/testsuite/test-dependencies.c
@@ -67,11 +67,10 @@ static noreturn int test_dependencies(const struct test *t)
        exit(EXIT_SUCCESS);
 }
 DEFINE_TEST(test_dependencies,
-       .description = "test if kmod_module_get_dependencies works",
-       .config = {
-               [TC_UNAME_R] = TEST_UNAME,
-               [TC_ROOTFS] = TESTSUITE_ROOTFS "test-dependencies/",
-       },
-       .need_spawn = true);
+           .description = "test if kmod_module_get_dependencies works",
+           .config = {
+                   [TC_UNAME_R] = TEST_UNAME,
+                   [TC_ROOTFS] = TESTSUITE_ROOTFS "test-dependencies/",
+           });
 
 TESTSUITE_MAIN();

diff --git a/testsuite/test-init.c b/testsuite/test-init.c
index 3bfb14eedeea1696a6fabbca3150a523359c0f95..d23e8a34df913305e0d4cd262a1b156feac9a06e 100644 (file)
--- a/testsuite/test-init.c
+++ b/testsuite/test-init.c
@@ -39,21 +39,24 @@ static noreturn int test_load_resources(const struct test *t)
 
        exit(EXIT_SUCCESS);
 }
-DEFINE_TEST_WITH_FUNC(test_load_resource1, test_load_resources,
-           .description = "test if kmod_load_resources works (recent modprobe on kernel without modules.builtin.modinfo)",
-           .config = {
+DEFINE_TEST_WITH_FUNC(
+       test_load_resource1, test_load_resources,
+       .description =
+               "test if kmod_load_resources works (recent modprobe on kernel without modules.builtin.modinfo)",
+       .config = {
                [TC_ROOTFS] = TESTSUITE_ROOTFS "test-init-load-resources/",
                [TC_UNAME_R] = "5.6.0",
-           },
-           .need_spawn = true);
+       });
 
-DEFINE_TEST_WITH_FUNC(test_load_resource2, test_load_resources,
-           .description = "test if kmod_load_resources works with empty modules.builtin.aliases.bin (recent depmod on kernel without modules.builtin.modinfo)",
-           .config = {
-               [TC_ROOTFS] = TESTSUITE_ROOTFS "test-init-load-resources-empty-builtin-aliases-bin/",
+DEFINE_TEST_WITH_FUNC(
+       test_load_resource2, test_load_resources,
+       .description =
+               "test if kmod_load_resources works with empty modules.builtin.aliases.bin (recent depmod on kernel without modules.builtin.modinfo)",
+       .config = {
+               [TC_ROOTFS] = TESTSUITE_ROOTFS
+               "test-init-load-resources-empty-builtin-aliases-bin/",
                [TC_UNAME_R] = "5.6.0",
-           },
-           .need_spawn = true);
+       });
 
 static noreturn int test_initlib(const struct test *t)
 {
@@ -102,8 +105,7 @@ DEFINE_TEST(test_insert,
                [TC_ROOTFS] = TESTSUITE_ROOTFS "test-init/",
                [TC_INIT_MODULE_RETCODES] = "bla:1:20",
        },
-       .modules_loaded = "mod_simple",
-       .need_spawn = true);
+       .modules_loaded = "mod_simple");
 
 static noreturn int test_remove(const struct test *t)
 {
@@ -144,13 +146,11 @@ static noreturn int test_remove(const struct test *t)
 
        exit(EXIT_SUCCESS);
 }
-DEFINE_TEST(test_remove,
-       .description = "test if libkmod's remove_module returns ok",
+DEFINE_TEST(
+       test_remove, .description = "test if libkmod's remove_module returns ok",
        .config = {
                [TC_ROOTFS] = TESTSUITE_ROOTFS "test-remove/",
-               [TC_DELETE_MODULE_RETCODES] =
-                       "mod-simple:0:0:bla:-1:" STRINGIFY(ENOENT),
-       },
-       .need_spawn = true);
+               [TC_DELETE_MODULE_RETCODES] = "mod-simple:0:0:bla:-1:" STRINGIFY(ENOENT),
+       });
 
 TESTSUITE_MAIN();

diff --git a/testsuite/test-initstate.c b/testsuite/test-initstate.c
index f29aced1b16b4ba80c2d684c15e33a9da750e379..92ad0e3746541101a9edc0f947577b7f19df5710 100644 (file)
--- a/testsuite/test-initstate.c
+++ b/testsuite/test-initstate.c
@@ -57,13 +57,14 @@ static noreturn int test_initstate_from_lookup(const struct test *t)
 
        exit(EXIT_SUCCESS);
 }
-DEFINE_TEST(test_initstate_from_lookup,
-       .description = "test if libkmod return correct initstate for builtin module from lookup",
+DEFINE_TEST(
+       test_initstate_from_lookup,
+       .description =
+               "test if libkmod return correct initstate for builtin module from lookup",
        .config = {
                [TC_ROOTFS] = TESTSUITE_ROOTFS "test-initstate",
                [TC_UNAME_R] = "4.4.4",
-       },
-       .need_spawn = true);
+       });
 
 static noreturn int test_initstate_from_name(const struct test *t)
 {
@@ -100,11 +101,11 @@ static noreturn int test_initstate_from_name(const struct test *t)
        exit(EXIT_SUCCESS);
 }
 DEFINE_TEST(test_initstate_from_name,
-       .description = "test if libkmod return correct initstate for builtin module from name",
-       .config = {
-               [TC_ROOTFS] = TESTSUITE_ROOTFS "test-initstate",
-               [TC_UNAME_R] = "4.4.4",
-       },
-       .need_spawn = true);
+           .description =
+                   "test if libkmod return correct initstate for builtin module from name",
+           .config = {
+                   [TC_ROOTFS] = TESTSUITE_ROOTFS "test-initstate",
+                   [TC_UNAME_R] = "4.4.4",
+           });
 
 TESTSUITE_MAIN();

diff --git a/testsuite/test-loaded.c b/testsuite/test-loaded.c
index 9bf8eb79f77acf64b76e4bbe2f36ff1d13717803..ad8e37e038e558c684f2d9568077694196675912 100644 (file)
--- a/testsuite/test-loaded.c
+++ b/testsuite/test-loaded.c
@@ -71,7 +71,6 @@ DEFINE_TEST(loaded_1,
        .config = {
                [TC_ROOTFS] = TESTSUITE_ROOTFS "test-loaded/",
        },
-       .need_spawn = true,
        .output = {
                .out = TESTSUITE_ROOTFS "test-loaded/correct.txt",
        });

diff --git a/testsuite/test-modprobe.c b/testsuite/test-modprobe.c
index 25b5af7cbfbd96d9bb2d3515b0b1590a1f05742e..8dc183111f6aad571fa8e02d5c3d727fae760a98 100644 (file)
--- a/testsuite/test-modprobe.c
+++ b/testsuite/test-modprobe.c
@@ -354,7 +354,6 @@ DEFINE_TEST(modprobe_module_from_relpath,
                [TC_ROOTFS] = TESTSUITE_ROOTFS "test-modprobe/module-from-relpath",
                [TC_INIT_MODULE_RETCODES] = "",
        },
-       .need_spawn = true,
        .modules_loaded = "mod-simple",
        );
 

diff --git a/testsuite/test-new-module.c b/testsuite/test-new-module.c
index 866526de68ba9b1a1595b3038d98928c972af9c9..ed051cae52f6bbcac466aaaf2f334e9c9e63e882 100644 (file)
--- a/testsuite/test-new-module.c
+++ b/testsuite/test-new-module.c
@@ -55,7 +55,6 @@ DEFINE_TEST(from_name,
        .config = {
                [TC_ROOTFS] = TESTSUITE_ROOTFS "test-new-module/from_name/",
        },
-       .need_spawn = true,
        .output = {
                .out = TESTSUITE_ROOTFS "test-new-module/from_name/correct.txt",
        });
@@ -100,7 +99,6 @@ DEFINE_TEST(from_alias,
        .config = {
                [TC_ROOTFS] = TESTSUITE_ROOTFS "test-new-module/from_alias/",
        },
-       .need_spawn = true,
        .output = {
                .out = TESTSUITE_ROOTFS "test-new-module/from_alias/correct.txt",
        });

diff --git a/testsuite/test-testsuite.c b/testsuite/test-testsuite.c
index eff911a78a55db0d05afca96227a0fdc89c4b3e1..42fcb6f2df7984487b688684218f6dbb65246de4 100644 (file)
--- a/testsuite/test-testsuite.c
+++ b/testsuite/test-testsuite.c
@@ -38,12 +38,10 @@ static noreturn int testsuite_uname(const struct test *t)
 
        exit(EXIT_SUCCESS);
 }
-DEFINE_TEST(testsuite_uname,
-       .description = "test if trap to uname() works",
-       .config = {
-               [TC_UNAME_R] = TEST_UNAME,
-       },
-       .need_spawn = true);
+DEFINE_TEST(testsuite_uname, .description = "test if trap to uname() works",
+           .config = {
+                   [TC_UNAME_R] = TEST_UNAME,
+           });
 
 static int testsuite_rootfs_fopen(const struct test *t)
 {
@@ -64,12 +62,10 @@ static int testsuite_rootfs_fopen(const struct test *t)
 
        return EXIT_SUCCESS;
 }
-DEFINE_TEST(testsuite_rootfs_fopen,
-       .description = "test if rootfs works - fopen()",
-       .config = {
-               [TC_ROOTFS] = TESTSUITE_ROOTFS "test-rootfs/",
-       },
-       .need_spawn = true);
+DEFINE_TEST(testsuite_rootfs_fopen, .description = "test if rootfs works - fopen()",
+           .config = {
+                   [TC_ROOTFS] = TESTSUITE_ROOTFS "test-rootfs/",
+           });
 
 static int testsuite_rootfs_open(const struct test *t)
 {
@@ -97,12 +93,10 @@ static int testsuite_rootfs_open(const struct test *t)
 
        return EXIT_SUCCESS;
 }
-DEFINE_TEST(testsuite_rootfs_open,
-       .description = "test if rootfs works - open()",
-       .config = {
-               [TC_ROOTFS] = TESTSUITE_ROOTFS "test-rootfs/",
-       },
-       .need_spawn = true);
+DEFINE_TEST(testsuite_rootfs_open, .description = "test if rootfs works - open()",
+           .config = {
+                   [TC_ROOTFS] = TESTSUITE_ROOTFS "test-rootfs/",
+           });
 
 static int testsuite_rootfs_stat(const struct test *t)
 {
@@ -115,12 +109,10 @@ static int testsuite_rootfs_stat(const struct test *t)
 
        return EXIT_SUCCESS;
 }
-DEFINE_TEST(testsuite_rootfs_stat,
-       .description = "test if rootfs works - stat()",
-       .config = {
-               [TC_ROOTFS] = TESTSUITE_ROOTFS "test-rootfs/",
-       },
-       .need_spawn = true);
+DEFINE_TEST(testsuite_rootfs_stat, .description = "test if rootfs works - stat()",
+           .config = {
+                   [TC_ROOTFS] = TESTSUITE_ROOTFS "test-rootfs/",
+           });
 
 static int testsuite_rootfs_opendir(const struct test *t)
 {
@@ -135,11 +127,9 @@ static int testsuite_rootfs_opendir(const struct test *t)
        closedir(d);
        return EXIT_SUCCESS;
 }
-DEFINE_TEST(testsuite_rootfs_opendir,
-       .description = "test if rootfs works - opendir()",
-       .config = {
-               [TC_ROOTFS] = TESTSUITE_ROOTFS "test-rootfs/",
-       },
-       .need_spawn = true);
+DEFINE_TEST(testsuite_rootfs_opendir, .description = "test if rootfs works - opendir()",
+           .config = {
+                   [TC_ROOTFS] = TESTSUITE_ROOTFS "test-rootfs/",
+           });
 
 TESTSUITE_MAIN();

diff --git a/testsuite/test-util.c b/testsuite/test-util.c
index e38b9f833f6fafd783a01aa87c5f4ddd2eb7ad1b..84a04ad95344085ed97624c03ab2626769201b2e 100644 (file)
--- a/testsuite/test-util.c
+++ b/testsuite/test-util.c
@@ -56,7 +56,6 @@ DEFINE_TEST(alias_1,
        .config = {
                [TC_ROOTFS] = TESTSUITE_ROOTFS "test-util/",
        },
-       .need_spawn = true,
        .output = {
                .out = TESTSUITE_ROOTFS "test-util/alias-correct.txt",
        });
@@ -86,7 +85,6 @@ DEFINE_TEST(test_freadline_wrapped,
        .config = {
                [TC_ROOTFS] = TESTSUITE_ROOTFS "test-util/",
        },
-       .need_spawn = true,
        .output = {
                .out = TESTSUITE_ROOTFS "test-util/freadline_wrapped-correct.txt",
        });
@@ -182,7 +180,6 @@ DEFINE_TEST(test_write_str_safe,
        .config = {
                [TC_ROOTFS] = TESTSUITE_ROOTFS "test-util2/",
        },
-       .need_spawn = true,
        .output = {
                .files = (const struct keyval[]) {
                        { TEST_WRITE_STR_SAFE_PATH ".txt",

diff --git a/testsuite/test-weakdep.c b/testsuite/test-weakdep.c
index 0c7aacd16d4f48812584d42922c552f81cc634bb..ca75349aaa6cfc3ed23abe144fee0073fb79a525 100644 (file)
--- a/testsuite/test-weakdep.c
+++ b/testsuite/test-weakdep.c
@@ -90,7 +90,6 @@ DEFINE_TEST(test_weakdep,
                [TC_ROOTFS] = TESTSUITE_ROOTFS "test-weakdep",
                [TC_INIT_MODULE_RETCODES] = "",
        },
-       .need_spawn = true,
        .output = {
                .out = TESTSUITE_ROOTFS "test-weakdep/correct-weakdep.txt",
        });

diff --git a/testsuite/testsuite.c b/testsuite/testsuite.c
index b1c0f362c9e89da57c04ce10a811f77de255f5d4..0490d5e77f530e108cc8e16ef67aa9c146a85392 100644 (file)
--- a/testsuite/testsuite.c
+++ b/testsuite/testsuite.c
@@ -264,10 +264,7 @@ static inline int test_run_child(const struct test *t, int fdout[2], int fderr[2
                }
        }
 
-       if (t->need_spawn)
-               return test_spawn_test(t);
-       else
-               return test_run_spawned(t);
+       return test_spawn_test(t);
 }
 
 #define BUFSZ 4096
@@ -1149,7 +1146,7 @@ int test_run(const struct test *t)
        int fderr[2];
        int fdmonitor[2];
 
-       if (t->need_spawn && oneshot)
+       if (oneshot)
                test_run_spawned(t);
 
        if (t->output.out != NULL) {

diff --git a/testsuite/testsuite.h b/testsuite/testsuite.h
index adcb0538c859470da50dfb01eddc179ba57d02c6..161817e7a669e7f197e3ef2c2dd584d4edef6bab 100644 (file)
--- a/testsuite/testsuite.h
+++ b/testsuite/testsuite.h
@@ -96,7 +96,6 @@ struct test {
        const char *config[_TC_LAST];
        const char *path;
        const struct keyval *env_vars;
-       bool need_spawn;
        bool expected_fail;
        /* allow to skip tests that don't meet compile-time dependencies */
        bool skip;