| Message ID | 172920085854.4578.9203147717033046574.stgit@skinsburskii-cloud-desktop.internal.cloudapp.net (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Brendan Higgins |
| Headers | show |
| Series | kunit: Introduce autorun option | expand |
On Thu, Oct 17, 2024 at 5:34 PM Stanislav Kinsburskii <skinsburskii@linux.microsoft.com> 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@linux.microsoft.com> 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@googlegroups.com. > 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.
On Tue, Oct 22, 2024 at 05:16:31PM -0400, Rae Moar wrote: > > 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? > Sure, I'll address it in the next revision. Thanks, Stanislav
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); } - __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);
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@linux.microsoft.com> --- 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(-)