On Thu, Oct 17, 2024 at 5:34 PM Stanislav Kinsburskii
<skinsburskii@xxxxxxxxxxxxxxxxxxx> wrote:
>
> The new option controls tests run on boot or module load. With the new
> debugfs "run" dentry allowing to run tests on demand, an ability to disable
> automatic tests run becomes a useful option in case of intrusive tests.
>
> The option is set to true by default to preserve the existent behavior. It
> can be overridden by either the corresponding module option or by the
> corresponding config build option.
>
> Signed-off-by: Stanislav Kinsburskii <skinsburskii@xxxxxxxxxxxxxxxxxxx>
Hello!
This patch looks good to me! The functionality seems to be working
well. I do have one comment below.
Thanks!
-Rae
> ---
> include/kunit/test.h | 4 +++-
> lib/kunit/Kconfig | 12 ++++++++++++
> lib/kunit/debugfs.c | 2 +-
> lib/kunit/executor.c | 18 +++++++++++++++++-
> lib/kunit/test.c | 6 ++++--
> 5 files changed, 37 insertions(+), 5 deletions(-)
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 34b71e42fb10..58dbab60f853 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -312,6 +312,7 @@ static inline void kunit_set_failure(struct kunit *test)
> }
>
> bool kunit_enabled(void);
> +bool kunit_autorun(void);
> const char *kunit_action(void);
> const char *kunit_filter_glob(void);
> char *kunit_filter(void);
> @@ -334,7 +335,8 @@ kunit_filter_suites(const struct kunit_suite_set *suite_set,
> int *err);
> void kunit_free_suite_set(struct kunit_suite_set suite_set);
>
> -int __kunit_test_suites_init(struct kunit_suite * const * const suites, int num_suites);
> +int __kunit_test_suites_init(struct kunit_suite * const * const suites, int num_suites,
> + bool run_tests);
>
> void __kunit_test_suites_exit(struct kunit_suite **suites, int num_suites);
>
> diff --git a/lib/kunit/Kconfig b/lib/kunit/Kconfig
> index 34d7242d526d..a97897edd964 100644
> --- a/lib/kunit/Kconfig
> +++ b/lib/kunit/Kconfig
> @@ -81,4 +81,16 @@ config KUNIT_DEFAULT_ENABLED
> In most cases this should be left as Y. Only if additional opt-in
> behavior is needed should this be set to N.
>
> +config KUNIT_AUTORUN_ENABLED
> + bool "Default value of kunit.autorun"
> + default y
> + help
> + Sets the default value of kunit.autorun. If set to N then KUnit
> + tests will not run after initialization unless kunit.autorun=1 is
> + passed to the kernel command line. The test can still be run manually
> + via debugfs interface.
> +
> + In most cases this should be left as Y. Only if additional opt-in
> + behavior is needed should this be set to N.
> +
> endif # KUNIT
> diff --git a/lib/kunit/debugfs.c b/lib/kunit/debugfs.c
> index d548750a325a..9df064f40d98 100644
> --- a/lib/kunit/debugfs.c
> +++ b/lib/kunit/debugfs.c
> @@ -145,7 +145,7 @@ static ssize_t debugfs_run(struct file *file,
> struct inode *f_inode = file->f_inode;
> struct kunit_suite *suite = (struct kunit_suite *) f_inode->i_private;
>
> - __kunit_test_suites_init(&suite, 1);
> + __kunit_test_suites_init(&suite, 1, true);
>
> return count;
> }
> diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
> index 34b7b6833df3..340723571b0f 100644
> --- a/lib/kunit/executor.c
> +++ b/lib/kunit/executor.c
> @@ -29,6 +29,22 @@ const char *kunit_action(void)
> return action_param;
> }
>
> +/*
> + * Run KUnit tests after initialization
> + */
> +#ifdef CONFIG_KUNIT_AUTORUN_ENABLED
> +static bool autorun_param = true;
> +#else
> +static bool autorun_param;
> +#endif
> +module_param_named(autorun, autorun_param, bool, 0);
> +MODULE_PARM_DESC(autorun, "Run KUnit tests after initialization");
> +
> +bool kunit_autorun(void)
> +{
> + return autorun_param;
> +}
> +
> static char *filter_glob_param;
> static char *filter_param;
> static char *filter_action_param;
> @@ -266,7 +282,7 @@ void kunit_exec_run_tests(struct kunit_suite_set *suite_set, bool builtin)
> pr_info("1..%zu\n", num_suites);
When using this feature, I still see some KTAP output that are printed
from this function (kunit_exec_run_tests). I think it would be great
if we could remove this output as to not clutter the kernel log.
At first, I was confused as to why we needed to call this function and
initialize the tests but I realized the debugfs suites need to be
created.
So instead, could we check for kunit_autorun() here instead as a
condition before printing the output?
> }
>
> - __kunit_test_suites_init(suite_set->start, num_suites);
> + __kunit_test_suites_init(suite_set->start, num_suites, kunit_autorun());
> }
>
> void kunit_exec_list_tests(struct kunit_suite_set *suite_set, bool include_attr)
> diff --git a/lib/kunit/test.c b/lib/kunit/test.c
> index 089c832e3cdb..146d1b48a096 100644
> --- a/lib/kunit/test.c
> +++ b/lib/kunit/test.c
> @@ -708,7 +708,8 @@ bool kunit_enabled(void)
> return enable_param;
> }
>
> -int __kunit_test_suites_init(struct kunit_suite * const * const suites, int num_suites)
> +int __kunit_test_suites_init(struct kunit_suite * const * const suites, int num_suites,
> + bool run_tests)
> {
> unsigned int i;
>
> @@ -731,7 +732,8 @@ int __kunit_test_suites_init(struct kunit_suite * const * const suites, int num_
>
> for (i = 0; i < num_suites; i++) {
> kunit_init_suite(suites[i]);
> - kunit_run_tests(suites[i]);
> + if (run_tests)
> + kunit_run_tests(suites[i]);
> }
>
> static_branch_dec(&kunit_running);
>
>
> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@xxxxxxxxxxxxxxxx.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/172920085854.4578.9203147717033046574.stgit%40skinsburskii-cloud-desktop.internal.cloudapp.net.
