On Thu, Oct 17, 2019 at 07:07:17PM +0100, Alan Maguire wrote:
> as tests are added to kunit, it will become less feasible to execute
^
nit: please capitalize "as".
> all built tests together. By supporting modular tests we provide
> a simple way to do selective execution on a running system; specifying
>
> CONFIG_KUNIT=y
> CONFIG_KUNIT_EXAMPLE_TEST=m
>
> ...means we can simply "insmod example-test.ko" to run the tests.
>
> To achieve this we need to
>
> o export the required symbols in kunit
> o for non-exported symbols, we need to utilize kunit_find_symbol;
> the simplest way is for the test suite init to call
> KUNIT_INIT_[FN|VAR]_SYMBOL() for each non-exported symbol.
> o support a new way of declaring test suites. Because a module cannot
> do multiple late_initcall()s, we provide a kunit_test_suites() macro
> to declare multiple suites within the same module at once.
>
> When compiled as a module, use of KUNIT_INIT_[FN|VAR]_symbol() will
> retrieve the symbol address via kunit_find_symbol() and assign a local
> variable with the same symbol name appropriately. When compiled builtin,
> these definitions are used to verify that the types we specify match
> the type of the symbol we are looking for. Compiler errors will be
> generated if not.
>
> One wrinkle here is that we cannot use the same names for local function
> pointer definitions; the reason for this is we have likely #included
> a definition for the function in question already, so an attempt to
> redefine it as a function pointer variable fails. As a result the
> KUNIT_INIT_FN_SYMBOL() macro requires a name for a local symbol we
> have defined as a function pointer (with a signature matching the
> desired function).
>
> Signed-off-by: Alan Maguire <alan.maguire@xxxxxxxxxx>
> Signed-off-by: Knut Omang <knut.omang@xxxxxxxxxx>
> ---
> include/kunit/test.h | 115 +++++++++++++++++++++++++++++++++++++----
> kernel/sysctl-test.c | 4 +-
> lib/Kconfig.debug | 2 +-
> lib/kunit/Kconfig | 4 +-
> lib/kunit/assert.c | 8 +++
> lib/kunit/example-test.c | 4 +-
> lib/kunit/string-stream-test.c | 44 ++++++++++++----
> lib/kunit/test-test.c | 32 ++++++++----
> lib/kunit/test.c | 9 ++++
> lib/kunit/try-catch.c | 2 +
> 10 files changed, 187 insertions(+), 37 deletions(-)
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index c645d18..9a3835a 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -12,6 +12,7 @@
> #include <kunit/assert.h>
> #include <kunit/try-catch.h>
> #include <linux/kernel.h>
> +#include <linux/module.h>
> #include <linux/slab.h>
> #include <linux/types.h>
>
> @@ -78,6 +79,86 @@ struct kunit_resource {
> struct list_head node;
> };
>
> +/**
> + * KUNIT_VAR_SYMBOL - A helper for defining non-exported variable symbols
> + *
> + * @name: name of the symbol.
> + * @type: type of symbol.
> + *
> + * In the module case, we define the pointer to the symbol type where
> + * we will store the symbol address; KUNIT_INIT_VAR_SYMBOL() will assign
> + * the symbol name to the dereferenced kunit_<symbol_name>. Note that
> + * in the builtin case we still define kunit_<symbol_name>; the reason
> + * for this is it allows us to verify that the type value is correct
> + * in the builtin case and has not fallen out-of-sync with its original
Very clever! Can you maybe elaborate on how? I didn't understand until I
looked at the initialization code. It would probably be sufficient to
just tell the reader to look at the initialization code.
> + * definition.
> + */
> +#ifdef MODULE
> +#define KUNIT_VAR_SYMBOL(symbol, type) \
> + type * kunit_##symbol; \
> + type symbol
> +#else
> +#define KUNIT_VAR_SYMBOL(symbol, type) \
> + type * kunit_##symbol
> +#endif
> +
> +/**
> + * KUNIT_INIT_VAR_SYMBOL - A helper for initializing non-exported variable
> + * symbols
> + * @test: optional pointer to test context
> + * @name: name of symbol
> + *
> + * In the module case, initialization consists of using kunit_find_symbol()
> + * to find the address of the symbol, and if found, we set the variable
> + * to the dereferenced address value. As mentioned above, in the builtin
> + * case we simply assing kunit_<symbol_name> to &<symbol_name> ; this will
> + * generate a compilation warning if the type we specified in KUNIT_VAR_SYMBOL
> + * and the type of the symbol itself do not match.
> + */
> +#ifdef MODULE
> +#define KUNIT_INIT_VAR_SYMBOL(test, symbol) \
> + do { \
> + if (!(kunit_##symbol)) { \
> + kunit_##symbol = kunit_find_symbol(#symbol); \
> + if (!IS_ERR((kunit_##symbol))) \
> + symbol = *(kunit_##symbol); \
> + } \
> + if (test) \
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, \
> + kunit_##symbol); \
> + } while (0)
> +#else
> +#define KUNIT_INIT_VAR_SYMBOL(test, symbol) \
> + kunit_##symbol = &(symbol)
> +#endif
> +
> +/**
> + * KUNIT_INIT_FN_SYMBOL - A helper for initializing non-exported function
> + * symbols
> + * @test: optional pointer to test context
> + * @symbol: name of symbol
> + * @name: local name of function used to store function pointer to symbol
> + *
> + * In the module case, initialization consists of using kunit_find_symbol()
> + * to find the address of the symbol, and if found, we set function pointer
> + * name to the function address value. In the non-module case, we simply
> + * assign name to symbol; this will generate a compilation error if the
> + * type we specified for function pointer @name does not match the symbol
> + * function type.
> + */
> +#ifdef MODULE
> +#define KUNIT_INIT_FN_SYMBOL(test, symbol, name) \
> + do { \
> + if (!name) \
> + name = kunit_find_symbol(#symbol); \
> + if (test) \
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, name); \
> + } while (0)
> +#else
> +#define KUNIT_INIT_FN_SYMBOL(test, symbol, name) \
> + name = symbol
> +#endif
> +
Can you put all the KUNIT_*_SYMBOL stuff in another patchset along with
the kunit_find_symbol?
> struct kunit;
>
> /**
> @@ -197,31 +278,45 @@ struct kunit {
> int kunit_run_tests(struct kunit_suite *suite);
>
> /**
> - * kunit_test_suite() - used to register a &struct kunit_suite with KUnit.
> + * kunit_test_suites() - used to register one or more &struct kunit_suite
> + * with KUnit.
> *
> - * @suite: a statically allocated &struct kunit_suite.
> + * @suites: a statically allocated list of &struct kunit_suite.
> *
> - * Registers @suite with the test framework. See &struct kunit_suite for
> + * Registers @suites with the test framework. See &struct kunit_suite for
> * more information.
> *
> - * NOTE: Currently KUnit tests are all run as late_initcalls; this means
> + * 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@xxxxxxxxxx): 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.
> */
> -#define kunit_test_suite(suite) \
> - static int kunit_suite_init##suite(void) \
> - { \
> - return kunit_run_tests(&suite); \
> - } \
> - late_initcall(kunit_suite_init##suite)
> +#define kunit_test_suites(...) \
> + static struct kunit_suite *suites[] = { __VA_ARGS__, NULL}; \
> + static int kunit_test_suites_init(void) \
> + { \
> + unsigned int i; \
> + for (i = 0; suites[i] != NULL; i++) \
> + kunit_run_tests(suites[i]); \
> + return 0; \
> + } \
> + late_initcall(kunit_test_suites_init); \
> + static void __exit kunit_test_suites_exit(void) \
> + { \
> + return; \
> + } \
> + module_exit(kunit_test_suites_exit)
>
> /*
> * Like kunit_alloc_resource() below, but returns the struct kunit_resource
