diff mbox series

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

Message ID 20200505102700.7912-1-anders.roxell@linaro.org (mailing list archive)
State New, archived
Headers show
Series Enable as many KUnit tests as possible | expand

Commit Message

Anders Roxell May 5, 2020, 10:27 a.m. UTC
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(+)

Comments

David Gow May 6, 2020, 5:08 a.m. UTC | #1
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
Anders Roxell May 6, 2020, 10:33 a.m. UTC | #2
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
David Gow May 7, 2020, 3:08 a.m. UTC | #3
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 mbox series

Patch

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