Add rust/.kunitconfig and document unit testing

[JoseExposito]

Hi everyone,

This series adds a rust/.kunitconfig file with the required configuration options to allow to easily run the KUnit tests without adding them manually using --kconfig_add.
A similar approach is used in other subsystems, like HID or DRM, to simplify running the tests.

I found information about the rusttest make target in the quick start guide, but, I wasn't able to find information about how to run the documentation tests, so I added a page explaining how to do it.

Note that --arch=x86_64 is mentioned in the documentation because UML is not working at the moment. Once #881 is merged, the flag can be dropped from the documentation.

cc @sulix

[sulix]

Thanks for adding this. I've tested it here and it all looks good to me.

Apart from possibly updating the title of the unit-testing page (as Miguel suggests), this is:

Reviewed-by: David Gow <[email protected]>

That being said, I do think the plan is ultimately to support running even rust-style unit tests under KUnit, where that makes sense, so this doc will hopefully need updating to support that at some point. Equally, as you note, it'd be nice for UML support to work again -- I suspect #881 is the right shorter-term solution for that, and the ideal fix will require some more serious work on the UML side so it doesn't do as many strange things.

As an aside, I'll note here (since I'm unlikely to have the time to work on it more this year), that I'd been experimenting with writing KUnit tests with Rust directly: i.e., without going through either the rust doctest/unit test features or using a C helper (which is how the doctests stuff is implemented): sulix@a7fed23
I'm not sure whether exposing a separate "kunit" API directly will ultimately be worth doing, or whether this is even the right path to go down for improvements to the current doctests implementation, but it's proving a good learning experience. (Though it turns out starting with the horrible unsafe FFI stuff is not a gentle introduction to the language... :-) )

[ojeda]

That being said, I do think the plan is ultimately to support running even rust-style unit tests under KUnit, where that makes sense, so this doc will hopefully need updating to support that at some point.

Definitely, the plan is to run the Rust-unit-tests in KUnit.

As an aside, I'll note here (since I'm unlikely to have the time to work on it more this year), that I'd been experimenting with writing KUnit tests with Rust directly: i.e., without going through either the rust doctest/unit test features or using a C helper (which is how the doctests stuff is implemented)

Thanks a lot for giving it a go! It is actually great to hear :)

I'm not sure whether exposing a separate "kunit" API directly will ultimately be worth doing, or whether this is even the right path to go down for improvements to the current doctests implementation,

About the C macros/helpers, in some cases we are looking into calling the C macros directly rather than re-implementing what they do (e.g. MODULE_* C macros in the module! Rust macro), since that usually means more complexity/maintenance. For similar reasons I went directly with that route for doctests (and I needed codegen anyway... :-) Of course, it needs to be evaluated case-by-case, but I think we shouldn't be shy of C code generation if that means we can reuse existing facilities (where it makes sense).

For the API itself (i.e. from the perspective of the user), I think it would be ideal to provide something like Rust #[test]s, i.e. let the compiler figure out which are the tests, without having to list them by the user (but with tweaks as needed, e.g. to allow for naming, creating KUnit suites, etc.). Last time I looked into it, I had to solve some details due to privacy and a compiler issue with the low-level test framework IIRC, but I think it would be great to figure that approach out -- it would make writing unit tests even simpler without manually handling the lists (i.e. kunit_case! into the statics). What do you think?

(Though it turns out starting with the horrible unsafe FFI stuff is not a gentle introduction to the language... :-) )

I think a few of us can relate... :)

[JoseExposito]

Thanks for adding this. I've tested it here and it all looks good to me.

Apart from possibly updating the title of the unit-testing page (as Miguel suggests), this is:

Reviewed-by: David Gow [email protected]

As always, thanks for reviewing my patches David, I added your reviewed-by :)

At the risk of self-promoting, maybe it's worth adding a reference to the KUnit or kunit.py documentation in Documentation/dev-tools/kunit (or Documentation/dev-tools/kunit/run_wrapper.rst ), which may provide a useful reference for more advanced use of kunit.py.

I also added a reference to the KUnit docs.

[dlatypov]

Note that --arch=x86_64 is mentioned in the documentation because UML is not working at the moment. Once #881 is merged, the flag can be dropped from the documentation.

Disclaimer: I have no idea what the UML incompatibility looks like atm (is it a compile time error? strange runtime errors?).
But if it's a confusing error, we can have kunit.py enforce we don't run against UML.

You can do so by adding either of these lines to the .kunitconfig

CONFIG_UML=n
# CONFIG_UML is not set

Sample error

Populating config with:
$ make ARCH=um O=.kunit olddefconfig
ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config.
This is probably due to unsatisfied dependencies.
Missing: # CONFIG_UML is not set
...

(I wish the error message here was better, but it's probably better than a strange runtime crash, and it'll happen faster)

[sulix]

Disclaimer: I have no idea what the UML incompatibility looks like atm (is it a compile time error? strange runtime errors?). But if it's a confusing error, we can have kunit.py enforce we don't run against UML.

On my machine, it shows up as rustc itself crashing (and, when that's fixed, a bunch of very strange relocation errors from the linker). So the kunit.py CONFIG_UML is not set error is, IMO, much more comprehensible.

So I'm all in favour of adding CONFIG_UML=n to the .kunitconfig.

That being said, I have just rebased #881 (and confirmed it still works) and so it may be equally possible to just merge that and make this work on UML anyway.

[sulix]

About the C macros/helpers, in some cases we are looking into calling the C macros directly rather than re-implementing what they do (e.g. MODULE_* C macros in the module! Rust macro), since that usually means more complexity/maintenance. For similar reasons I went directly with that route for doctests (and I needed codegen anyway... :-) Of course, it needs to be evaluated case-by-case, but I think we shouldn't be shy of C code generation if that means we can reuse existing facilities (where it makes sense).

Yeah: I agree that this is a judgement call. (Personally, I mostly wanted to experiment to see if it's possible.) A part of me does think that doing this in rust rather than generating C will be easier than debugging some of the build system requirements (especially for modules), but I suspect it's too early to tell for sure.

For the API itself (i.e. from the perspective of the user), I think it would be ideal to provide something like Rust #[test]s, i.e. let the compiler figure out which are the tests, without having to list them by the user (but with tweaks as needed, e.g. to allow for naming, creating KUnit suites, etc.). Last time I looked into it, I had to solve some details due to privacy and a compiler issue with the low-level test framework IIRC, but I think it would be great to figure that approach out -- it would make writing unit tests even simpler without manually handling the lists (i.e. kunit_case! into the statics). What do you think?

Yeah: I think that'd be a great idea. I do think there will be a need for a more direct set of bindings to various KUnit APIs if we want to be able to use some of the more advanced KUnit features like resources (which need access to the struct kunit pointer). The more direct the bindings are, the easier it is to share documentation between the C and Rust sides, too. But equally, function attributes could be a great improvement over some of the C macros for other things like static stubbing.

To what extent it makes sense to have a two or three layered set of bindings (unsafe, safe-but-relatively-"direct", more-idiomatic) is something we'll no doubt work out as we go. (And it's probably off-topic to go into it further on this PR, so I'll leave it for now. :-) )

Cheers,
-- David

[JoseExposito]

Disclaimer: I have no idea what the UML incompatibility looks like atm (is it a compile time error? strange runtime errors?).
But if it's a confusing error, we can have kunit.py enforce we don't run against UML.

That's a very good idea, the error even suggest to use --arch=x86_64 so it is self documented:

$ ./tools/testing/kunit/kunit.py run --kunitconfig=rust --make_options LLVM=1 
[12:42:55] Configuring KUnit Kernel ...
Regenerating .config ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig LLVM=1
ERROR:root:Not all Kconfig options selected in kunitconfig were in the generated .config.
This is probably due to unsatisfied dependencies.
Missing: # CONFIG_UML is not set
Note: many Kconfig options aren't available on UML. You can try running on a different architecture with something like "--arch=x86_64".

Much better than a Error 133 I spent a while debugging. Added to v3 👍

[dirkbehme]

I think this needs an update/rebase regarding the recently merged https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/Documentation/rust/general-information.rst?id=be412baf72402bd732e250033e03ad159d060a30

I'd propose to send this to the RFL mailing list (instead of doing a PR here) as it seems the chance of handling (merging) changes is higher, there.

I found the commit message of https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit?id=a66d733da8010c732979041cd602cfceab7f587b a quite nice description. Maybe some parts, at least the first introduction part, could somehow picked?

[ojeda]

as it seems the chance of handling (merging) changes is higher, there.

All changes going to upstream Linux go through the mailing list. PRs here were used before we were upstream, and nowadays are used for e.g. early reviews or to share code someone is working on.

Maybe some parts, at least the first introduction part, could somehow picked?

Yeah, that would be nice. I should have probably done it directly in that commit :)

[dirkbehme]

Yeah, that would be nice. I should have probably done it directly in that commit :)

A proposal for this: https://lore.kernel.org/rust-for-linux/[email protected]/T/#u

I tried to express your authorship with the Co-developed-by :)