On Tue, 18 Jul 2023 at 13:28, Miguel Ojeda <ojeda@xxxxxxxxxx> wrote: > > v1: https://lore.kernel.org/rust-for-linux/20230614180837.630180-1-ojeda@xxxxxxxxxx/ > v2: > > - Rebased on top of v6.5-rc1, which requires a change from > `kunit_do_failed_assertion` to `__kunit_do_failed_assertion` (since > the former became a macro) and the addition of a call to > `__kunit_abort` (since previously the call was done by the old > function which we cannot use anymore since it is a macro). (David) > > - Added prerequisite patch to KUnit header to include `stddef.h` to > support the `KUNIT=y` case. (Reported by Boqun) > > - Added comment on the purpose of `trait FromErrno`. (Martin asked > about it) > > - Simplify code to use `std::fs::write` instead of `write!`, which > improves code size too. (Suggested by Alice) > > - Fix copy-paste type in docs from "error" to "info" and, to make it > proper English, copy the `printk` docs style, i.e. from "info" > to "info-level message" -- and same for the "error" one. (Miguel) > > - Swap `FILE` and `LINE` `static`s to keep the same order as done > elsewhere. (Miguel) > > - Rename config option from `RUST_KERNEL_KUNIT_TEST` to > `RUST_KERNEL_DOCTESTS` (and update its title), so that we can use > the former for the "normal" (i.e. non-doctests, e.g. `#[test]` ones) > tests in the future. (David) > > - Follow the syntax proposed for declaring test metadata in the KTAP > v2 spec, which may also get used for the KUnit test attributes API. > > Thus, instead of "# Doctest from line {line}", use > "# {test_name}.location: {file}.{line}", which ideally will allow to > migrate to a KUnit attribute later. > > This is done in all cases, i.e. when the tests succeeds, because > it may be useful for users running KUnit manually, or when passing > `--raw_output` to `kunit.py`. (David) > > David: I used "location" instead of your suggested "line" alone, in > order to have both in a single line, which looked nice and closer to > the "ASSERTION FAILURE" case/line, since now we do have the original > file (please see below). I like "location" better, personally. The attributes work is still ongoing, and while there's some benefit to having "file" and "line" separate (it could potentially simplify some implementation on the C side), we'll cross that bridge when we come to it. > > - Figure out the original line. This is done by deploying an anchor > so that the difference in lines between the beginning of the test > and the assert, in the generated file, can be computed. Then, we > offset the line number of the original test, which is given by > `rustdoc`. (developed by Boqun) > > - Figure out the original file. This is done by walking the > filesystem, checking directory prefixes to reduce the amount of > combinations to check, and it is only done once per file (rather > than per test). > > Ambiguities are detected and reported. It does limit the filenames > (module names) we can use, but it is unlikely we will hit it soon > and this should be temporary anyway until `rustdoc` provides us > with the real path. (Miguel) > > Tested with both in-tree and `O=` builds, but I would appreciate > extra testing on this one, including via the `kunit.py` script. > This seems to be working well on the existing cases under kunit.py here. I'll continue to play with it, but no worries on my end thus far. > - The three last items combined means that we now see this output even > for successful cases: > > # rust_doctest_kernel_sync_locked_by_rs_0.location: rust/kernel/sync/locked_by.rs:28 > ok 53 rust_doctest_kernel_sync_locked_by_rs_0 > > Which basically gives the user all the information they need to go > back to the source code of the doctest, while keeping them fairly > stable for bisection, and while avoiding to require users to write > test names manually. (David + Boqun + Miguel) > > David: from what I saw in v2 of the RFC for the test attributes API, > the syntax still contains the test name when it is not a suite, so > I followed that, but if you prefer to omit it, please feel free to > do so (for me either way it is fine, and if this is the expected > attribute syntax, I guess it is worth to follow it to make migration > easier later on): > > # location: rust/kernel/sync/locked_by.rs:28 > ok 53 rust_doctest_kernel_sync_locked_by_rs_0 Thanks: while we're still arguing a bit about exactly what the format of these will look like in the KUnit/KTAP attributes spec/patches, what you've used matches what we've been proposing so far. Let's stick with <test name>.location for now, and change it if needed when the attributes spec is finalised. > > - Collected `Reviewed-by`s and `Tested-by`s, except for the main > commit due to the substantial changes. > > Miguel Ojeda (7): > kunit: test-bug.h: include `stddef.h` for `NULL` > rust: init: make doctests compilable/testable > rust: str: make doctests compilable/testable > rust: sync: make doctests compilable/testable > rust: types: make doctests compilable/testable > rust: support running Rust documentation tests as KUnit ones > MAINTAINERS: add Rust KUnit files to the KUnit entry These are all (still) looking pretty good to me. If there are no objections, I'd like to take these into kselftest/kunit as-is and if we need to change anything (e.g. for consistency with attributes when they land), do that as a follow-up. Thanks again, Miguel, for all the work getting this going! Cheers, -- David > > MAINTAINERS | 2 + > include/kunit/test-bug.h | 2 + > lib/Kconfig.debug | 13 ++ > rust/.gitignore | 2 + > rust/Makefile | 29 ++++ > rust/bindings/bindings_helper.h | 1 + > rust/helpers.c | 7 + > rust/kernel/init.rs | 26 +-- > rust/kernel/kunit.rs | 163 +++++++++++++++++++ > rust/kernel/lib.rs | 2 + > rust/kernel/str.rs | 4 +- > rust/kernel/sync/arc.rs | 9 +- > rust/kernel/sync/lock/mutex.rs | 1 + > rust/kernel/sync/lock/spinlock.rs | 1 + > rust/kernel/types.rs | 6 +- > scripts/.gitignore | 2 + > scripts/Makefile | 4 + > scripts/rustdoc_test_builder.rs | 72 +++++++++ > scripts/rustdoc_test_gen.rs | 260 ++++++++++++++++++++++++++++++ > 19 files changed, 591 insertions(+), 15 deletions(-) > create mode 100644 rust/kernel/kunit.rs > create mode 100644 scripts/rustdoc_test_builder.rs > create mode 100644 scripts/rustdoc_test_gen.rs > > > base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5 > -- > 2.41.0 > > -- > You received this message because you are subscribed to the Google Groups "KUnit Development" group. > To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@xxxxxxxxxxxxxxxx. > To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20230718052752.1045248-1-ojeda%40kernel.org.
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
