There are several tests which depend on PCI, and hence need a bunch of
extra options to run under UML. This makes it awkward to give
configuration instructions (whether in documentation, or as part of a
.kunitconfig file), as two separate, incompatible sets of config options
are required for UML and "most other architectures".
For non-UML architectures, it's possible to add default kconfig options
via the qemu_config python files, but there's no equivalent for UML. Add
a new tools/testing/kunit/configs/arch_uml.config file containing extra
kconfig options to use on UML.
Tested-by: José Expósito <jose.exposito89@gmail.com>
Reviewed-by: Daniel Latypov <dlatypov@google.com>
Signed-off-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
It's come up a few times that it would be useful to have --kunitconfig
be repeatable [1][2].
This could be done before with a bit of shell-fu, e.g.
$ find fs/ -name '.kunitconfig' -exec cat {} + | \
./tools/testing/kunit/kunit.py run --kunitconfig=/dev/stdin
or equivalently:
$ cat fs/ext4/.kunitconfig fs/fat/.kunitconfig | \
./tools/testing/kunit/kunit.py run --kunitconfig=/dev/stdin
But this can be fairly clunky to use in practice.
And having explicit support in kunit.py opens the door to having more
config fragments of interest, e.g. options for PCI on UML [1], UML
coverage [2], variants of tests [3].
There's another argument to be made that users can just use multiple
--kconfig_add's, but this gets very clunky very fast (e.g. [2]).
Note: there's a big caveat here that some kconfig options might be
incompatible. We try to give a clearish error message in the simple case
where the same option appears multiple times with conflicting values,
but more subtle ones (e.g. mutually exclusive options) will be
potentially very confusing for the user. I don't know we can do better.
Note 2: if you want to combine a --kunitconfig with the default, you
either have to do to specify the current build_dir
> --kunitconfig=.kunit --kunitconfig=additional.config
or
> --kunitconfig=tools/testing/kunit/configs/default.config --kunitconifg=additional.config
each of which have their downsides (former depends on --build_dir,
doesn't work if you don't have a .kunitconfig yet), etc.
Example with conflicting values:
> $ ./tools/testing/kunit/kunit.py config --kunitconfig=lib/kunit --kunitconfig=/dev/stdin <<EOF
> CONFIG_KUNIT_TEST=n
> CONFIG_KUNIT=m
> EOF
> ...
> kunit_kernel.ConfigError: Multiple values specified for 2 options in kunitconfig:
> CONFIG_KUNIT=y
> vs from /dev/stdin
> CONFIG_KUNIT=m
>
> CONFIG_KUNIT_TEST=y
> vs from /dev/stdin
> # CONFIG_KUNIT_TEST is not set
[1] https://lists.freedesktop.org/archives/dri-devel/2022-June/357616.html
[2] https://lore.kernel.org/linux-kselftest/CAFd5g45f3X3xF2vz2BkTHRqOC4uW6GZxtUUMaP5mwwbK8uNVtA@mail.gmail.com/
[3] https://lore.kernel.org/linux-kselftest/CANpmjNOdSy6DuO6CYZ4UxhGxqhjzx4tn0sJMbRqo2xRFv9kX6Q@mail.gmail.com/
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Now that kunit.py's --kunitconfig is repeatable, let's create a file to
hold the various options needed to enable coverage under UML.
This can be used like so:
$ ./tools/testing/kunit/kunit.py run \
--kunitconfig=tools/testing/kunit/configs/all_tests_uml.config \
--kunitconfig=tools/testing/kunit/configs/coverage_uml.config \
--make_options=CC=/usr/bin/gcc-6
which on my system is enough to get coverage working [1].
This is still a clunky command, but far better than before.
[1] at the time of this commit, I get:
Overall coverage rate:
lines......: 11.6% (34112 of 295033 lines)
functions..: 15.3% (3721 of 24368 functions)
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Currently, you cannot ovewrwrite what's in your kunitconfig via
--kconfig_add.
Nor can you override something in a qemu_config via either means.
This patch makes it so we have this level of priority
* --kconfig_add
* kunitconfig file (the default or the one from --kunitconfig)
* qemu_config
The rationale for this order is that the more "dynamic" sources of
kconfig options should take priority.
--kconfig_add is obviously the most dynamic.
And for kunitconfig, users probably tweak the file manually or specify
--kunitconfig more often than they delve into qemu_config python files.
And internally, we convert the kconfigs from a python list into a set or
dict fairly often. We should just use a dict internally.
We exposed the set transform in the past since we didn't define __eq__,
so also take the chance to shore up the kunit_kconfig.Kconfig interface.
Example
=======
Let's consider the unrealistic example where someone would want to
disable CONFIG_KUNIT.
I.e. they run
$ ./tools/testing/kunit/kunit.py config --kconfig_add=CONFIG_KUNIT=n
Before
------
We'd write the following
> # CONFIG_KUNIT is not set
> CONFIG_KUNIT_ALL_TESTS=y
> CONFIG_KUNIT_TEST=y
> CONFIG_KUNIT=y
> CONFIG_KUNIT_EXAMPLE_TEST=y
And we'd error out with
> ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config.
> This is probably due to unsatisfied dependencies.
> Missing: # CONFIG_KUNIT is not set
After
-----
We'd write the following
> # CONFIG_KUNIT is not set
> CONFIG_KUNIT_TEST=y
> CONFIG_KUNIT_ALL_TESTS=y
> CONFIG_KUNIT_EXAMPLE_TEST=y
And we'd error out with
> ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config.
> This is probably due to unsatisfied dependencies.
> Missing: CONFIG_KUNIT_EXAMPLE_TEST=y, CONFIG_KUNIT_TEST=y, CONFIG_KUNIT_ALL_TESTS=y
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Example usage:
$ ./tools/testing/kunit/kunit.py run --arch=x86_64 \
--kconfig_add=CONFIG_SMP=y --qemu_args='-smp 8'
Looking in the test.log, one can see
> smp: Bringing up secondary CPUs ...
> .... node #0, CPUs: #1#2#3#4#5#6#7
> smp: Brought up 1 node, 8 CPUs
This flag would allow people to make tweaks like this without having to
create custom qemu_config files.
For consistency with --kernel_args, we allow users to repeat this
argument, e.g. you can tack on a --qemu_args='-m 2048', or you could
just append it to the first string ('-smp 8 -m 2048').
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Drop get_source_tree_ops() and just call what used to be
get_source_tree_ops_from_qemu_config() in both cases.
Also rename the functions to have shorter names and add a "_" prefix to
note they're not meant to be used outside this function.
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Context:
When using a non-UML arch, kunit.py will boot the test kernel with
options like these by default (this is x86_64):
> mem=1G console=tty kunit_shutdown=halt console=ttyS0 kunit_shutdown=reboot
The first three options are added unconditionally but are only intended
for UML.
1. 'mem=1G' is redundant with the '-m 1024' that we hard-code into the
qemu commandline.
2. We specify a 'console' for all tools/testing/kunit/qemu_configs/*.py
already, so 'console=tty' gets overwritten.
3. For QEMU, we need to use 'reboot', and for UML we need to use 'halt'.
If you switch them, kunit.py will hang until the --timeout expires.
This patch:
Having these duplicate options is a bit noisy.
Switch so we only add UML-specific options for UML.
I.e. we now get
UML: 'mem=1G console=tty kunit_shutdown=halt' (unchanged)
x86_64: 'console=ttyS0 kunit_shutdown=reboot'
Side effect: you can't overwrite these options on UML w/ --kernel_arg.
But you already couldn't for QEMU (console, kunit_shutdown), and why
would you want to?
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Context:
* kunit_kernel.py is importing kunit_parser.py just to use the
print_with_timestamp() function
* the parser is directly printing to stdout, which will become an issue
if we ever try to run multiple kernels in parallel
This patch introduces a kunit_printer.py file and migrates callers of
kunit_parser.print_with_timestamp() to call
kunit_printer.stdout.print_with_timestamp() instead.
Future changes:
If we want to support showing results for parallel runs, we could then
create new Printer's that don't directly write to stdout and refactor
the code to pass around these Printer objects.
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Our main function currently has an optional `linux` argument which is
used to by our unit tests to inject a mock.
We currently have the same code copy-pasted several times to do
if not linux:
linux = MakeRealInstance(cli_args.foo, cli_args.bar, ...)
But in python, dependency injection isn't necessary or idiomatic when we
can just use mock.patch() to mock things out.
This change
1. adds a helper to create a LinuxSourceTree from the cli_args
2. drops the `linux` parameter in favor of mocking the __init__ func.
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
We're currently using the x86_64 qemu for i386 builds. While this is not
incorrect, it's probably more sensible to use the i386 one, which will
at least fail properly if we accidentally were to build a 64-bit kernel.
Signed-off-by: David Gow <davidgow@google.com>
Tested-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
The config for the serial console for riscv,
CONFIG_SERIAL_EARLYCON_RISCV_SBI, added a dependency,
CONFIG_RISCV_SBI_V01, at some point, so add that in to the base arch
config.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
It's often desirable (particularly in test automation) to run as many
tests as possible. This config enables all the tests which work as
builtins under UML at present, increasing the total tests run from 156
to 342 (not counting 36 'skipped' tests).
They can be run with:
./tools/testing/kunit/kunit.py run
--kunitconfig=./tools/testing/kunit/configs/all_tests_uml.config
This acts as an in-between point between the KUNIT_ALL_TESTS config
(which enables only tests whose dependencies are already enabled), and
the kunit_tool --alltests option, which tries to use allyesconfig,
taking a very long time to build and breaking very often.
Signed-off-by: David Gow <davidgow@google.com>
Tested-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
This primarily comes from running pylint over kunit tool code and
ignoring some warnings we don't care about.
If we ever got a fully clean setup, we could add this to run_checks.py,
but we're not there yet.
Fix things like
* Drop unused imports
* check `is None`, not `== None` (see PEP 8)
* remove redundant parens around returns
* remove redundant `else` / convert `elif` to `if` where appropriate
* rename make_arch_qemuconfig() param to base_kunitconfig (this is the
name used in the subclass, and it's a better one)
* kunit_tool_test: check the exit code for SystemExit (could be 0)
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
There should be no behavioral changes from this patch.
This patch removes redundant comment text, inlines a function used in
only one place, and other such minor tweaks.
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Consider this invocation
$ ./tools/testing/kunit/kunit.py parse <<EOF
TAP version 14
1..2
ok 1 - suite
# Subtest: no_tests_suite
# catastrophic error!
not ok 1 - no_tests_suite
EOF
It will have a 0 exit code even though there's a "not ok".
Consider this one:
$ ./tools/testing/kunit/kunit.py parse <<EOF
TAP version 14
1..2
ok 1 - suite
not ok 1 - no_tests_suite
EOF
It will a non-zero exit code.
Why?
We have this line in the kunit_parser.py
> parent_test = parse_test_header(lines, test)
where we have special handling when we see "# Subtest" and we ignore the
explicit reported "not ok 1" status!
Also, NO_TESTS at a suite-level only results in a non-zero status code
where then there's only one suite atm.
This change is the minimal one to make sure we don't overwrite it.
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
This logic depends on the kernel logging a message containing
'kunit test case crashed', but there is no corresponding logic to do so.
This is likely a relic of the revision process KUnit initially went
through when being upstreamed.
Delete it given
1) it's been missing for years and likely won't get implemented
2) the parser has been moving to be a more general KTAP parser,
kunit-only magic like this isn't how we'd want to implement it.
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Before:
$ ./tools/testing/kunit/kunit.py parse /dev/null
...
[ERROR] Test : invalid KTAP input!
After:
$ ./tools/testing/kunit/kunit.py parse /dev/null
...
[ERROR] Test <missing>: could not find any KTAP output!
This error message gets printed out when extract_tap_output() yielded no
lines. So while it could be because of malformed KTAP output from KUnit,
it could also be due to not having any KTAP output at all.
Try and make the error message here more clear.
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Note: this potentially breaks custom qemu_configs if people are using
them! But the fix for them is simple, don't specify multiple arguments
in one string and don't add on a redundant ''.
It feels a bit iffy to be using a shell in the first place.
There's the usual shenanigans where people could pass in arbitrary shell
commands via --kernel_arg (since we're just adding '' around the
kernel_cmdline) or via a custom qemu_config.
This isn't too much of a concern given the nature of this script (and
the qemu_config file is in python, you can do w/e you want already).
But it does have some other drawbacks.
One example of a kunit-specific pain point:
If the relevant qemu binary is missing, we get output like this:
> /bin/sh: line 1: qemu-system-aarch64: command not found
This in turn results in our KTAP parser complaining about
missing/invalid KTAP, but we don't directly show the error!
It's even more annoying to debug when you consider --raw_output only
shows KUnit output by default, i.e. you need --raw_output=all to see it.
Whereas directly invoking the binary, Python will raise a
FileNotFoundError for us, which is a noisier but more clear.
Making this change requires
* splitting parameters like ['-m 256'] into ['-m', '256'] in
kunit/qemu_configs/*.py
* change [''] to [] in kunit/qemu_configs/*.py since otherwise
QEMU fails w/ 'Device needs media, but drive is empty'
* dropping explicit quoting of the kernel cmdline
* using shlex.quote() when we print what command we're running
so the user can copy-paste and run it
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Before:
> Testing complete. Passed: 137, Failed: 0, Crashed: 0, Skipped: 36, Errors: 0
After:
> Testing complete. Ran 173 tests: passed: 137, skipped: 36
Even with our current set of statuses, the output is a bit verbose.
It could get worse in the future if we add more (e.g. timeout, kasan).
Let's only print the relevant ones.
I had previously been sympathetic to the argument that always
printing out all the statuses would make it easier to parse results.
But now we have commit acd8e8407b ("kunit: Print test statistics on
failure"), there are test counts printed out in the raw output.
We don't currently print out an overall total across all suites, but it
would be easy to add, if we see a need for that.
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Co-developed-by: David Gow <davidgow@google.com>
Signed-off-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Before, our help output contained lines like
--kconfig_add KCONFIG_ADD
--qemu_config qemu_config
--jobs jobs
They're not very helpful.
The former kind come from the automatic 'metavar' we get from argparse,
the uppercase version of the flag name.
The latter are where we manually specified metavar as the flag name.
After:
--build_dir DIR
--make_options X=Y
--kunitconfig PATH
--kconfig_add CONFIG_X=Y
--arch ARCH
--cross_compile PREFIX
--qemu_config FILE
--jobs N
--timeout SECONDS
--raw_output [{all,kunit}]
--json [FILE]
This patch tries to make the code more clear by specifying the _type_ of
input we expect, e.g. --build_dir is a DIR, --qemu_config is a FILE.
I also switched it to uppercase since it looked more clearly like
placeholder text that way.
This patch also changes --raw_output to specify `choices` to make it
more clear what the options are, and this way argparse can validate it
for us, as shown by the added test case.
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Before, kunit.py always printed "arch": "UM" in its json output, but...
1. With `kunit.py parse`, we could be parsing output from anywhere, so
we can't say that.
2. Capitalizing it is probably wrong, as it's `ARCH=um`
3. Commit 87c9c16317 ("kunit: tool: add support for QEMU") made it so
kunit.py could knowingly run a different arch, yet we'd still always
claim "UM".
This patch addresses all of those. E.g.
1.
$ ./tools/testing/kunit/kunit.py parse .kunit/test.log --json | grep -o '"arch.*' | sort -u
"arch": "",
2.
$ ./tools/testing/kunit/kunit.py run --json | ...
"arch": "um",
3.
$ ./tools/testing/kunit/kunit.py run --json --arch=x86_64 | ...
"arch": "x86_64",
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
When using --json, kunit.py run/exec/parse will produce results in
KernelCI json format.
As part of that, we include the build_dir that was used, and we
(incorrectly) hardcode in the arch, etc.
We'll want a way to plumb more values (as well as the correct `arch`),
so this patch groups those fields into kunit_json.Metadata type.
This patch should have no user visible changes.
And since we only used build_dir in KunitParseRequest for json, we can
now move it out of that struct and add it into KunitExecRequest, which
needs it and used to get it via inheritance.
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
Use a more idiomatic check that a list is non-empty (`if mylist:`) and
simplify the function body by dedenting and using a dict to map between
the kunit TestStatus enum => KernelCI json status string.
The dict hopefully makes it less likely to have bugs like commit
9a6bb30a88 ("kunit: tool: fix --json output for skipped tests").
Signed-off-by: Daniel Latypov <dlatypov@google.com>
Reviewed-by: David Gow <davidgow@google.com>
Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
