33d4a933 | 12-May-2022 |
Daniel Latypov <dlatypov@google.com> |
kunit: tool: remove dead parse_crash_in_log() logic
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 li
kunit: tool: remove dead parse_crash_in_log() logic
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>
show more ...
|
9660209d | 29-Mar-2022 |
Daniel Latypov <dlatypov@google.com> |
kunit: tool: print clearer error message when there's no TAP output
Before: $ ./tools/testing/kunit/kunit.py parse /dev/null ... [ERROR] Test : invalid KTAP input!
After: $ ./tools/testing/kunit/ku
kunit: tool: print clearer error message when there's no TAP output
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>
show more ...
|
3f0a50f3 | 12-May-2022 |
Daniel Latypov <dlatypov@google.com> |
kunit: tool: stop using a shell to run kernel under QEMU
Note: this potentially breaks custom qemu_configs if people are using them! But the fix for them is simple, don't specify multiple arguments
kunit: tool: stop using a shell to run kernel under QEMU
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>
show more ...
|
c2497643 | 08-Apr-2022 |
Daniel Latypov <dlatypov@google.com> |
kunit: tool: update test counts summary line format
Before: > Testing complete. Passed: 137, Failed: 0, Crashed: 0, Skipped: 36, Errors: 0
After: > Testing complete. Ran 173 tests: passed: 137, ski
kunit: tool: update test counts summary line format
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 acd8e8407b8f ("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>
show more ...
|
baa33315 | 26-Feb-2022 |
Daniel Latypov <dlatypov@google.com> |
kunit: tool: more descriptive metavars/--help output
Before, our help output contained lines like --kconfig_add KCONFIG_ADD --qemu_config qemu_config --jobs jobs
They're not very helpful.
Th
kunit: tool: more descriptive metavars/--help output
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>
show more ...
|
d34f82d6 | 23-Feb-2022 |
Kees Cook <keescook@chromium.org> |
kunit: tool: Do not colorize output when redirected
Filling log files with color codes makes diffs and other comparisons difficult. Only emit vt100 codes when the stdout is a TTY.
Cc: Brendan Higgi
kunit: tool: Do not colorize output when redirected
Filling log files with color codes makes diffs and other comparisons difficult. Only emit vt100 codes when the stdout is a TTY.
Cc: Brendan Higgins <brendanhiggins@google.com> Cc: linux-kselftest@vger.kernel.org Cc: kunit-dev@googlegroups.com Signed-off-by: Kees Cook <keescook@chromium.org> Reviewed-by: David Gow <davidgow@google.com> Reviewed-by: Brendan Higgins <brendanhiggins@google.com> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
show more ...
|
885210d3 | 24-Feb-2022 |
Daniel Latypov <dlatypov@google.com> |
kunit: tool: properly report the used arch for --json, or '' if not known
Before, kunit.py always printed "arch": "UM" in its json output, but... 1. With `kunit.py parse`, we could be parsing output
kunit: tool: properly report the used arch for --json, or '' if not known
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 87c9c1631788 ("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>
show more ...
|
ee96d25f | 24-Feb-2022 |
Daniel Latypov <dlatypov@google.com> |
kunit: tool: refactor how we plumb metadata into JSON
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
kunit: tool: refactor how we plumb metadata into JSON
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>
show more ...
|
aa1c0555 | 18-Jan-2022 |
Daniel Latypov <dlatypov@google.com> |
kunit: tool: simplify code since build_dir can't be None
--build_dir is set to a default of '.kunit' since commit ddbd60c779b4 ("kunit: use --build_dir=.kunit as default"), but even before then it w
kunit: tool: simplify code since build_dir can't be None
--build_dir is set to a default of '.kunit' since commit ddbd60c779b4 ("kunit: use --build_dir=.kunit as default"), but even before then it was explicitly set to ''.
So outside of one unit test, there was no way for the build_dir to be ever be None, and we can simplify code by fixing the unit test and enforcing that via updated type annotations.
E.g. this lets us drop `get_file_path()` since it's now exactly equivalent to os.path.join().
Note: there's some `if build_dir` checks that also fail if build_dir is explicitly set to '' that just guard against passing "O=" to make. But running `make O=` works just fine, so drop these checks.
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>
show more ...
|
e6f61920 | 18-Jan-2022 |
Daniel Latypov <dlatypov@google.com> |
kunit: tool: drop last uses of collections.namedtuple
Since we formally require python3.7+ since commit df4b0807ca1a ("kunit: tool: Assert the version requirement"), we can just use @dataclasses.dat
kunit: tool: drop last uses of collections.namedtuple
Since we formally require python3.7+ since commit df4b0807ca1a ("kunit: tool: Assert the version requirement"), we can just use @dataclasses.dataclass instead.
In kunit_config.py, we used namedtuple to create a hashable type that had `name` and `value` fields and had to subclass it to define a custom `__str__()`. @datalcass lets us just define one type instead.
In qemu_config.py, we use namedtuple to allow modules to define various parameters. Using @dataclass, we can add type-annotations for all these fields, making our code more typesafe and making it easier for users to figure out how to define new configs.
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>
show more ...
|
89aa72cd | 18-Jan-2022 |
Daniel Latypov <dlatypov@google.com> |
kunit: tool: drop unused KernelDirectoryPath var
Commit be886ba90cce ("kunit: run kunit_tool from any directory") introduced this variable, but it was unused even in that commit.
Since it's still u
kunit: tool: drop unused KernelDirectoryPath var
Commit be886ba90cce ("kunit: run kunit_tool from any directory") introduced this variable, but it was unused even in that commit.
Since it's still unused now and callers can instead use get_kernel_root_path(), delete this var.
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>
show more ...
|
de4d73b1 | 27-Jan-2022 |
Daniel Latypov <dlatypov@google.com> |
kunit: fix missing f in f-string in run_checks.py
We're missing the `f` prefix to have python do string interpolation, so we'd never end up printing what the actual "unexpected" error is.
Fixes: ee
kunit: fix missing f in f-string in run_checks.py
We're missing the `f` prefix to have python do string interpolation, so we'd never end up printing what the actual "unexpected" error is.
Fixes: ee92ed38364e ("kunit: add run_checks.py script to validate kunit changes") 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>
show more ...
|
ad659ccb | 15-Dec-2021 |
David Gow <davidgow@google.com> |
kunit: tool: Default --jobs to number of CPUs
The --jobs parameter for kunit_tool currently defaults to 8 CPUs, regardless of the number available. For systems with significantly more (or less), thi
kunit: tool: Default --jobs to number of CPUs
The --jobs parameter for kunit_tool currently defaults to 8 CPUs, regardless of the number available. For systems with significantly more (or less), this is not as efficient. Instead, default --jobs to the number of CPUs available to the process: while there are as many superstitions as to exactly what the ideal jobs:CPU ratio is, this seems sufficiently sensible to me.
A new helper function to get the default number of jobs is added: get_default_jobs() -- this is used in kunit_tool_test instead of a hardcoded value, or an explicit call to len(os.sched_getaffinity()), so should be more flexible if this needs to change in the future.
Signed-off-by: David Gow <davidgow@google.com> Signed-off-by: Daniel Latypov <dlatypov@google.com> Reviewed-by: Daniel Latypov <dlatypov@google.com> Reviewed-by: Brendan Higgins <brendanhiggins@google.com> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
show more ...
|
85310a62 | 15-Dec-2021 |
Daniel Latypov <dlatypov@google.com> |
kunit: tool: fix newly introduced typechecker errors
After upgrading mypy and pytype from pip, we see 2 new errors when running ./tools/testing/kunit/run_checks.py.
Error #1: mypy and pytype They n
kunit: tool: fix newly introduced typechecker errors
After upgrading mypy and pytype from pip, we see 2 new errors when running ./tools/testing/kunit/run_checks.py.
Error #1: mypy and pytype They now deduce that importlib.util.spec_from_file_location() can return None and note that we're not checking for this.
We validate that the arch is valid (i.e. the file exists) beforehand. Add in an `asssert spec is not None` to appease the checkers.
Error #2: pytype bug https://github.com/google/pytype/issues/1057 It doesn't like `from datetime import datetime`, specifically that a type shares a name with a module.
We can workaround this by either * renaming the import or just using `import datetime` * passing the new `--fix-module-collisions` flag to pytype.
We pick the first option for now because * the flag is quite new, only in the 2021.11.29 release. * I'd prefer if people can just run `pytype <file>`
Signed-off-by: Daniel Latypov <dlatypov@google.com> Reviewed-by: Brendan Higgins <brendanhiggins@google.com> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
show more ...
|
1ee2ba89 | 14-Dec-2021 |
Daniel Latypov <dlatypov@google.com> |
kunit: tool: make `build` subcommand also reconfigure if needed
If I created a kunitconfig file that was incomplete, then $ ./tools/testing/kunit/kunit.py build --kunitconfig=my_kunitconfig would si
kunit: tool: make `build` subcommand also reconfigure if needed
If I created a kunitconfig file that was incomplete, then $ ./tools/testing/kunit/kunit.py build --kunitconfig=my_kunitconfig would silently drop all the options with unmet dependencies!
This is because it doesn't do the config check that `kunit.py config` does.
So if I want to safely build a kernel for testing, I have to do $ ./tools/testing/kunit/kunit.py config <flags> $ ./tools/testing/kunit/kunit.py build <flags, again>
It seems unlikely that any user of kunit.py would want the current `build` semantics. So make it effectively do `kunit.py config` + `kunit.py build`.
Signed-off-by: Daniel Latypov <dlatypov@google.com> Reviewed-by: Brendan Higgins <brendanhiggins@google.com> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
show more ...
|
e0cc8c05 | 14-Dec-2021 |
Daniel Latypov <dlatypov@google.com> |
kunit: tool: delete kunit_parser.TestResult type
The `log` field is unused, and the `status` field is accessible via `test.status`.
So it's simpler to just return the main `Test` object directly.
kunit: tool: delete kunit_parser.TestResult type
The `log` field is unused, and the `status` field is accessible via `test.status`.
So it's simpler to just return the main `Test` object directly.
And since we're no longer returning a namedtuple, which has no type annotations, this hopefully means typecheckers are better equipped to find any errors.
Signed-off-by: Daniel Latypov <dlatypov@google.com> Reviewed-by: Brendan Higgins <brendanhiggins@google.com> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
show more ...
|
db167981 | 14-Dec-2021 |
Daniel Latypov <dlatypov@google.com> |
kunit: tool: use dataclass instead of collections.namedtuple
namedtuple is a terse way of defining a collection of fields. However, it does not allow us to annotate the type of these fields. It also
kunit: tool: use dataclass instead of collections.namedtuple
namedtuple is a terse way of defining a collection of fields. However, it does not allow us to annotate the type of these fields. It also doesn't let us have any sort of inheritance between types.
Since commit df4b0807ca1a ("kunit: tool: Assert the version requirement"), kunit.py has asserted that it's running on python >=3.7.
So in that case use a 3.7 feature, dataclasses, to replace these.
Changes in detail: * Make KunitExecRequest contain all the fields needed for exec_tests * Use inheritance to dedupe fields * also allows us to e.g. pass a KUnitRequest in as a KUnitParseRequest * this has changed around the order of some fields * Use named arguments when constructing all request objects in kunit.py * This is to prevent accidentally mixing up fields, etc.
Signed-off-by: Daniel Latypov <dlatypov@google.com> Reviewed-by: Brendan Higgins <brendanhiggins@google.com> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
show more ...
|
7fa7ffcf | 19-Nov-2021 |
Daniel Latypov <dlatypov@google.com> |
kunit: tool: suggest using decode_stacktrace.sh on kernel crash
kunit.py isn't very clear that 1) it stashes a copy of the unparsed output in $BUILD_DIR/test.log 2) it sets $BUILD_DIR=.kunit by defa
kunit: tool: suggest using decode_stacktrace.sh on kernel crash
kunit.py isn't very clear that 1) it stashes a copy of the unparsed output in $BUILD_DIR/test.log 2) it sets $BUILD_DIR=.kunit by default
So it's trickier than it should be for a user to come up with the right command to do so.
Make kunit.py print out a command for this if a) we saw a test case crash b) we only ran one kernel (test.log only contains output from the last)
Example suggested command: $ scripts/decode_stacktrace.sh .kunit/vmlinux .kunit < .kunit/test.log | tee .kunit/decoded.log | ./tools/testing/kunit/kunit.py parse
Without debug info a user might see something like [14:11:25] Call Trace: [14:11:25] ? kunit_binary_assert_format (:?) [14:11:25] kunit_try_run_case (test.c:?) [14:11:25] ? __kthread_parkme (kthread.c:?) [14:11:25] kunit_generic_run_threadfn_adapter (try-catch.c:?) [14:11:25] ? kunit_generic_run_threadfn_adapter (try-catch.c:?) [14:11:25] kthread (kthread.c:?) [14:11:25] new_thread_handler (:?) [14:11:25] [CRASHED]
`tee` is in GNU coreutils, so it seems fine to add that into the pipeline by default, that way users can inspect the otuput in more detail.
Note: to turn on debug info, users would need to do something like $ echo -e 'CONFIG_DEBUG_KERNEL=y\nCONFIG_DEBUG_INFO=y' >> .kunit/.kunitconfig $ ./tools/testing/kunit/kunit.py config $ ./tools/testing/kunit/kunit.py build $ <then run decode_stacktrace.sh now vmlinux is updated>
This feels too clunky to include in the instructions. With --kconfig_add [1], it would become a bit less painful.
[1] https://lore.kernel.org/linux-kselftest/20211106013058.2621799-2-dlatypov@google.com/
Signed-off-by: Daniel Latypov <dlatypov@google.com> Reviewed-by: Brendan Higgins <brendanhiggins@google.com> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
show more ...
|
4c2911f1 | 19-Nov-2021 |
Daniel Latypov <dlatypov@google.com> |
kunit: tool: reconfigure when the used kunitconfig changes
Problem: currently, if you remove something from your kunitconfig, kunit.py will not regenerate the .config file. The same thing happens if
kunit: tool: reconfigure when the used kunitconfig changes
Problem: currently, if you remove something from your kunitconfig, kunit.py will not regenerate the .config file. The same thing happens if you did --kunitconfig_add=CONFIG_KASAN=y [1] and then ran again without it. Your new run will still have KASAN.
The reason is that kunit.py won't regenerate the .config file if it's a superset of the kunitconfig. This speeds it up a bit for iterating.
This patch adds an additional check that forces kunit.py to regenerate the .config file if the current kunitconfig doesn't match the previous one.
What this means: * deleting entries from .kunitconfig works as one would expect * dropping a --kunitconfig_add also triggers a rebuild * you can still edit .config directly to turn on new options
We implement this by creating a `last_used_kunitconfig` file in the build directory (so .kunit, by default) after we generate the .config. When comparing the kconfigs, we compare python sets, so duplicates and permutations don't trip us up.
The majority of this patch is adding unit tests for the existing logic and for the new case where `last_used_kunitconfig` differs.
[1] https://lore.kernel.org/linux-kselftest/20211106013058.2621799-2-dlatypov@google.com/
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>
show more ...
|
c44895b6 | 11-Nov-2021 |
Daniel Latypov <dlatypov@google.com> |
kunit: tool: revamp message for invalid kunitconfig
The current error message is precise, but not very clear if you don't already know what it's talking about, e.g.
> $ make ARCH=um olddefconfig O=
kunit: tool: revamp message for invalid kunitconfig
The current error message is precise, but not very clear if you don't already know what it's talking about, e.g.
> $ make ARCH=um olddefconfig O=.kunit > ERROR:root:Provided Kconfig is not contained in validated .config. Following fields found in kunitconfig, but not in .config: CONFIG_DRM=y
Try to reword the error message so that it's * your missing options usually have unsatisified dependencies * if you're on UML, that might be the cause (it is, in this example)
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>
show more ...
|