[PATCH RFC 0/3] rust: kunit: #[should_panic] and same test name with different #[cfg(...)] support

["Nicolás Antinori" <nico.antinori.7@gmail.com>]

This patch series intends to implement two features for KUnit tests
written in Rust. The work is based on a TODO comment made in the
`bitmap.rs` module [1].

1. Supporting `#[should_panic]` [2]:

KUnit tests in Rust follow the user-space syntax, but at the moment
`#[should_panic]` is not supported. The first patch of this series adds
support for the attribute (only in its basic form, `#[should_panic =
"message"]` is not supported, and I don't know if it makes sense to
support it)

The way it is supported is by having a separate `#[panic_handler]` when
`CONFIG_KUNIT` is enabled. When a test is marked with `#[should_panic]`,
a static value (KUNIT_SHOULD_PANIC = 0xDEAD7357) is assigned to the
kunit's `priv` field, since it is meant for saving arbitrary user data
[3]. At the moment, I did not find any place where Rust tests use that
field, so it should be safe to write it.

When the test panics, the `#[panic_handler]` function is called, obtains
the kunit current test and checks if the `priv` field is not null and
contains the value `KUNIT_SHOULD_PANIC`.

If those conditions are true, it marks the test as successful (since it
panicked as expected) and calls `__kunit_abort_expecting_error`, a new
function that exits the testing thread but fills `try_catch->try_result`
with a 0 so the test runner does not mistake it as a failed test.

If those conditions are not true:
 - If `priv` is null, the test panics as it would have before having
   this feature, priv = null means that the test was not expected to
   panic.
 - If `priv` is not null but its value is not `KUNIT_SHOULD_PANIC`, the
   test panics with an error message informing that the code found in
   `priv` was invalid.

If the test does not panic, the `#[panic_handler]` is not triggered. The
test is marked as failed (since it was expected to panic).

Regarding this feature:
 - Do this approach make sense?
 - Is it ok to mark the `#[should_panic]` tests with a static constant?
   Is another mechanism better to check in the `#[panic_handler]` that
   the test was supposed to panic?

2. Allow same test name with different #[cfg(...)]:

When testing `#[should_panic]` in `bitmap.rs` (check the last patch of
the series) I found that the test that was supposed to panic had the
same name as another one, but they were run on different configurations.
This caused the following compilation error:

ERROR:root:error[E0428]: the name `kunit_rust_wrapper_owned_bitmap_out_of_bounds` is defined multiple times
   --> ../rust/kernel/bitmap.rs:503:1
    |
503 | #[macros::kunit_tests(rust_kernel_bitmap)]
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `kunit_rust_wrapper_owned_bitmap_out_of_bounds` redefined here
    |
    = note: `kunit_rust_wrapper_owned_bitmap_out_of_bounds` must be defined only once in the value namespace of this module
    = note: this error originates in the attribute macro `macros::kunit_tests` (in Nightly builds, run with -Z macro-backtrace for more info)


To fix this problem, I appended to the `kunit_rust_wrapper_*`
identifiers an 'stringified' version of the test's #[cfg(...)]
arguments. The purpose of this is that, if we have a test with the same
name and configuration, it would fail.

The configuration string was also appended to the tests names. This was
done to have a better test run report:

...
[SKIPPED] owned_bitmap_out_of_bounds_cfg_not_config_rust_bitmap_hardened
[PASSED] owned_bitmap_out_of_bounds_cfg_config_rust_bitmap_hardened
...

Otherwise we would have something like the following:
...
[SKIPPED] owned_bitmap_out_of_bounds
[PASSED] owned_bitmap_out_of_bounds
...

Regarding this:
- Does it makes sense to allow same test names with different cfgs?
- Is it ok to 'stringify' the configuration so it can be distinguished
  in the report? Would you prefer something like `_case_1` `_case_2` ..
  instead?

This is the first RFC patch I send to the LKML, if there's something not
right with it please let me know.

Kind Regards,
Nicolás

[1] https://github.com/Rust-for-Linux/linux/blob/fd73f4a6659897191fa0d40695fe370925dd3780/rust/kernel/bitmap.rs#L592-L600
[2] https://doc.rust-lang.org/rust-by-example/testing/unit_testing.html#testing-panics
[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/kunit/test.h?id=f6e7b42bf05b2427fb8a7a1d1c387a86638bb413#n314

Nicolás Antinori (3):
  rust: kunit: add #[should_panic] support
  rust: kunit: allow same test name with different #[cfg(...)]
  rust: bitmap: kunit: uncomment owned_bitmap_out_of_bounds panic case

 include/kunit/test.h      |  1 +
 include/kunit/try-catch.h |  1 +
 lib/kunit/test.c          | 14 +++++++
 lib/kunit/try-catch.c     |  7 ++++
 rust/kernel/bitmap.rs     | 40 +++++++++-----------
 rust/kernel/kunit.rs      | 24 ++++++++++++
 rust/kernel/lib.rs        | 46 +++++++++++++++++++++--
 rust/macros/kunit.rs      | 78 ++++++++++++++++++++++++++++++++++++---
 8 files changed, 181 insertions(+), 30 deletions(-)

--
2.47.3