On 4/13/20 4:15 PM, Tim Bird wrote:
> From: Tim Bird <[email protected]>
>
> 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?
> 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.
> Signed-off-by: Tim Bird <[email protected]>
> ---
> 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 <[email protected]>
> +L: [email protected]
> +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.
> KERNEL UNIT TESTING FRAMEWORK (KUnit)
> M: Brendan Higgins <[email protected]>
> L: [email protected]
> 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.
> +# 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.
thanks,
-- Shuah
> -----Original Message-----
> From: shuah <[email protected]>
>
> On 4/13/20 4:15 PM, Tim Bird wrote:
> > From: Tim Bird <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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 <[email protected]>
> > +L: [email protected]
> > +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 <[email protected]>
> > L: [email protected]
> > 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
On 4/14/20 10:06 AM, Bird, Tim wrote:
>> -----Original Message-----
>> From: shuah <[email protected]>
>>
>> On 4/13/20 4:15 PM, Tim Bird wrote:
>>> From: Tim Bird <[email protected]>
>>>
>>> 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
> ???
>
Sorry for the dangling sentence. :) Can we parse the make kselftest-all
output instead?
>>> 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.
>
>>
I still want to see a way to use output from build and install
steps and parse it, instead of a whole new script.
>>
>>> Signed-off-by: Tim Bird <[email protected]>
>>> ---
>>> 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 <[email protected]>
>>> +L: [email protected]
>>> +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?
>
Yeah. checkpatch warn in this case is reminder in case a new entry
is needed. A new entry isn't necessary in most cases.
>>
>>> KERNEL UNIT TESTING FRAMEWORK (KUnit)
>>> M: Brendan Higgins <[email protected]>
>>> L: [email protected]
>>> 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.
>
Agreed. I think it is still a better idea to parse to find error and
display them.
> 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.
>
Right. Hence the reason why I am asking if you explored using exiting
parsing build and install output. Approaching this as a parsing also
reduces maintenance overhead for this script. I am not questioning the
value of the script, I am asking about the approach.
> 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.
>
We have efforts such as lkft and soon kernelci do cross-compiles.
powerpc tests have good coverage and so do arm. The problem is tests
that can run on multiple architectures at times come in without good
support.
>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).
Testing as service by Fuego and fixes to problems you find is great.
thanks,
-- Shuah