[PATCH] rust: kunit: allow `cfg` on `test`s

[Kaibo Ma]

The `kunit_test` proc macro only checks for the `test` attribute
immediately preceding a `fn`. If the function is disabled via a `cfg`,
the generated code would result in a compile error referencing a
non-existent function [1].

This collects attributes and specifically cherry-picks `cfg` attributes
to be duplicated inside KUnit wrapper functions such that a test function
disabled via `cfg` compiles and is ignored correctly.

Link: https://lore.kernel.org/rust-for-linux/CANiq72==48=69hYiDo1321pCzgn_n1_jg=ez5UYXX9...@mail.gmail.com/ [1]
Closes: https://github.com/Rust-for-Linux/linux/issues/1185
Suggested-by: Miguel Ojeda <oj...@kernel.org>
Signed-off-by: Kaibo Ma <ent3...@gmail.com>
---
rust/kernel/kunit.rs | 7 +++++++
rust/macros/kunit.rs | 46 ++++++++++++++++++++++++++++++++------------
2 files changed, 41 insertions(+), 12 deletions(-)

diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index 41efd8759..32640dfc9 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -357,4 +357,11 @@ fn rust_test_kunit_example_test() {
fn rust_test_kunit_in_kunit_test() {
assert!(in_kunit_test());
}
+
+ #[test]
+ #[cfg(not(all()))]
+ fn rust_test_kunit_always_disabled_test() {
+ // This test should never run because of the `cfg`.
+ assert!(false);
+ }
}
diff --git a/rust/macros/kunit.rs b/rust/macros/kunit.rs
index 81d18149a..850a321e5 100644
--- a/rust/macros/kunit.rs
+++ b/rust/macros/kunit.rs
@@ -5,6 +5,7 @@
//! Copyright (c) 2023 José Expósito <jose.ex...@gmail.com>

use proc_macro::{Delimiter, Group, TokenStream, TokenTree};
+use std::collections::HashMap;
use std::fmt::Write;

pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
@@ -41,20 +42,32 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
// Get the functions set as tests. Search for `[test]` -> `fn`.
let mut body_it = body.stream().into_iter();
let mut tests = Vec::new();
+ let mut attributes: HashMap<String, TokenStream> = HashMap::new();
while let Some(token) = body_it.next() {
match token {
- TokenTree::Group(ident) if ident.to_string() == "[test]" => match body_it.next() {
- Some(TokenTree::Ident(ident)) if ident.to_string() == "fn" => {
- let test_name = match body_it.next() {
- Some(TokenTree::Ident(ident)) => ident.to_string(),
- _ => continue,
- };
- tests.push(test_name);
+ TokenTree::Punct(ref p) if p.as_char() == '#' => match body_it.next() {
+ Some(TokenTree::Group(g)) if g.delimiter() == Delimiter::Bracket => {
+ if let Some(TokenTree::Ident(name)) = g.stream().into_iter().next() {
+ // Collect attributes because we need to find which are tests. We also
+ // need to copy `cfg` attributes so tests can be conditionally enabled.
+ attributes
+ .entry(name.to_string())
+ .or_default()
+ .extend([token, TokenTree::Group(g)]);
+ }
+ continue;
}
- _ => continue,
+ _ => (),
},
+ TokenTree::Ident(i) if i.to_string() == "fn" && attributes.contains_key("test") => {
+ if let Some(TokenTree::Ident(test_name)) = body_it.next() {
+ tests.push((test_name, attributes.remove("cfg").unwrap_or_default()))
+ }
+ }
+
_ => (),
}
+ attributes.clear();
}

// Add `#[cfg(CONFIG_KUNIT="y")]` before the module declaration.
@@ -100,11 +113,20 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
let mut test_cases = "".to_owned();
let mut assert_macros = "".to_owned();
let path = crate::helpers::file();
- for test in &tests {
+ let num_tests = tests.len();
+ for (test, cfg_attr) in tests {
let kunit_wrapper_fn_name = format!("kunit_rust_wrapper_{test}");
- // An extra `use` is used here to reduce the length of the message.
+ // Append any `cfg` attributes the user might have written on their tests so we don't
+ // attempt to call them when they are `cfg`'d out. An extra `use` is used here to reduce
+ // the length of the assert message.
let kunit_wrapper = format!(
- "unsafe extern \"C\" fn {kunit_wrapper_fn_name}(_test: *mut ::kernel::bindings::kunit) {{ use ::kernel::kunit::is_test_result_ok; assert!(is_test_result_ok({test}())); }}",
+ r#"unsafe extern "C" fn {kunit_wrapper_fn_name}(_test: *mut ::kernel::bindings::kunit)
+ {{
+ {cfg_attr} {{
+ use ::kernel::kunit::is_test_result_ok;
+ assert!(is_test_result_ok({test}()));
+ }}
+ }}"#,
);
writeln!(kunit_macros, "{kunit_wrapper}").unwrap();
writeln!(
@@ -139,7 +161,7 @@ macro_rules! assert_eq {{
writeln!(
kunit_macros,
"static mut TEST_CASES: [::kernel::bindings::kunit_case; {}] = [\n{test_cases} ::kernel::kunit::kunit_case_null(),\n];",
- tests.len() + 1
+ num_tests + 1
)
.unwrap();

--
2.50.1

[David Gow]

On Mon, 8 Sept 2025 at 04:17, Kaibo Ma <ent3...@gmail.com> wrote:
>
> The `kunit_test` proc macro only checks for the `test` attribute
> immediately preceding a `fn`. If the function is disabled via a `cfg`,
> the generated code would result in a compile error referencing a
> non-existent function [1].
>
> This collects attributes and specifically cherry-picks `cfg` attributes
> to be duplicated inside KUnit wrapper functions such that a test function
> disabled via `cfg` compiles and is ignored correctly.
>
> Link: https://lore.kernel.org/rust-for-linux/CANiq72==48=69hYiDo1321pCzgn_n1_jg=ez5UYXX9...@mail.gmail.com/ [1]
> Closes: https://github.com/Rust-for-Linux/linux/issues/1185
> Suggested-by: Miguel Ojeda <oj...@kernel.org>
> Signed-off-by: Kaibo Ma <ent3...@gmail.com>
> ---

Thanks very much: I think this is a great improvement over not having
'cfg' work at all.

I do think, though, that we need to handle disabled tests differently,
as it causes two issues:
1. Currently, disabled tests are still included in the suite which
contains them, and always pass. It'd be nice if they either were
missing from the suite, or were marked 'skipped' by default.
2. It's not possible to have several implementations of the same test
(or, depending on how you look at it, several tests with the same
name) with different #[cfg] attributes, even if all but one are
disabled.

My ideal solution to both of these issues would be to only include
tests (i.e., the wrapper function and the kunit_case struct) if
they're enabled. That's possibly more difficult than it looks, though:
my initial attempt at implementing this ran aground trying to
calculate num_tests.

Even if that's not feasible, we should at least make disabled tests be
marked 'skipped'. A quick way of doing this would be to mark the test
as skipped in the wrapper function:
(*_test).status = ::kernel::bindings::kunit_status_KUNIT_SKIPPED;
And then re-mark it as success (KUnit tests expect the initial state
to be success) within the {cfg_attr} block:
(*_test).status = ::kernel::bindings::kunit_status_KUNIT_SUCCESS;

That doesn't solve issue #2, but I suspect that's rare enough that we
can leave it until someone actually has a good reason to have two test
implementations.

Otherwise, this seems fine to me.

Thanks,
-- David

[Kaibo Ma]

The `kunit_test` proc macro only checks for the `test` attribute
immediately preceding a `fn`. If the function is disabled via a `cfg`,
the generated code would result in a compile error referencing a
non-existent function [1].

This collects attributes and specifically cherry-picks `cfg` attributes
to be duplicated inside KUnit wrapper functions such that a test function

disabled via `cfg` compiles and is marked as skipped in KUnit correctly.

Link: https://lore.kernel.org/rust-for-linux/CANiq72==48=69hYiDo1321pCzgn_n1_jg=ez5UYXX9...@mail.gmail.com/ [1]
Closes: https://github.com/Rust-for-Linux/linux/issues/1185
Suggested-by: Miguel Ojeda <oj...@kernel.org>

Suggested-by: David Gow <davi...@google.com>

Signed-off-by: Kaibo Ma <ent3...@gmail.com>
---

v1 -> v2: applied suggestion such that cfg'd out tests show as skipped on KUnit

rust/kernel/kunit.rs | 7 +++++++
rust/macros/kunit.rs | 48 +++++++++++++++++++++++++++++++++-----------
2 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index 41efd8759..32640dfc9 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -357,4 +357,11 @@ fn rust_test_kunit_example_test() {
fn rust_test_kunit_in_kunit_test() {
assert!(in_kunit_test());
}
+
+ #[test]
+ #[cfg(not(all()))]
+ fn rust_test_kunit_always_disabled_test() {
+ // This test should never run because of the `cfg`.
+ assert!(false);
+ }
}
diff --git a/rust/macros/kunit.rs b/rust/macros/kunit.rs

index 81d18149a..b395bb053 100644

@@ -100,11 +113,22 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {

let mut test_cases = "".to_owned();
let mut assert_macros = "".to_owned();
let path = crate::helpers::file();
- for test in &tests {
+ let num_tests = tests.len();
+ for (test, cfg_attr) in tests {
let kunit_wrapper_fn_name = format!("kunit_rust_wrapper_{test}");
- // An extra `use` is used here to reduce the length of the message.
+ // Append any `cfg` attributes the user might have written on their tests so we don't
+ // attempt to call them when they are `cfg`'d out. An extra `use` is used here to reduce
+ // the length of the assert message.
let kunit_wrapper = format!(
- "unsafe extern \"C\" fn {kunit_wrapper_fn_name}(_test: *mut ::kernel::bindings::kunit) {{ use ::kernel::kunit::is_test_result_ok; assert!(is_test_result_ok({test}())); }}",
+ r#"unsafe extern "C" fn {kunit_wrapper_fn_name}(_test: *mut ::kernel::bindings::kunit)
+ {{

+ (*_test).status = ::kernel::bindings::kunit_status_KUNIT_SKIPPED;
+ {cfg_attr} {{
+ (*_test).status = ::kernel::bindings::kunit_status_KUNIT_SUCCESS;

+ use ::kernel::kunit::is_test_result_ok;
+ assert!(is_test_result_ok({test}()));
+ }}
+ }}"#,
);
writeln!(kunit_macros, "{kunit_wrapper}").unwrap();
writeln!(

@@ -139,7 +163,7 @@ macro_rules! assert_eq {{

[David Gow]

On Tue, 16 Sept 2025 at 10:13, Kaibo Ma <ent3...@gmail.com> wrote:
>
> The `kunit_test` proc macro only checks for the `test` attribute
> immediately preceding a `fn`. If the function is disabled via a `cfg`,
> the generated code would result in a compile error referencing a
> non-existent function [1].
>
> This collects attributes and specifically cherry-picks `cfg` attributes
> to be duplicated inside KUnit wrapper functions such that a test function
> disabled via `cfg` compiles and is marked as skipped in KUnit correctly.
>
> Link: https://lore.kernel.org/rust-for-linux/CANiq72==48=69hYiDo1321pCzgn_n1_jg=ez5UYXX9...@mail.gmail.com/ [1]
> Closes: https://github.com/Rust-for-Linux/linux/issues/1185
> Suggested-by: Miguel Ojeda <oj...@kernel.org>
> Suggested-by: David Gow <davi...@google.com>
> Signed-off-by: Kaibo Ma <ent3...@gmail.com>
> ---
> v1 -> v2: applied suggestion such that cfg'd out tests show as skipped on KUnit
>

Thanks: this looks good to me now. I'd still love for there to be a
way to totally remove tests which are disabled, but I suspect it's
best to avoid adding the complexity involved in handling that until we
actually need it. (Maybe never!)

So this is:
Reviewed-by: David Gow <davi...@google.com>

Thanks,
-- David