[PATCH v2 1/6] kunit: Kconfig: enable a KUNIT_RUN_ALL fragment

[PATCH v2 1/6] kunit: Kconfig: enable a KUNIT_RUN_ALL fragment

[Anders Roxell]

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"
+	help
+	  Enables all KUnit tests, if they can be enabled.
+	  That depends on if KUnit is enabled as a module or builtin.
+
 endif # KUNIT
-- 
2.20.1

Re: [PATCH v2 1/6] kunit: Kconfig: enable a KUNIT_RUN_ALL fragment

[David Gow]

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

Re: [PATCH v2 1/6] kunit: Kconfig: enable a KUNIT_RUN_ALL fragment

[Anders Roxell]

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

Re: [PATCH v2 1/6] kunit: Kconfig: enable a KUNIT_RUN_ALL fragment

[David Gow]

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