RE: [PATCH] selftest: add a script to perform selftest compile tests

[Bird, Tim]

> -----Original Message-----
> From: shuah <shuah@xxxxxxxxxx>
> 
> On 4/13/20 4:15 PM, Tim Bird wrote:
> > From: Tim Bird <tim.bird@xxxxxxxx>
> >
> > Add ksft-compile-test.sh.  This is a program used to test
> > cross-compilation and installation of selftest tests.
> >
> > See the test usage for help
> >
> > This program currently tests 3 scenarios out of a larger matrix
> > of possibly interesting scenarios.  For each scenario, it conducts
> > multiple tests for correctness.  This version tests:
> >   1) does the test compile
> 
> Is it necessary to write this long a script? Could we just parse
> the "kselftest
???

> >   2) is the kernel source directory clean after the compile
> 
> Can you use make mrproper and see if anything needs cleaning?
I'll check into this.  Does 'make mrproper' return an error code if
it found something that needed cleaning?  Or do I have to parse
stuff.  The actual code to check if the directory is clean is pretty
short.

> 
> >   3) does the test install operation succeed
> >   4) does the test run script reference the test
> >
> 
> I like the idea of being able to test, however I am not convinced
> you would need a whole new script for it.

The current build system is broken in a few different ways.
I have only enabled a few test cases out of the test matrix, to
be able to isolate some of the obvious problems from individual
target areas.  One of the reasons I wrote a full script was to more easily
enable additional tests, once functionality in the current build
system was fixed, to notify us of regressions going forward.

> 
> 
> > Signed-off-by: Tim Bird <tim.bird@xxxxxxxx>
> > ---
> >   MAINTAINERS                                  |   6 +
> >   tools/testing/selftests/ksft-compile-test.sh | 567 +++++++++++++++++++++++++++
> >   2 files changed, 573 insertions(+)
> >   create mode 100755 tools/testing/selftests/ksft-compile-test.sh
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index cc1d18c..a6289c7 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -9127,6 +9127,12 @@ S:	Maintained
> >   F:	tools/testing/selftests/
> >   F:	Documentation/dev-tools/kselftest*
> >
> > +KERNEL SELFTEST SELFTEST
> > +M:	Tim Bird <tim.bird@xxxxxxxx>
> > +L:	linux-kselftest@xxxxxxxxxxxxxxx
> > +S:	Maintained
> > +F:	tools/testing/selftests/ksft-compile-test.sh
> > +
> 
> Please don't add another entry to the MAINTAINERS file just
> for a shell script under tools/testing/selftests
> 
> This doesn't make sense.
OK. I only added this to eliminate a checkpatch.pl warning.
It seems like overkill to me also, but I was trying to obey the tools.
:-)

Maybe that warning from checkpatch is too aggressive?

> 
> >   KERNEL UNIT TESTING FRAMEWORK (KUnit)
> >   M:	Brendan Higgins <brendanhiggins@xxxxxxxxxx>
> >   L:	linux-kselftest@xxxxxxxxxxxxxxx
> > diff --git a/tools/testing/selftests/ksft-compile-test.sh b/tools/testing/selftests/ksft-compile-test.sh
> > new file mode 100755
> > index 0000000..e36e858
> > --- /dev/null
> > +++ b/tools/testing/selftests/ksft-compile-test.sh
> > @@ -0,0 +1,567 @@
> > +#!/bin/bash
> > +# SPDX-License-Identifier: GPL-2.0-only OR MIT
> > +#
> > +# ksft-compile-test.sh - test compiling Linux kernel selftests under lots of
> > +#   different configurations.  This is used to check that cross-compilation
> > +#   and install works properly for a newly submitted test target, and
> > +#   also that changes to existing test Makefiles don't regress with regard to
> > +#   this functionality.
> > +#
> > +# Copyright 2020 Sony Corporation
> > +#
> > +# Here are the things that Shuah Kahn asked for on 3/6/2020
> > +# 1. Cross-compilation & relocatable build support
> > +# 2. Generates objects in objdir/kselftest without cluttering main objdir
> > +# 3. Leave source directory clean
> > +# 4. Installs correctly in objdir/kselftest/kselftest_install and adds
> > +#        itself to run_kselftest.sh script generated during install.
> > +#
> 
> I was asking for fixes to individual tests.
Well, I used this to find some things to fix.  I have some patches queued,
but I thought the tool might be useful for others.  I'll send the patches
instead of posting the tool.

> 
> > +# Would be nice to make sure other features also work:
> > +# 5. can use absolute, relative, or current directory for output directory
> > +# 6. can use ~ in output directory path
> > +#
> 
> I do think this can be achieved with a simpler script wrapper around
> existing commands and kselftest_install.sh instead of writing a whole
> new shell script.

Well, my pain point is the build system itself, not kselftest_install.sh.
There are still some bugs in the build system, and it appears that people
still sometimes submit new tests with subtle problems compiling under
different build configurations.

My goal was to be able to test a whole matrix of build configurations,
to detect these problems.  But making a generic system to test a matrix
of configurations requires more than just putting together a few wrapper
scripts.  However, I'm not as familiar with the existing commands as you
are, so maybe I missed some functionality I could reuse.

One of the significant problems here, IMO, is that since most kernel developers
don't cross-compile, it introduces a whole range of potential
errors in the build system that they can't really test for.

The reason that the script is so long is that it tries to be bullet-proof
in the face of running on different systems with different build configurations.
Even at that, this is a first draft of the test.  To make it even more resilient
and useful, I would expect the script to get even longer as additional build
configurations are added, and different tests for correctness are added.

I originally wrote this as a Fuego test, but then thought that other kernel CI
systems might want to use it as well to test their kselftest build configurations.
So I re-wrote it as a generic, standalone script, so that others could use it.

I'm happy to leave this outside the kernel tree, and provide 'testing as a service'
(by Fuego!) to find bugs in the kselftest build system.  In that case, I'll just report
bugs that this finds (along with fixes where possible).

Note that I'm not recommending adding this to TARGETS.  I considered putting the
script itself in the tools directory, instead of tools/testing/selftests, as I thought that
maybe it should be separate from the rest of the selftest infrastructure.  In any event,
no one needs to run this if they don't want to. 

Please confirm that you'd rather not see this in tree, and I'll focus my efforts elsewhere.
 -- Tim