| Message ID | 20200505102700.7912-1-anders.roxell@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Enable as many KUnit tests as possible | expand |
On Tue, May 5, 2020 at 6:27 PM Anders Roxell <anders.roxell@linaro.org> wrote: > > Make it easier to enable all KUnit fragments. This is needed for kernel > test-systems, so its easy to get all KUnit tests enabled and if new gets > added they will be enabled as well. Fragments that has to be builtin > will be missed if CONFIG_KUNIT_RUN_ALL is set as a module. > > Signed-off-by: Anders Roxell <anders.roxell@linaro.org> > --- > lib/kunit/Kconfig | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/lib/kunit/Kconfig b/lib/kunit/Kconfig > index 95d12e3d6d95..537f37bc8400 100644 > --- a/lib/kunit/Kconfig > +++ b/lib/kunit/Kconfig > @@ -41,4 +41,10 @@ config KUNIT_EXAMPLE_TEST > is intended for curious hackers who would like to understand how to > use KUnit for kernel development. > > +config KUNIT_RUN_ALL > + tristate "KUnit run all test" I'm not 100% sure about this name and description. It only actually "runs" the tests if they're builtin (as modules, they'll only run when loaded). Would KUNIT_BUILD_ALL or KUNIT_ALL_TESTS or similar be better? It also, as mentioned, only really runs all available (i.e., with dependencies met in the current .config) tests (as distinct from the --alltests flag to kunit.py, which builds UML with allyesconfig), it might be good to add this to the description or help. Something like "Enable all KUnit tests" or "Enable all available KUnit tests" (or even something about "all KUnit tests with satisfied dependencies") might be clearer. > + help > + Enables all KUnit tests, if they can be enabled. > + That depends on if KUnit is enabled as a module or builtin. > + I like the first line here, but the second one could use a bit more explanation. Maybe copy some of the boilerplate text from other tests, e.g.: KUnit tests run during boot and output the results to the debug log in TAP format (http://testanything.org/). Only useful for kernel devs running the KUnit test harness, and not intended for inclusion into a production build. For more information on KUnit and unit tests in general please refer to the KUnit documentation in Documentation/dev-tools/kunit/. If unsure, say N. > endif # KUNIT > -- > 2.20.1 > Otherwise, this is looking good. I've done some quick testing, and it all seems to work for me. As long as it's clear what the difference between this and other ways of running "all tests" (like the --alltests kunit.py option), I'm all for including this in. Cheers, -- David
Hi David, Thank you for the review. On Wed, 6 May 2020 at 07:08, David Gow <davidgow@google.com> wrote: > > On Tue, May 5, 2020 at 6:27 PM Anders Roxell <anders.roxell@linaro.org> wrote: > > > > Make it easier to enable all KUnit fragments. This is needed for kernel > > test-systems, so its easy to get all KUnit tests enabled and if new gets > > added they will be enabled as well. Fragments that has to be builtin > > will be missed if CONFIG_KUNIT_RUN_ALL is set as a module. > > > > Signed-off-by: Anders Roxell <anders.roxell@linaro.org> > > --- > > lib/kunit/Kconfig | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/lib/kunit/Kconfig b/lib/kunit/Kconfig > > index 95d12e3d6d95..537f37bc8400 100644 > > --- a/lib/kunit/Kconfig > > +++ b/lib/kunit/Kconfig > > @@ -41,4 +41,10 @@ config KUNIT_EXAMPLE_TEST > > is intended for curious hackers who would like to understand how to > > use KUnit for kernel development. > > > > +config KUNIT_RUN_ALL > > + tristate "KUnit run all test" > > I'm not 100% sure about this name and description. It only actually > "runs" the tests if they're builtin (as modules, they'll only run when > loaded). > > Would KUNIT_BUILD_ALL or KUNIT_ALL_TESTS I would like to go with KUNIT_ALL_TESTS if no one objects. > or similar be better? > > It also, as mentioned, only really runs all available (i.e., with > dependencies met in the current .config) tests (as distinct from the > --alltests flag to kunit.py, which builds UML with allyesconfig), it > might be good to add this to the description or help. > > Something like "Enable all KUnit tests" or "Enable all available KUnit > tests" (or even something about "all KUnit tests with satisfied > dependencies") might be clearer. I like "All KUnit tests with satisfied dependencies". > > > + help > > + Enables all KUnit tests, if they can be enabled. > > + That depends on if KUnit is enabled as a module or builtin. > > + > I like the first line here, but the second one could use a bit more > explanation. Maybe copy some of the boilerplate text from other tests, > e.g.: > > KUnit tests run during boot and output the results to the debug log > in TAP format (http://testanything.org/). Only useful for kernel devs > running the KUnit test harness, and not intended for inclusion into a > production build. > > For more information on KUnit and unit tests in general please refer > to the KUnit documentation in Documentation/dev-tools/kunit/. > > If unsure, say N. Makes much more sense, thanks. > > > endif # KUNIT > > -- > > 2.20.1 > > > > Otherwise, this is looking good. I've done some quick testing, and it > all seems to work for me. As long as it's clear what the difference > between this and other ways of running "all tests" (like the > --alltests kunit.py option), Do you think it should be made clearer in some way? > I'm all for including this in. > Cheers, Anders
On Wed, May 6, 2020 at 6:33 PM Anders Roxell <anders.roxell@linaro.org> wrote: > > Hi David, > > Thank you for the review. > > On Wed, 6 May 2020 at 07:08, David Gow <davidgow@google.com> wrote: > > > > On Tue, May 5, 2020 at 6:27 PM Anders Roxell <anders.roxell@linaro.org> wrote: > > > > > > Make it easier to enable all KUnit fragments. This is needed for kernel > > > test-systems, so its easy to get all KUnit tests enabled and if new gets > > > added they will be enabled as well. Fragments that has to be builtin > > > will be missed if CONFIG_KUNIT_RUN_ALL is set as a module. > > > > > > Signed-off-by: Anders Roxell <anders.roxell@linaro.org> > > > --- > > > lib/kunit/Kconfig | 6 ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/lib/kunit/Kconfig b/lib/kunit/Kconfig > > > index 95d12e3d6d95..537f37bc8400 100644 > > > --- a/lib/kunit/Kconfig > > > +++ b/lib/kunit/Kconfig > > > @@ -41,4 +41,10 @@ config KUNIT_EXAMPLE_TEST > > > is intended for curious hackers who would like to understand how to > > > use KUnit for kernel development. > > > > > > +config KUNIT_RUN_ALL > > > + tristate "KUnit run all test" > > > > I'm not 100% sure about this name and description. It only actually > > "runs" the tests if they're builtin (as modules, they'll only run when > > loaded). > > > > Would KUNIT_BUILD_ALL or KUNIT_ALL_TESTS > > I would like to go with KUNIT_ALL_TESTS if no one objects. > Personally, I'm fine with that. Brendan, thoughts? > > or similar be better? > > > > It also, as mentioned, only really runs all available (i.e., with > > dependencies met in the current .config) tests (as distinct from the > > --alltests flag to kunit.py, which builds UML with allyesconfig), it > > might be good to add this to the description or help. > > > > Something like "Enable all KUnit tests" or "Enable all available KUnit > > tests" (or even something about "all KUnit tests with satisfied > > dependencies") might be clearer. > > I like "All KUnit tests with satisfied dependencies". > > > > > > + help > > > + Enables all KUnit tests, if they can be enabled. > > > + That depends on if KUnit is enabled as a module or builtin. > > > + > > I like the first line here, but the second one could use a bit more > > explanation. Maybe copy some of the boilerplate text from other tests, > > e.g.: > > > > KUnit tests run during boot and output the results to the debug log > > in TAP format (http://testanything.org/). Only useful for kernel devs > > running the KUnit test harness, and not intended for inclusion into a > > production build. > > > > For more information on KUnit and unit tests in general please refer > > to the KUnit documentation in Documentation/dev-tools/kunit/. > > > > If unsure, say N. > > Makes much more sense, thanks. > > > > > > endif # KUNIT > > > -- > > > 2.20.1 > > > > > > > Otherwise, this is looking good. I've done some quick testing, and it > > all seems to work for me. As long as it's clear what the difference > > between this and other ways of running "all tests" (like the > > --alltests kunit.py option), > > Do you think it should be made clearer in some way? > I think the changes above should do it. -- David
diff --git a/lib/kunit/Kconfig b/lib/kunit/Kconfig index 95d12e3d6d95..537f37bc8400 100644 --- a/lib/kunit/Kconfig +++ b/lib/kunit/Kconfig @@ -41,4 +41,10 @@ config KUNIT_EXAMPLE_TEST is intended for curious hackers who would like to understand how to use KUnit for kernel development. +config KUNIT_RUN_ALL + tristate "KUnit run all test" + help + Enables all KUnit tests, if they can be enabled. + That depends on if KUnit is enabled as a module or builtin. + endif # KUNIT
Make it easier to enable all KUnit fragments. This is needed for kernel test-systems, so its easy to get all KUnit tests enabled and if new gets added they will be enabled as well. Fragments that has to be builtin will be missed if CONFIG_KUNIT_RUN_ALL is set as a module. Signed-off-by: Anders Roxell <anders.roxell@linaro.org> --- lib/kunit/Kconfig | 6 ++++++ 1 file changed, 6 insertions(+)