Re: [PATCH linux-kselftest/test v6] lib/list-test: add a test for the 'list' doubly linked list

[David Gow <davidgow@google.com>]

On Thu, Oct 31, 2019 at 11:51 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Thu, Oct 24, 2019 at 03:46:31PM -0700, David Gow wrote:
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 7ef985e01457..f3d0c6e42b97 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -9504,6 +9504,13 @@ F:     Documentation/misc-devices/lis3lv02d.rst
> >  F:   drivers/misc/lis3lv02d/
> >  F:   drivers/platform/x86/hp_accel.c
> >
> > +LIST KUNIT TEST
> > +M:   David Gow <davidgow@google.com>
> > +L:   linux-kselftest@vger.kernel.org
> > +L:   kunit-dev@googlegroups.com
> > +S:   Maintained
> > +F:   lib/list-test.c
>
> Should KUnit be the first name here? Then all KUnit tests appear in the
> same location in the MAINTAINERS file, or should it be like it is here,
> so that KUnit tests are close to the same-named area?

Thus far, we haven't standardised on anything re: MAINTAINERS entries
for tests. For the sysctl test, for instance, the file has been added
to the general "PROC SYSCTL" section.
There's no existing MAINTAINERS entry for list.h at all, though, so
that's couldn't be done here.

My suspicion is that it doesn't matter all that much (isn't everyone
just grepping MAINTAINERS anyway?), but that long-term, tests are more
likely to be being maintained in parallel with the code under test,
rather than in one group block of tests. I don't mind changing it if
anyone has stronger opinions, though...

> > +
> >  LIVE PATCHING
> >  M:   Josh Poimboeuf <jpoimboe@redhat.com>
> >  M:   Jiri Kosina <jikos@kernel.org>
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index a3017a5dadcd..6c1be6181e38 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -1961,6 +1961,24 @@ config SYSCTL_KUNIT_TEST
> >
> >         If unsure, say N.
> >
> > +config LIST_KUNIT_TEST
>
> Similarly for the Kconfig name. (Also aren't KUNIT and TEST redundant?)
>
> config KUNIT_LIST
>
> ?
>
> config LIST_KUNIT
>

This matches what's being done with the existing sysctl test, which
uses SYSCTL_KUNIT_TEST as its config name.
So, we've kind-of standardised on x_KUNIT_TEST thus far, even if it is
a bit redundant.

> > --- a/lib/Makefile
> > +++ b/lib/Makefile
> > @@ -292,3 +292,6 @@ obj-$(CONFIG_GENERIC_LIB_MULDI3) += muldi3.o
> >  obj-$(CONFIG_GENERIC_LIB_CMPDI2) += cmpdi2.o
> >  obj-$(CONFIG_GENERIC_LIB_UCMPDI2) += ucmpdi2.o
> >  obj-$(CONFIG_OBJAGG) += objagg.o
> > +
> > +# KUnit tests
> > +obj-$(CONFIG_LIST_KUNIT_TEST) += list-test.o
>
> And again, list-kunit.o? Other things have -test (or more commonly
> _test) suffixes. (So maybe list_kunit.o?)
>
> But as I said last time, I'll live with whatever, I'd just like a
> documented best-practice with a reasonable rationale. :)
>

Similarly, we've been going with a -test suffix thus far.

I definitely agree that these conventions should be documented, though.

Cheers,
-- David