## TL;DR
This patchset adds a centralized executor to dispatch tests rather than relying on late_initcall to schedule each test suite separately along with a couple of new features that depend on it.
Also, sorry for the extreme delay in getting this out. Part of the delay came from finding that there were actually several architectures that the previous revision of this patchset didn't work on, so I went through and attempted to test this patchset on every architecture - more on that later.
## What am I trying to do?
Conceptually, I am trying to provide a mechanism by which test suites can be grouped together so that they can be reasoned about collectively. The last two of three patches in this series add features which depend on this:
PATCH 8/11 Prints out a test plan[1] right before KUnit tests are run; this is valuable because it makes it possible for a test harness to detect whether the number of tests run matches the number of tests expected to be run, ensuring that no tests silently failed. The test plan includes a count of tests that will run. With the centralized executor, the tests are located in a single data structure and thus can be counted.
PATCH 9/11 Add a new kernel command-line option which allows the user to specify that the kernel poweroff, halt, or reboot after completing all KUnit tests; this is very handy for running KUnit tests on UML or a VM so that the UML/VM process exits cleanly immediately after running all tests without needing a special initramfs. The centralized executor provides a definitive point when all tests have completed and the poweroff, halt, or reboot could occur.
In addition, by dispatching tests from a single location, we can guarantee that all KUnit tests run after late_init is complete, which was a concern during the initial KUnit patchset review (this has not been a problem in practice, but resolving with certainty is nevertheless desirable).
Other use cases for this exist, but the above features should provide an idea of the value that this could provide.
## Changes since last revision: - On the last revision I got some messages from 0day that showed that this patchset didn't work on several architectures, one issue that this patchset addresses is that we were aligning both memory segments as well as structures in the segments to specific byte boundaries which was incorrect. - The issue mentioned above also caused me to test on additional architectures which revealed that some architectures other than UML do not use the default init linker section macro that most architectures use. There are now several new patches (2, 3, 4, and 6). - Fixed a formatting consistency issue in the kernel params documentation patch (9/9). - Add a brief blurb on how and when the kunit_test_suite macro works.
## Remaining work to be done:
The only architecture for which I was able to get a compiler, but was apparently unable to get KUnit into a section that the executor to see was m68k - not sure why.
Alan Maguire (1): kunit: test: create a single centralized executor for all tests
Brendan Higgins (10): vmlinux.lds.h: add linker section for KUnit test suites arch: arm64: add linker section for KUnit test suites arch: microblaze: add linker section for KUnit test suites arch: powerpc: add linker section for KUnit test suites arch: um: add linker section for KUnit test suites arch: xtensa: add linker section for KUnit test suites init: main: add KUnit to kernel init kunit: test: add test plan to KUnit TAP format Documentation: Add kunit_shutdown to kernel-parameters.txt Documentation: kunit: add a brief blurb about kunit_test_suite
.../admin-guide/kernel-parameters.txt | 8 ++ Documentation/dev-tools/kunit/usage.rst | 5 ++ arch/arm64/kernel/vmlinux.lds.S | 3 + arch/microblaze/kernel/vmlinux.lds.S | 4 + arch/powerpc/kernel/vmlinux.lds.S | 4 + arch/um/include/asm/common.lds.S | 4 + arch/xtensa/kernel/vmlinux.lds.S | 4 + include/asm-generic/vmlinux.lds.h | 8 ++ include/kunit/test.h | 73 ++++++++++++----- init/main.c | 4 + lib/kunit/Makefile | 3 +- lib/kunit/executor.c | 63 +++++++++++++++ lib/kunit/test.c | 13 +-- tools/testing/kunit/kunit_kernel.py | 2 +- tools/testing/kunit/kunit_parser.py | 74 +++++++++++++++--- .../test_is_test_passed-all_passed.log | Bin 1562 -> 1567 bytes .../test_data/test_is_test_passed-crash.log | Bin 3016 -> 3021 bytes .../test_data/test_is_test_passed-failure.log | Bin 1700 -> 1705 bytes 18 files changed, 226 insertions(+), 46 deletions(-) create mode 100644 lib/kunit/executor.c
base-commit: 4333a9b0b67bb4e8bcd91bdd80da80b0ec151162 prerequisite-patch-id: 2d4b5aa9fa8ada9ae04c8584b47c299a822b9455 prerequisite-patch-id: 582b6d9d28ce4b71628890ec832df6522ca68de0
These patches are available for download with dependencies here:
https://kunit-review.googlesource.com/c/linux/+/3829
[1] https://github.com/isaacs/testanything.github.io/blob/tap14/tap-version-14-s... [2] https://patchwork.kernel.org/patch/11383635/
Add a linker section where KUnit can put references to its test suites. This patch is the first step in transitioning to dispatching all KUnit tests from a centralized executor rather than having each as its own separate late_initcall.
Co-developed-by: Iurii Zaikin yzaikin@google.com Signed-off-by: Iurii Zaikin yzaikin@google.com Signed-off-by: Brendan Higgins brendanhiggins@google.com Reviewed-by: Stephen Boyd sboyd@kernel.org --- include/asm-generic/vmlinux.lds.h | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h index db600ef218d7d..4f9b036fc9616 100644 --- a/include/asm-generic/vmlinux.lds.h +++ b/include/asm-generic/vmlinux.lds.h @@ -881,6 +881,13 @@ KEEP(*(.con_initcall.init)) \ __con_initcall_end = .;
+/* Alignment must be consistent with (kunit_suite *) in include/kunit/test.h */ +#define KUNIT_TEST_SUITES \ + . = ALIGN(8); \ + __kunit_suites_start = .; \ + KEEP(*(.kunit_test_suites)) \ + __kunit_suites_end = .; + #ifdef CONFIG_BLK_DEV_INITRD #define INIT_RAM_FS \ . = ALIGN(4); \ @@ -1056,6 +1063,7 @@ INIT_CALLS \ CON_INITCALL \ INIT_RAM_FS \ + KUNIT_TEST_SUITES \ }
#define BSS_SECTION(sbss_align, bss_align, stop_align) \
Add a linker section to arm64 where KUnit can put references to its test suites. This patch is an early step in transitioning to dispatching all KUnit tests from a centralized executor rather than having each as its own separate late_initcall.
Signed-off-by: Brendan Higgins brendanhiggins@google.com --- arch/arm64/kernel/vmlinux.lds.S | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S index 6827da7f3aa54..a1cae9cc655d7 100644 --- a/arch/arm64/kernel/vmlinux.lds.S +++ b/arch/arm64/kernel/vmlinux.lds.S @@ -181,6 +181,9 @@ SECTIONS INIT_RAM_FS *(.init.rodata.* .init.bss) /* from the EFI stub */ } + .kunit_test_suites : { + KUNIT_TEST_SUITES + } .exit.data : { EXIT_DATA }
Add a linker section to microblaze where KUnit can put references to its test suites. This patch is an early step in transitioning to dispatching all KUnit tests from a centralized executor rather than having each as its own separate late_initcall.
Signed-off-by: Brendan Higgins brendanhiggins@google.com --- arch/microblaze/kernel/vmlinux.lds.S | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/microblaze/kernel/vmlinux.lds.S b/arch/microblaze/kernel/vmlinux.lds.S index df07b3d06cd6b..4fc32f8979a60 100644 --- a/arch/microblaze/kernel/vmlinux.lds.S +++ b/arch/microblaze/kernel/vmlinux.lds.S @@ -128,6 +128,10 @@ SECTIONS {
__init_end = .;
+ .kunit_test_suites : { + KUNIT_TEST_SUITES + } + .bss ALIGN (PAGE_SIZE) : AT(ADDR(.bss) - LOAD_OFFSET) { /* page aligned when MMU used */ __bss_start = . ;
Add a linker section to powerpc where KUnit can put references to its test suites. This patch is an early step in transitioning to dispatching all KUnit tests from a centralized executor rather than having each as its own separate late_initcall.
Signed-off-by: Brendan Higgins brendanhiggins@google.com --- arch/powerpc/kernel/vmlinux.lds.S | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S index 326e113d2e456..0cc97dbfde0ad 100644 --- a/arch/powerpc/kernel/vmlinux.lds.S +++ b/arch/powerpc/kernel/vmlinux.lds.S @@ -202,6 +202,10 @@ SECTIONS CON_INITCALL }
+ .kunit_test_suites : { + KUNIT_TEST_SUITES + } + . = ALIGN(8); __ftr_fixup : AT(ADDR(__ftr_fixup) - LOAD_OFFSET) { __start___ftr_fixup = .;
Add a linker section to UML where KUnit can put references to its test suites. This patch is an early step in transitioning to dispatching all KUnit tests from a centralized executor rather than having each as its own separate late_initcall.
Signed-off-by: Brendan Higgins brendanhiggins@google.com Reviewed-by: Stephen Boyd sboyd@kernel.org --- arch/um/include/asm/common.lds.S | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/um/include/asm/common.lds.S b/arch/um/include/asm/common.lds.S index eca6c452a41bd..9a9c97f45694c 100644 --- a/arch/um/include/asm/common.lds.S +++ b/arch/um/include/asm/common.lds.S @@ -52,6 +52,10 @@ CON_INITCALL }
+ .kunit_test_suites : { + KUNIT_TEST_SUITES + } + .exitcall : { __exitcall_begin = .; *(.exitcall.exit)
Add a linker section to xtensa where KUnit can put references to its test suites. This patch is an early step in transitioning to dispatching all KUnit tests from a centralized executor rather than having each as its own separate late_initcall.
Signed-off-by: Brendan Higgins brendanhiggins@google.com --- arch/xtensa/kernel/vmlinux.lds.S | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/arch/xtensa/kernel/vmlinux.lds.S b/arch/xtensa/kernel/vmlinux.lds.S index d23a6e38f0625..9aec4ef67d0b0 100644 --- a/arch/xtensa/kernel/vmlinux.lds.S +++ b/arch/xtensa/kernel/vmlinux.lds.S @@ -216,6 +216,10 @@ SECTIONS INIT_RAM_FS }
+ .kunit_test_suites : { + KUNIT_TEST_SUITES + } + PERCPU_SECTION(XCHAL_ICACHE_LINESIZE)
/* We need this dummy segment here */
From: Alan Maguire alan.maguire@oracle.com
Add a centralized executor to dispatch tests rather than relying on late_initcall to schedule each test suite separately. Centralized execution is for built-in tests only; modules will execute tests when loaded.
Signed-off-by: Alan Maguire alan.maguire@oracle.com Co-developed-by: Iurii Zaikin yzaikin@google.com Signed-off-by: Iurii Zaikin yzaikin@google.com Co-developed-by: Brendan Higgins brendanhiggins@google.com Signed-off-by: Brendan Higgins brendanhiggins@google.com Reviewed-by: Stephen Boyd sboyd@kernel.org --- include/kunit/test.h | 64 ++++++++++++++++++++++++++++++-------------- lib/kunit/Makefile | 3 ++- lib/kunit/executor.c | 28 +++++++++++++++++++ lib/kunit/test.c | 2 +- 4 files changed, 75 insertions(+), 22 deletions(-) create mode 100644 lib/kunit/executor.c
diff --git a/include/kunit/test.h b/include/kunit/test.h index 47e61e1d53370..d13965eb624d4 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -224,7 +224,7 @@ size_t kunit_suite_num_test_cases(struct kunit_suite *suite); unsigned int kunit_test_case_num(struct kunit_suite *suite, struct kunit_case *test_case);
-int __kunit_test_suites_init(struct kunit_suite **suites); +int __kunit_test_suites_init(struct kunit_suite * const * const suites);
void __kunit_test_suites_exit(struct kunit_suite **suites);
@@ -237,34 +237,58 @@ void __kunit_test_suites_exit(struct kunit_suite **suites); * Registers @suites_list with the test framework. See &struct kunit_suite for * more information. * - * When builtin, KUnit tests are all run as late_initcalls; this means - * that they cannot test anything where tests must run at a different init - * phase. One significant restriction resulting from this is that KUnit - * cannot reliably test anything that is initialize in the late_init phase; - * another is that KUnit is useless to test things that need to be run in - * an earlier init phase. - * - * An alternative is to build the tests as a module. Because modules - * do not support multiple late_initcall()s, we need to initialize an - * array of suites for a module. - * - * TODO(brendanhiggins@google.com): Don't run all KUnit tests as - * late_initcalls. I have some future work planned to dispatch all KUnit - * tests from the same place, and at the very least to do so after - * everything else is definitely initialized. + * If a test suite is built-in, module_init() gets translated into + * an initcall which we don't want as the idea is that for builtins + * the executor will manage execution. So ensure we do not define + * module_{init|exit} functions for the builtin case when registering + * suites via kunit_test_suites() below. */ -#define kunit_test_suites(suites_list...) \ - static struct kunit_suite *suites[] = {suites_list, NULL}; \ - static int kunit_test_suites_init(void) \ +#ifdef MODULE +#define kunit_test_suites_for_module(__suites) \ + static int __init kunit_test_suites_init(void) \ { \ + struct kunit_suite *suites[] = (__suites); \ return __kunit_test_suites_init(suites); \ } \ - late_initcall(kunit_test_suites_init); \ + module_init(kunit_test_suites_init); \ + \ static void __exit kunit_test_suites_exit(void) \ { \ return __kunit_test_suites_exit(suites); \ } \ module_exit(kunit_test_suites_exit) +#else +#define kunit_test_suites_for_module(__suites) +#endif /* MODULE */ + +#define __kunit_test_suites(unique_array, unique_suites, ...) \ + static struct kunit_suite *unique_array[] = { __VA_ARGS__, NULL }; \ + kunit_test_suites_for_module(unique_array); \ + static struct kunit_suite **unique_suites \ + __used __section(.kunit_test_suites) = unique_array + +/** + * kunit_test_suites() - used to register one or more &struct kunit_suite + * with KUnit. + * + * @suites: a statically allocated list of &struct kunit_suite. + * + * Registers @suites with the test framework. See &struct kunit_suite for + * more information. + * + * When builtin, KUnit tests are all run via executor; this is done + * by placing the array of struct kunit_suite * in the .kunit_test_suites + * ELF section. + * + * An alternative is to build the tests as a module. Because modules do not + * support multiple initcall()s, we need to initialize an array of suites for a + * module. + * + */ +#define kunit_test_suites(...) \ + __kunit_test_suites(__UNIQUE_ID(array), \ + __UNIQUE_ID(suites), \ + __VA_ARGS__)
#define kunit_test_suite(suite) kunit_test_suites(&suite)
diff --git a/lib/kunit/Makefile b/lib/kunit/Makefile index 724b94311ca36..c49f4ffb6273a 100644 --- a/lib/kunit/Makefile +++ b/lib/kunit/Makefile @@ -3,7 +3,8 @@ obj-$(CONFIG_KUNIT) += kunit.o kunit-objs += test.o \ string-stream.o \ assert.o \ - try-catch.o + try-catch.o \ + executor.o
ifeq ($(CONFIG_KUNIT_DEBUGFS),y) kunit-objs += debugfs.o diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c new file mode 100644 index 0000000000000..7015e7328dce7 --- /dev/null +++ b/lib/kunit/executor.c @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <kunit/test.h> + +/* + * These symbols point to the .kunit_test_suites section and are defined in + * include/asm-generic/vmlinux.lds.h, and consequently must be extern. + */ +extern struct kunit_suite * const * const __kunit_suites_start[]; +extern struct kunit_suite * const * const __kunit_suites_end[]; + +#if IS_BUILTIN(CONFIG_KUNIT) + +static int kunit_run_all_tests(void) +{ + struct kunit_suite * const * const *suites; + + for (suites = __kunit_suites_start; + suites < __kunit_suites_end; + suites++) + __kunit_test_suites_init(*suites); + + return 0; +} + +late_initcall(kunit_run_all_tests); + +#endif /* IS_BUILTIN(CONFIG_KUNIT) */ diff --git a/lib/kunit/test.c b/lib/kunit/test.c index ccb2ffad8dcfa..918dff400a9d7 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -380,7 +380,7 @@ static void kunit_init_suite(struct kunit_suite *suite) kunit_debugfs_create_suite(suite); }
-int __kunit_test_suites_init(struct kunit_suite **suites) +int __kunit_test_suites_init(struct kunit_suite * const * const suites) { unsigned int i;
Remove KUnit from init calls entirely, instead call directly from kernel_init().
Co-developed-by: Alan Maguire alan.maguire@oracle.com Signed-off-by: Alan Maguire alan.maguire@oracle.com Signed-off-by: Brendan Higgins brendanhiggins@google.com Reviewed-by: Stephen Boyd sboyd@kernel.org --- include/kunit/test.h | 9 +++++++++ init/main.c | 4 ++++ lib/kunit/executor.c | 4 +--- 3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/include/kunit/test.h b/include/kunit/test.h index d13965eb624d4..7cb1c47388c56 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -228,6 +228,15 @@ int __kunit_test_suites_init(struct kunit_suite * const * const suites);
void __kunit_test_suites_exit(struct kunit_suite **suites);
+#if IS_BUILTIN(CONFIG_KUNIT) +int kunit_run_all_tests(void); +#else +static inline int kunit_run_all_tests(void) +{ + return 0; +} +#endif /* IS_BUILTIN(CONFIG_KUNIT) */ + /** * kunit_test_suites() - used to register one or more &struct kunit_suite * with KUnit. diff --git a/init/main.c b/init/main.c index 0ead83e86b5aa..d3101d8874dea 100644 --- a/init/main.c +++ b/init/main.c @@ -106,6 +106,8 @@ #define CREATE_TRACE_POINTS #include <trace/events/initcall.h>
+#include <kunit/test.h> + static int kernel_init(void *);
extern void init_IRQ(void); @@ -1504,6 +1506,8 @@ static noinline void __init kernel_init_freeable(void)
do_basic_setup();
+ kunit_run_all_tests(); + console_on_rootfs();
/* diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index 7015e7328dce7..4aab7f70a88c3 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -11,7 +11,7 @@ extern struct kunit_suite * const * const __kunit_suites_end[];
#if IS_BUILTIN(CONFIG_KUNIT)
-static int kunit_run_all_tests(void) +int kunit_run_all_tests(void) { struct kunit_suite * const * const *suites;
@@ -23,6 +23,4 @@ static int kunit_run_all_tests(void) return 0; }
-late_initcall(kunit_run_all_tests); - #endif /* IS_BUILTIN(CONFIG_KUNIT) */
TAP 14 allows an optional test plan to be emitted before the start of the start of testing[1]; this is valuable because it makes it possible for a test harness to detect whether the number of tests run matches the number of tests expected to be run, ensuring that no tests silently failed.
Link[1]: https://github.com/isaacs/testanything.github.io/blob/tap14/tap-version-14-s... Signed-off-by: Brendan Higgins brendanhiggins@google.com Reviewed-by: Stephen Boyd sboyd@kernel.org --- lib/kunit/executor.c | 37 +++++++++ lib/kunit/test.c | 11 --- tools/testing/kunit/kunit_kernel.py | 2 +- tools/testing/kunit/kunit_parser.py | 74 +++++++++++++++--- .../test_is_test_passed-all_passed.log | Bin 1562 -> 1567 bytes .../test_data/test_is_test_passed-crash.log | Bin 3016 -> 3021 bytes .../test_data/test_is_test_passed-failure.log | Bin 1700 -> 1705 bytes 7 files changed, 100 insertions(+), 24 deletions(-)
diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c index 4aab7f70a88c3..38061d456afb2 100644 --- a/lib/kunit/executor.c +++ b/lib/kunit/executor.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0
+#include <linux/reboot.h> #include <kunit/test.h>
/* @@ -11,15 +12,51 @@ extern struct kunit_suite * const * const __kunit_suites_end[];
#if IS_BUILTIN(CONFIG_KUNIT)
+static char *kunit_shutdown; +core_param(kunit_shutdown, kunit_shutdown, charp, 0644); + +static void kunit_handle_shutdown(void) +{ + if (!kunit_shutdown) + return; + + if (!strcmp(kunit_shutdown, "poweroff")) + kernel_power_off(); + else if (!strcmp(kunit_shutdown, "halt")) + kernel_halt(); + else if (!strcmp(kunit_shutdown, "reboot")) + kernel_restart(NULL); + +} + +static void kunit_print_tap_header(void) +{ + struct kunit_suite * const * const *suites, * const *subsuite; + int num_of_suites = 0; + + for (suites = __kunit_suites_start; + suites < __kunit_suites_end; + suites++) + for (subsuite = *suites; *subsuite != NULL; subsuite++) + num_of_suites++; + + pr_info("TAP version 14\n"); + pr_info("1..%d\n", num_of_suites); +} + int kunit_run_all_tests(void) { struct kunit_suite * const * const *suites;
+ kunit_print_tap_header(); + for (suites = __kunit_suites_start; suites < __kunit_suites_end; suites++) __kunit_test_suites_init(*suites);
+ kunit_handle_shutdown(); + return 0; }
diff --git a/lib/kunit/test.c b/lib/kunit/test.c index 918dff400a9d7..b1835ccb3fce2 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -19,16 +19,6 @@ static void kunit_set_failure(struct kunit *test) WRITE_ONCE(test->success, false); }
-static void kunit_print_tap_version(void) -{ - static bool kunit_has_printed_tap_version; - - if (!kunit_has_printed_tap_version) { - pr_info("TAP version 14\n"); - kunit_has_printed_tap_version = true; - } -} - /* * Append formatted message to log, size of which is limited to * KUNIT_LOG_SIZE bytes (including null terminating byte). @@ -68,7 +58,6 @@ EXPORT_SYMBOL_GPL(kunit_suite_num_test_cases);
static void kunit_print_subtest_start(struct kunit_suite *suite) { - kunit_print_tap_version(); kunit_log(KERN_INFO, suite, KUNIT_SUBTEST_INDENT "# Subtest: %s", suite->name); kunit_log(KERN_INFO, suite, KUNIT_SUBTEST_INDENT "1..%zd", diff --git a/tools/testing/kunit/kunit_kernel.py b/tools/testing/kunit/kunit_kernel.py index 63dbda2d029f6..d6a575f92317c 100644 --- a/tools/testing/kunit/kunit_kernel.py +++ b/tools/testing/kunit/kunit_kernel.py @@ -172,7 +172,7 @@ class LinuxSourceTree(object): return self.validate_config(build_dir)
def run_kernel(self, args=[], build_dir='', timeout=None): - args.extend(['mem=1G']) + args.extend(['mem=1G', 'kunit_shutdown=halt']) outfile = 'test.log' self._ops.linux_bin(args, timeout, build_dir, outfile) subprocess.call(['stty', 'sane']) diff --git a/tools/testing/kunit/kunit_parser.py b/tools/testing/kunit/kunit_parser.py index 64aac9dcd4314..a8998a5effaad 100644 --- a/tools/testing/kunit/kunit_parser.py +++ b/tools/testing/kunit/kunit_parser.py @@ -45,6 +45,7 @@ class TestStatus(Enum): FAILURE = auto() TEST_CRASHED = auto() NO_TESTS = auto() + FAILURE_TO_PARSE_TESTS = auto()
kunit_start_re = re.compile(r'TAP version [0-9]+$') kunit_end_re = re.compile('(List of all partitions:|' @@ -109,7 +110,7 @@ OkNotOkResult = namedtuple('OkNotOkResult', ['is_ok','description', 'text'])
OK_NOT_OK_SUBTEST = re.compile(r'^[\s]+(ok|not ok) [0-9]+ - (.*)$')
-OK_NOT_OK_MODULE = re.compile(r'^(ok|not ok) [0-9]+ - (.*)$') +OK_NOT_OK_MODULE = re.compile(r'^(ok|not ok) ([0-9]+) - (.*)$')
def parse_ok_not_ok_test_case(lines: List[str], test_case: TestCase) -> bool: save_non_diagnositic(lines, test_case) @@ -197,7 +198,9 @@ def max_status(left: TestStatus, right: TestStatus) -> TestStatus: else: return TestStatus.SUCCESS
-def parse_ok_not_ok_test_suite(lines: List[str], test_suite: TestSuite) -> bool: +def parse_ok_not_ok_test_suite(lines: List[str], + test_suite: TestSuite, + expected_suite_index: int) -> bool: consume_non_diagnositic(lines) if not lines: test_suite.status = TestStatus.TEST_CRASHED @@ -210,6 +213,12 @@ def parse_ok_not_ok_test_suite(lines: List[str], test_suite: TestSuite) -> bool: test_suite.status = TestStatus.SUCCESS else: test_suite.status = TestStatus.FAILURE + suite_index = int(match.group(2)) + if suite_index != expected_suite_index: + print_with_timestamp( + red('[ERROR] ') + 'expected_suite_index ' + + str(expected_suite_index) + ', but got ' + + str(suite_index)) return True else: return False @@ -222,7 +231,7 @@ def bubble_up_test_case_errors(test_suite: TestSuite) -> TestStatus: max_test_case_status = bubble_up_errors(lambda x: x.status, test_suite.cases) return max_status(max_test_case_status, test_suite.status)
-def parse_test_suite(lines: List[str]) -> TestSuite: +def parse_test_suite(lines: List[str], expected_suite_index: int) -> TestSuite: if not lines: return None consume_non_diagnositic(lines) @@ -241,7 +250,7 @@ def parse_test_suite(lines: List[str]) -> TestSuite: break test_suite.cases.append(test_case) expected_test_case_num -= 1 - if parse_ok_not_ok_test_suite(lines, test_suite): + if parse_ok_not_ok_test_suite(lines, test_suite, expected_suite_index): test_suite.status = bubble_up_test_case_errors(test_suite) return test_suite elif not lines: @@ -261,6 +270,17 @@ def parse_tap_header(lines: List[str]) -> bool: else: return False
+TEST_PLAN = re.compile(r'[0-9]+..([0-9]+)') + +def parse_test_plan(lines: List[str]) -> int: + consume_non_diagnositic(lines) + match = TEST_PLAN.match(lines[0]) + if match: + lines.pop(0) + return int(match.group(1)) + else: + return None + def bubble_up_suite_errors(test_suite_list: List[TestSuite]) -> TestStatus: return bubble_up_errors(lambda x: x.status, test_suite_list)
@@ -269,19 +289,34 @@ def parse_test_result(lines: List[str]) -> TestResult: return TestResult(TestStatus.NO_TESTS, [], lines) consume_non_diagnositic(lines) if not parse_tap_header(lines): - return None + return TestResult(TestStatus.NO_TESTS, [], lines) + expected_test_suite_num = parse_test_plan(lines) + if not expected_test_suite_num: + return TestResult(TestStatus.FAILURE_TO_PARSE_TESTS, [], lines) test_suites = [] - test_suite = parse_test_suite(lines) - while test_suite: - test_suites.append(test_suite) - test_suite = parse_test_suite(lines) - return TestResult(bubble_up_suite_errors(test_suites), test_suites, lines) + for i in range(1, expected_test_suite_num + 1): + test_suite = parse_test_suite(lines, i) + if test_suite: + test_suites.append(test_suite) + else: + print_with_timestamp( + red('[ERROR] ') + ' expected ' + + str(expected_test_suite_num) + + ' test suites, but got ' + str(i - 2)) + break + test_suite = parse_test_suite(lines, -1) + if test_suite: + print_with_timestamp(red('[ERROR] ') + + 'got unexpected test suite: ' + test_suite.name) + if test_suites: + return TestResult(bubble_up_suite_errors(test_suites), test_suites, lines) + else: + return TestResult(TestStatus.NO_TESTS, [], lines)
-def parse_run_tests(kernel_output) -> TestResult: +def print_and_count_results(test_result: TestResult) -> None: total_tests = 0 failed_tests = 0 crashed_tests = 0 - test_result = parse_test_result(list(isolate_kunit_output(kernel_output))) for test_suite in test_result.suites: if test_suite.status == TestStatus.SUCCESS: print_suite_divider(green('[PASSED] ') + test_suite.name) @@ -303,6 +338,21 @@ def parse_run_tests(kernel_output) -> TestResult: print_with_timestamp(red('[FAILED] ') + test_case.name) print_log(map(yellow, test_case.log)) print_with_timestamp('') + return total_tests, failed_tests, crashed_tests + +def parse_run_tests(kernel_output) -> TestResult: + total_tests = 0 + failed_tests = 0 + crashed_tests = 0 + test_result = parse_test_result(list(isolate_kunit_output(kernel_output))) + if test_result.status == TestStatus.NO_TESTS: + print(red('[ERROR] ') + yellow('no tests run!')) + elif test_result.status == TestStatus.FAILURE_TO_PARSE_TESTS: + print(red('[ERROR] ') + yellow('could not parse test results!')) + else: + (total_tests, + failed_tests, + crashed_tests) = print_and_count_results(test_result) print_with_timestamp(DIVIDER) fmt = green if test_result.status == TestStatus.SUCCESS else red print_with_timestamp( diff --git a/tools/testing/kunit/test_data/test_is_test_passed-all_passed.log b/tools/testing/kunit/test_data/test_is_test_passed-all_passed.log index 62ebc0288355c4b122ccc18ae2505f971efa57bc..bc0dc8fe35b760b1feb74ec419818dbfae1adb5c 100644 GIT binary patch delta 28 jcmbQmGoME|#4$jjEVZaOGe1wk(1goSPtRy09}gP<dC~`u
delta 23 ecmbQwGmD2W#4$jjEVZaOGe1wk&}5@94;uhhkp{*9
diff --git a/tools/testing/kunit/test_data/test_is_test_passed-crash.log b/tools/testing/kunit/test_data/test_is_test_passed-crash.log index 0b249870c8be417a5865bd40a24c8597bb7f5ab1..4d97f6708c4a5ad5bb2ac879e12afca6e816d83d 100644 GIT binary patch delta 15 WcmX>hepY;fFN>j`p3z318g2k9Uj*m?
delta 10 RcmX>renNbL@5Z2NZU7lr1S$Xk
diff --git a/tools/testing/kunit/test_data/test_is_test_passed-failure.log b/tools/testing/kunit/test_data/test_is_test_passed-failure.log index 9e89d32d5667a59d137f8adacf3a88fdb7f88baf..7a416497e3bec044eefc1535f7d84ee85703ba97 100644 GIT binary patch delta 28 jcmZ3&yOLKp#4$jjEVZaOGe1wk(1goSPtRy0-!wJ=eKrU$
delta 23 ecmZ3<yM&i7#4$jjEVZaOGe1wk&}5_VG&TTPhX-Z=
Add kunit_shutdown, an option to specify that the kernel shutsdown after running KUnit tests, to the kernel-parameters.txt documentation.
Signed-off-by: Brendan Higgins brendanhiggins@google.com Reviewed-by: Stephen Boyd sboyd@kernel.org --- Documentation/admin-guide/kernel-parameters.txt | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index fb95fad81c79a..e7d5eb7249e7f 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -2183,6 +2183,14 @@ 0: force disabled 1: force enabled
+ kunit_shutdown=[KERNEL UNIT TESTING FRAMEWORK] Shutdown kernel after + running built-in tests. Tests configured as modules will + not be run. + Default: (flag not present) don't shutdown + poweroff: poweroff the kernel after running tests + halt: halt the kernel after running tests + reboot: reboot the kernel after running tests + kvm.ignore_msrs=[KVM] Ignore guest accesses to unhandled MSRs. Default is 0 (don't ignore, but inject #GP)
Add a brief blurb saying how and when the kunit_test_suite() macro works to the usage documentation.
Signed-off-by: Brendan Higgins brendanhiggins@google.com --- Documentation/dev-tools/kunit/usage.rst | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst index 3c3fe8b5feccf..961d3ea3ca19a 100644 --- a/Documentation/dev-tools/kunit/usage.rst +++ b/Documentation/dev-tools/kunit/usage.rst @@ -211,6 +211,11 @@ KUnit test framework. .. note:: A test case will only be run if it is associated with a test suite.
+``kunit_test_suite(...)`` is a macro which tells the linker to put the specified +test suite in a special linker section so that it can be run by KUnit either +after late_init, or when the test module is loaded (depending on whether the +test was built in or not). + For more information on these types of things see the :doc:`api/test`.
Isolating Behavior
Glad this is back out there: a couple of minor nitpicks below:
On Thu, Jun 25, 2020 at 4:58 AM Brendan Higgins brendanhiggins@google.com wrote:
## TL;DR
This patchset adds a centralized executor to dispatch tests rather than relying on late_initcall to schedule each test suite separately along with a couple of new features that depend on it.
Also, sorry for the extreme delay in getting this out. Part of the delay came from finding that there were actually several architectures that the previous revision of this patchset didn't work on, so I went through and attempted to test this patchset on every architecture - more on that later.
## What am I trying to do?
Conceptually, I am trying to provide a mechanism by which test suites can be grouped together so that they can be reasoned about collectively. The last two of three patches in this series add features which depend on this:
PATCH 8/11 Prints out a test plan[1] right before KUnit tests are run; this is valuable because it makes it possible for a test harness to detect whether the number of tests run matches the number of tests expected to be run, ensuring that no tests silently failed. The test plan includes a count of tests that will run. With the centralized executor, the tests are located in a single data structure and thus can be counted.
This appears to actually be patch 9/11.
PATCH 9/11 Add a new kernel command-line option which allows the user to specify that the kernel poweroff, halt, or reboot after completing all KUnit tests; this is very handy for running KUnit tests on UML or a VM so that the UML/VM process exits cleanly immediately after running all tests without needing a special initramfs. The centralized executor provides a definitive point when all tests have completed and the poweroff, halt, or reboot could occur.
This seems to have been merged into the above patch (9/11).
In addition, by dispatching tests from a single location, we can guarantee that all KUnit tests run after late_init is complete, which was a concern during the initial KUnit patchset review (this has not been a problem in practice, but resolving with certainty is nevertheless desirable).
Other use cases for this exist, but the above features should provide an idea of the value that this could provide.
## Changes since last revision:
- On the last revision I got some messages from 0day that showed that this patchset didn't work on several architectures, one issue that this patchset addresses is that we were aligning both memory segments as well as structures in the segments to specific byte boundaries which was incorrect.
- The issue mentioned above also caused me to test on additional architectures which revealed that some architectures other than UML do not use the default init linker section macro that most architectures use. There are now several new patches (2, 3, 4, and 6).
- Fixed a formatting consistency issue in the kernel params documentation patch (9/9).
- Add a brief blurb on how and when the kunit_test_suite macro works.
## Remaining work to be done:
The only architecture for which I was able to get a compiler, but was apparently unable to get KUnit into a section that the executor to see was m68k - not sure why.
Alan Maguire (1): kunit: test: create a single centralized executor for all tests
Brendan Higgins (10): vmlinux.lds.h: add linker section for KUnit test suites arch: arm64: add linker section for KUnit test suites arch: microblaze: add linker section for KUnit test suites arch: powerpc: add linker section for KUnit test suites arch: um: add linker section for KUnit test suites arch: xtensa: add linker section for KUnit test suites init: main: add KUnit to kernel init kunit: test: add test plan to KUnit TAP format Documentation: Add kunit_shutdown to kernel-parameters.txt Documentation: kunit: add a brief blurb about kunit_test_suite
.../admin-guide/kernel-parameters.txt | 8 ++ Documentation/dev-tools/kunit/usage.rst | 5 ++ arch/arm64/kernel/vmlinux.lds.S | 3 + arch/microblaze/kernel/vmlinux.lds.S | 4 + arch/powerpc/kernel/vmlinux.lds.S | 4 + arch/um/include/asm/common.lds.S | 4 + arch/xtensa/kernel/vmlinux.lds.S | 4 + include/asm-generic/vmlinux.lds.h | 8 ++ include/kunit/test.h | 73 ++++++++++++----- init/main.c | 4 + lib/kunit/Makefile | 3 +- lib/kunit/executor.c | 63 +++++++++++++++ lib/kunit/test.c | 13 +-- tools/testing/kunit/kunit_kernel.py | 2 +- tools/testing/kunit/kunit_parser.py | 74 +++++++++++++++--- .../test_is_test_passed-all_passed.log | Bin 1562 -> 1567 bytes .../test_data/test_is_test_passed-crash.log | Bin 3016 -> 3021 bytes .../test_data/test_is_test_passed-failure.log | Bin 1700 -> 1705 bytes 18 files changed, 226 insertions(+), 46 deletions(-) create mode 100644 lib/kunit/executor.c
base-commit: 4333a9b0b67bb4e8bcd91bdd80da80b0ec151162 prerequisite-patch-id: 2d4b5aa9fa8ada9ae04c8584b47c299a822b9455 prerequisite-patch-id: 582b6d9d28ce4b71628890ec832df6522ca68de0
These patches are available for download with dependencies here:
https://kunit-review.googlesource.com/c/linux/+/3829
[1] https://github.com/isaacs/testanything.github.io/blob/tap14/tap-version-14-s... [2] https://patchwork.kernel.org/patch/11383635/
-- 2.27.0.212.ge8ba1cc988-goog
On Wed, Jun 24, 2020 at 6:47 PM David Gow davidgow@google.com wrote:
Glad this is back out there: a couple of minor nitpicks below:
On Thu, Jun 25, 2020 at 4:58 AM Brendan Higgins brendanhiggins@google.com wrote:
## TL;DR
This patchset adds a centralized executor to dispatch tests rather than relying on late_initcall to schedule each test suite separately along with a couple of new features that depend on it.
Also, sorry for the extreme delay in getting this out. Part of the delay came from finding that there were actually several architectures that the previous revision of this patchset didn't work on, so I went through and attempted to test this patchset on every architecture - more on that later.
## What am I trying to do?
Conceptually, I am trying to provide a mechanism by which test suites can be grouped together so that they can be reasoned about collectively. The last two of three patches in this series add features which depend on this:
PATCH 8/11 Prints out a test plan[1] right before KUnit tests are run; this is valuable because it makes it possible for a test harness to detect whether the number of tests run matches the number of tests expected to be run, ensuring that no tests silently failed. The test plan includes a count of tests that will run. With the centralized executor, the tests are located in a single data structure and thus can be counted.
This appears to actually be patch 9/11.
PATCH 9/11 Add a new kernel command-line option which allows the user to specify that the kernel poweroff, halt, or reboot after completing all KUnit tests; this is very handy for running KUnit tests on UML or a VM so that the UML/VM process exits cleanly immediately after running all tests without needing a special initramfs. The centralized executor provides a definitive point when all tests have completed and the poweroff, halt, or reboot could occur.
This seems to have been merged into the above patch (9/11).
Whoops, good catch.
Fixed in v5!
In addition, by dispatching tests from a single location, we can guarantee that all KUnit tests run after late_init is complete, which was a concern during the initial KUnit patchset review (this has not been a problem in practice, but resolving with certainty is nevertheless desirable).
Other use cases for this exist, but the above features should provide an idea of the value that this could provide.
## Changes since last revision:
- On the last revision I got some messages from 0day that showed that this patchset didn't work on several architectures, one issue that this patchset addresses is that we were aligning both memory segments as well as structures in the segments to specific byte boundaries which was incorrect.
- The issue mentioned above also caused me to test on additional architectures which revealed that some architectures other than UML do not use the default init linker section macro that most architectures use. There are now several new patches (2, 3, 4, and 6).
- Fixed a formatting consistency issue in the kernel params documentation patch (9/9).
- Add a brief blurb on how and when the kunit_test_suite macro works.
## Remaining work to be done:
The only architecture for which I was able to get a compiler, but was apparently unable to get KUnit into a section that the executor to see was m68k - not sure why.
Alan Maguire (1): kunit: test: create a single centralized executor for all tests
Brendan Higgins (10): vmlinux.lds.h: add linker section for KUnit test suites arch: arm64: add linker section for KUnit test suites arch: microblaze: add linker section for KUnit test suites arch: powerpc: add linker section for KUnit test suites arch: um: add linker section for KUnit test suites arch: xtensa: add linker section for KUnit test suites init: main: add KUnit to kernel init kunit: test: add test plan to KUnit TAP format Documentation: Add kunit_shutdown to kernel-parameters.txt Documentation: kunit: add a brief blurb about kunit_test_suite
.../admin-guide/kernel-parameters.txt | 8 ++ Documentation/dev-tools/kunit/usage.rst | 5 ++ arch/arm64/kernel/vmlinux.lds.S | 3 + arch/microblaze/kernel/vmlinux.lds.S | 4 + arch/powerpc/kernel/vmlinux.lds.S | 4 + arch/um/include/asm/common.lds.S | 4 + arch/xtensa/kernel/vmlinux.lds.S | 4 + include/asm-generic/vmlinux.lds.h | 8 ++ include/kunit/test.h | 73 ++++++++++++----- init/main.c | 4 + lib/kunit/Makefile | 3 +- lib/kunit/executor.c | 63 +++++++++++++++ lib/kunit/test.c | 13 +-- tools/testing/kunit/kunit_kernel.py | 2 +- tools/testing/kunit/kunit_parser.py | 74 +++++++++++++++--- .../test_is_test_passed-all_passed.log | Bin 1562 -> 1567 bytes .../test_data/test_is_test_passed-crash.log | Bin 3016 -> 3021 bytes .../test_data/test_is_test_passed-failure.log | Bin 1700 -> 1705 bytes 18 files changed, 226 insertions(+), 46 deletions(-) create mode 100644 lib/kunit/executor.c
base-commit: 4333a9b0b67bb4e8bcd91bdd80da80b0ec151162 prerequisite-patch-id: 2d4b5aa9fa8ada9ae04c8584b47c299a822b9455 prerequisite-patch-id: 582b6d9d28ce4b71628890ec832df6522ca68de0
These patches are available for download with dependencies here:
https://kunit-review.googlesource.com/c/linux/+/3829
[1] https://github.com/isaacs/testanything.github.io/blob/tap14/tap-version-14-s... [2] https://patchwork.kernel.org/patch/11383635/
-- 2.27.0.212.ge8ba1cc988-goog
linux-kselftest-mirror@lists.linaro.org