All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] rust: kunit: Support KUnit tests with a user-space like syntax
@ 2023-07-20  6:38 David Gow
  2023-07-20  6:38 ` [PATCH 1/3] rust: kunit: add KUnit case and suite macros David Gow
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: David Gow @ 2023-07-20  6:38 UTC (permalink / raw)
  To: Brendan Higgins, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
	Boqun Feng, Gary Guo, Benno Lossin
  Cc: David Gow, José Expósito, Björn Roy Baron,
	linux-kselftest, kunit-dev, rust-for-linux, linux-kernel

This series was originally written by José Expósito, and can be found
here:
https://github.com/Rust-for-Linux/linux/pull/950

Add support for writing KUnit tests in Rust. While Rust doctests are
already converted to KUnit tests and run, they're really better suited
for examples, rather than as first-class unit tests.

This series implements a series of direct Rust bindings for KUnit tests,
as well as a new macro which allows KUnit tests to be written using a
close variant of normal Rust unit test syntax. The only change required
is replacing '#[cfg(test)]' with '#[kunit_tests(kunit_test_suite_name)]'

An example test would look like:
	#[kunit_tests(rust_kernel_hid_driver)]
	mod tests {
	    use super::*;
	    use crate::{c_str, driver, hid, prelude::*};
	    use core::ptr;

	    struct SimpleTestDriver;
	    impl Driver for SimpleTestDriver {
	        type Data = ();
	    }

	    #[test]
	    fn rust_test_hid_driver_adapter() {
	        let mut hid = bindings::hid_driver::default();
	        let name = c_str!("SimpleTestDriver");
	        static MODULE: ThisModule = unsafe { ThisModule::from_ptr(ptr::null_mut()) };

        	let res = unsafe {
	            <hid::Adapter<SimpleTestDriver> as driver::DriverOps>::register(&mut hid, name, &MODULE)
	        };
	        assert_eq!(res, Err(ENODEV)); // The mock returns -19
	    }
	}

Changes since the GitHub PR:
- Rebased on top of kselftest/kunit
- Add const_mut_refs feature
  This may conflict with https://lore.kernel.org/lkml/20230503090708.2524310-6-nmi@metaspace.dk/
- Add rust/macros/kunit.rs to the KUnit MAINTAINERS entry

Signed-off-by: David Gow <davidgow@google.com>
---
José Expósito (3):
      rust: kunit: add KUnit case and suite macros
      rust: macros: add macro to easily run KUnit tests
      rust: kunit: allow to know if we are in a test

 MAINTAINERS          |   1 +
 rust/kernel/kunit.rs | 181 +++++++++++++++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs   |   1 +
 rust/macros/kunit.rs | 149 ++++++++++++++++++++++++++++++++++++++++++
 rust/macros/lib.rs   |  29 +++++++++
 5 files changed, 361 insertions(+)
---
base-commit: 64bd4641310c41a1ecf07c13c67bc0ed61045dfd
change-id: 20230720-rustbind-477964954da5

Best regards,
-- 
David Gow <davidgow@google.com>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/3] rust: kunit: add KUnit case and suite macros
  2023-07-20  6:38 [PATCH 0/3] rust: kunit: Support KUnit tests with a user-space like syntax David Gow
@ 2023-07-20  6:38 ` David Gow
  2023-07-25 18:07   ` Boqun Feng
  2023-07-20  6:38 ` [PATCH 2/3] rust: macros: add macro to easily run KUnit tests David Gow
  2023-07-20  6:38 ` [PATCH 3/3] rust: kunit: allow to know if we are in a test David Gow
  2 siblings, 1 reply; 8+ messages in thread
From: David Gow @ 2023-07-20  6:38 UTC (permalink / raw)
  To: Brendan Higgins, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
	Boqun Feng, Gary Guo, Benno Lossin
  Cc: David Gow, José Expósito, Björn Roy Baron,
	linux-kselftest, kunit-dev, rust-for-linux, linux-kernel

From: José Expósito <jose.exposito89@gmail.com>

Add a couple of Rust macros to allow to develop KUnit tests without
relying on generated C code:

 - The `kunit_unsafe_test_suite!` Rust macro is similar to the
   `kunit_test_suite` C macro.
 - The `kunit_case!` Rust macro is similar to the `KUNIT_CASE` C macro.
   It can be used with parameters to generate a test case or without
   parameters to be used as delimiter in `kunit_test_suite!`.

While these macros can be used on their own, a future patch will
introduce another macro to create KUnit tests using a user-space like
syntax.

Co-developed-by: David Gow <davidgow@google.com>
Signed-off-by: David Gow <davidgow@google.com>
Signed-off-by: José Expósito <jose.exposito89@gmail.com>
---
 rust/kernel/kunit.rs | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs   |  1 +
 2 files changed, 93 insertions(+)

diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index 722655b2d62d..4cffc71e463b 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -161,3 +161,95 @@ macro_rules! kunit_assert_eq {
         $crate::kunit_assert!($name, $file, $diff, $left == $right);
     }};
 }
+
+/// Represents an individual test case.
+///
+/// The test case should have the signature
+/// `unsafe extern "C" fn test_case(test: *mut crate::bindings::kunit)`.
+///
+/// The `kunit_unsafe_test_suite!` macro expects a NULL-terminated list of test cases. This macro
+/// can be invoked without parameters to generate the delimiter.
+#[macro_export]
+macro_rules! kunit_case {
+    () => {
+        $crate::bindings::kunit_case {
+            run_case: None,
+            name: core::ptr::null_mut(),
+            generate_params: None,
+            status: $crate::bindings::kunit_status_KUNIT_SUCCESS,
+            log: core::ptr::null_mut(),
+        }
+    };
+    ($name:ident, $run_case:ident) => {
+        $crate::bindings::kunit_case {
+            run_case: Some($run_case),
+            name: $crate::c_str!(core::stringify!($name)).as_char_ptr(),
+            generate_params: None,
+            status: $crate::bindings::kunit_status_KUNIT_SUCCESS,
+            log: core::ptr::null_mut(),
+        }
+    };
+}
+
+/// Registers a KUnit test suite.
+///
+/// # Safety
+///
+/// `test_cases` must be a NULL terminated array of test cases.
+///
+/// # Examples
+///
+/// ```ignore
+/// unsafe extern "C" fn test_fn(_test: *mut crate::bindings::kunit) {
+///     let actual = 1 + 1;
+///     let expected = 2;
+///     assert_eq!(actual, expected);
+/// }
+///
+/// static mut KUNIT_TEST_CASE: crate::bindings::kunit_case = crate::kunit_case!(name, test_fn);
+/// static mut KUNIT_NULL_CASE: crate::bindings::kunit_case = crate::kunit_case!();
+/// static mut KUNIT_TEST_CASES: &mut[crate::bindings::kunit_case] = unsafe {
+///     &mut[KUNIT_TEST_CASE, KUNIT_NULL_CASE]
+/// };
+/// crate::kunit_unsafe_test_suite!(suite_name, KUNIT_TEST_CASES);
+/// ```
+#[macro_export]
+macro_rules! kunit_unsafe_test_suite {
+    ($name:ident, $test_cases:ident) => {
+        const _: () = {
+            static KUNIT_TEST_SUITE_NAME: [i8; 256] = {
+                let name_u8 = core::stringify!($name).as_bytes();
+                let mut ret = [0; 256];
+
+                let mut i = 0;
+                while i < name_u8.len() {
+                    ret[i] = name_u8[i] as i8;
+                    i += 1;
+                }
+
+                ret
+            };
+
+            // SAFETY: `test_cases` is valid as it should be static.
+            static mut KUNIT_TEST_SUITE: core::cell::UnsafeCell<$crate::bindings::kunit_suite> =
+                core::cell::UnsafeCell::new($crate::bindings::kunit_suite {
+                    name: KUNIT_TEST_SUITE_NAME,
+                    test_cases: unsafe { $test_cases.as_mut_ptr() },
+                    suite_init: None,
+                    suite_exit: None,
+                    init: None,
+                    exit: None,
+                    status_comment: [0; 256usize],
+                    debugfs: core::ptr::null_mut(),
+                    log: core::ptr::null_mut(),
+                    suite_init_err: 0,
+                });
+
+            // SAFETY: `KUNIT_TEST_SUITE` is static.
+            #[used]
+            #[link_section = ".kunit_test_suites"]
+            static mut KUNIT_TEST_SUITE_ENTRY: *const $crate::bindings::kunit_suite =
+                unsafe { KUNIT_TEST_SUITE.get() };
+        };
+    };
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 3642cadc34b1..ec81fd28d71a 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -18,6 +18,7 @@
 #![feature(new_uninit)]
 #![feature(receiver_trait)]
 #![feature(unsize)]
+#![feature(const_mut_refs)]
 
 // Ensure conditional compilation based on the kernel configuration works;
 // otherwise we may silently break things like initcall handling.

-- 
2.41.0.255.g8b1d071c50-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] rust: macros: add macro to easily run KUnit tests
  2023-07-20  6:38 [PATCH 0/3] rust: kunit: Support KUnit tests with a user-space like syntax David Gow
  2023-07-20  6:38 ` [PATCH 1/3] rust: kunit: add KUnit case and suite macros David Gow
@ 2023-07-20  6:38 ` David Gow
  2023-07-30 21:49   ` Boqun Feng
  2023-08-01 14:44   ` Miguel Ojeda
  2023-07-20  6:38 ` [PATCH 3/3] rust: kunit: allow to know if we are in a test David Gow
  2 siblings, 2 replies; 8+ messages in thread
From: David Gow @ 2023-07-20  6:38 UTC (permalink / raw)
  To: Brendan Higgins, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
	Boqun Feng, Gary Guo, Benno Lossin
  Cc: David Gow, José Expósito, Björn Roy Baron,
	linux-kselftest, kunit-dev, rust-for-linux, linux-kernel

From: José Expósito <jose.exposito89@gmail.com>

Add a new procedural macro (`#[kunit_tests(kunit_test_suit_name)]`) to
run KUnit tests using a user-space like syntax.

The macro, that should be used on modules, transforms every `#[test]`
in a `kunit_case!` and adds a `kunit_unsafe_test_suite!` registering
all of them.

The only difference with user-space tests is that instead of using
`#[cfg(test)]`, `#[kunit_tests(kunit_test_suit_name)]` is used.

Note that `#[cfg(CONFIG_KUNIT)]` is added so the test module is not
compiled when `CONFIG_KUNIT` is set to `n`.

Reviewed-by: David Gow <davidgow@google.com>
Signed-off-by: José Expósito <jose.exposito89@gmail.com>
---
 MAINTAINERS          |   1 +
 rust/kernel/kunit.rs |  11 ++++
 rust/macros/kunit.rs | 149 +++++++++++++++++++++++++++++++++++++++++++++++++++
 rust/macros/lib.rs   |  29 ++++++++++
 4 files changed, 190 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2a942fe59144..c32ba6b29a96 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11373,6 +11373,7 @@ F:	Documentation/dev-tools/kunit/
 F:	include/kunit/
 F:	lib/kunit/
 F:	rust/kernel/kunit.rs
+F:	rust/macros/kunit.rs
 F:	scripts/rustdoc_test_*
 F:	tools/testing/kunit/
 
diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index 4cffc71e463b..44ea67028316 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -40,6 +40,8 @@ pub fn info(args: fmt::Arguments<'_>) {
     }
 }
 
+use macros::kunit_tests;
+
 /// Asserts that a boolean expression is `true` at runtime.
 ///
 /// Public but hidden since it should only be used from generated tests.
@@ -253,3 +255,12 @@ macro_rules! kunit_unsafe_test_suite {
         };
     };
 }
+
+#[kunit_tests(rust_kernel_kunit)]
+mod tests {
+    #[test]
+    fn rust_test_kunit_kunit_tests() {
+        let running = true;
+        assert_eq!(running, true);
+    }
+}
diff --git a/rust/macros/kunit.rs b/rust/macros/kunit.rs
new file mode 100644
index 000000000000..69dac253232f
--- /dev/null
+++ b/rust/macros/kunit.rs
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Procedural macro to run KUnit tests using a user-space like syntax.
+//!
+//! Copyright (c) 2023 José Expósito <jose.exposito89@gmail.com>
+
+use proc_macro::{Delimiter, Group, TokenStream, TokenTree};
+use std::fmt::Write;
+
+pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
+    if attr.to_string().is_empty() {
+        panic!("Missing test name in #[kunit_tests(test_name)] macro")
+    }
+
+    let mut tokens: Vec<_> = ts.into_iter().collect();
+
+    // Scan for the "mod" keyword.
+    tokens
+        .iter()
+        .find_map(|token| match token {
+            TokenTree::Ident(ident) => match ident.to_string().as_str() {
+                "mod" => Some(true),
+                _ => None,
+            },
+            _ => None,
+        })
+        .expect("#[kunit_tests(test_name)] attribute should only be applied to modules");
+
+    // Retrieve the main body. The main body should be the last token tree.
+    let body = match tokens.pop() {
+        Some(TokenTree::Group(group)) if group.delimiter() == Delimiter::Brace => group,
+        _ => panic!("cannot locate main body of module"),
+    };
+
+    // Get the functions set as tests. Search for `[test]` -> `fn`.
+    let mut body_it = body.stream().into_iter();
+    let mut tests = Vec::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);
+                }
+                _ => continue,
+            },
+            _ => (),
+        }
+    }
+
+    // Add `#[cfg(CONFIG_KUNIT)]` before the module declaration.
+    let config_kunit = "#[cfg(CONFIG_KUNIT)]".to_owned().parse().unwrap();
+    tokens.insert(
+        0,
+        TokenTree::Group(Group::new(Delimiter::None, config_kunit)),
+    );
+
+    // Generate the test KUnit test suite and a test case for each `#[test]`.
+    // The code generated for the following test module:
+    //
+    // ```
+    // #[kunit_tests(kunit_test_suit_name)]
+    // mod tests {
+    //     #[test]
+    //     fn foo() {
+    //         assert_eq!(1, 1);
+    //     }
+    //
+    //     #[test]
+    //     fn bar() {
+    //         assert_eq!(2, 2);
+    //     }
+    // ```
+    //
+    // Looks like:
+    //
+    // ```
+    // unsafe extern "C" fn kunit_rust_wrapper_foo(_test: *mut kernel::bindings::kunit) {
+    //     foo();
+    // }
+    // static mut KUNIT_CASE_FOO: kernel::bindings::kunit_case =
+    //     kernel::kunit_case!(foo, kunit_rust_wrapper_foo);
+    //
+    // unsafe extern "C" fn kunit_rust_wrapper_bar(_test: * mut kernel::bindings::kunit) {
+    //     bar();
+    // }
+    // static mut KUNIT_CASE_BAR: kernel::bindings::kunit_case =
+    //     kernel::kunit_case!(bar, kunit_rust_wrapper_bar);
+    //
+    // static mut KUNIT_CASE_NULL: kernel::bindings::kunit_case = kernel::kunit_case!();
+    //
+    // static mut TEST_CASES : &mut[kernel::bindings::kunit_case] = unsafe {
+    //     &mut [KUNIT_CASE_FOO, KUNIT_CASE_BAR, KUNIT_CASE_NULL]
+    // };
+    //
+    // kernel::kunit_unsafe_test_suite!(kunit_test_suit_name, TEST_CASES);
+    // ```
+    let mut kunit_macros = "".to_owned();
+    let mut test_cases = "".to_owned();
+    for test in tests {
+        let kunit_wrapper_fn_name = format!("kunit_rust_wrapper_{}", test);
+        let kunit_case_name = format!("KUNIT_CASE_{}", test.to_uppercase());
+        let kunit_wrapper = format!(
+            "unsafe extern \"C\" fn {}(_test: *mut kernel::bindings::kunit) {{ {}(); }}",
+            kunit_wrapper_fn_name, test
+        );
+        let kunit_case = format!(
+            "static mut {}: kernel::bindings::kunit_case = kernel::kunit_case!({}, {});",
+            kunit_case_name, test, kunit_wrapper_fn_name
+        );
+        writeln!(kunit_macros, "{kunit_wrapper}").unwrap();
+        writeln!(kunit_macros, "{kunit_case}").unwrap();
+        writeln!(test_cases, "{kunit_case_name},").unwrap();
+    }
+
+    writeln!(
+        kunit_macros,
+        "static mut KUNIT_CASE_NULL: kernel::bindings::kunit_case = kernel::kunit_case!();"
+    )
+    .unwrap();
+
+    writeln!(
+        kunit_macros,
+        "static mut TEST_CASES : &mut[kernel::bindings::kunit_case] = unsafe {{ &mut[{test_cases} KUNIT_CASE_NULL] }};"
+    )
+    .unwrap();
+
+    writeln!(
+        kunit_macros,
+        "kernel::kunit_unsafe_test_suite!({attr}, TEST_CASES);"
+    )
+    .unwrap();
+
+    let new_body: TokenStream = vec![body.stream(), kunit_macros.parse().unwrap()]
+        .into_iter()
+        .collect();
+
+    // Remove the `#[test]` macros.
+    let new_body = new_body.to_string().replace("#[test]", "");
+    tokens.push(TokenTree::Group(Group::new(
+        Delimiter::Brace,
+        new_body.parse().unwrap(),
+    )));
+
+    tokens.into_iter().collect()
+}
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index 3fc74cb4ea19..cd0b720514ff 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -6,6 +6,7 @@
 mod quote;
 mod concat_idents;
 mod helpers;
+mod kunit;
 mod module;
 mod pin_data;
 mod pinned_drop;
@@ -246,3 +247,31 @@ pub fn pin_data(inner: TokenStream, item: TokenStream) -> TokenStream {
 pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
     pinned_drop::pinned_drop(args, input)
 }
+
+/// Registers a KUnit test suite and its test cases using a user-space like syntax.
+///
+/// This macro should be used on modules. If `CONFIG_KUNIT` (in `.config`) is `n`, the target module
+/// is ignored.
+///
+/// # Examples
+///
+/// ```ignore
+/// # use macros::kunit_tests;
+///
+/// #[kunit_tests(kunit_test_suit_name)]
+/// mod tests {
+///     #[test]
+///     fn foo() {
+///         assert_eq!(1, 1);
+///     }
+///
+///     #[test]
+///     fn bar() {
+///         assert_eq!(2, 2);
+///     }
+/// }
+/// ```
+#[proc_macro_attribute]
+pub fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
+    kunit::kunit_tests(attr, ts)
+}

-- 
2.41.0.255.g8b1d071c50-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] rust: kunit: allow to know if we are in a test
  2023-07-20  6:38 [PATCH 0/3] rust: kunit: Support KUnit tests with a user-space like syntax David Gow
  2023-07-20  6:38 ` [PATCH 1/3] rust: kunit: add KUnit case and suite macros David Gow
  2023-07-20  6:38 ` [PATCH 2/3] rust: macros: add macro to easily run KUnit tests David Gow
@ 2023-07-20  6:38 ` David Gow
  2023-07-25 23:34   ` Boqun Feng
  2 siblings, 1 reply; 8+ messages in thread
From: David Gow @ 2023-07-20  6:38 UTC (permalink / raw)
  To: Brendan Higgins, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
	Boqun Feng, Gary Guo, Benno Lossin
  Cc: David Gow, José Expósito, Björn Roy Baron,
	linux-kselftest, kunit-dev, rust-for-linux, linux-kernel

From: José Expósito <jose.exposito89@gmail.com>

In some cases, you need to call test-only code from outside the test
case, for example, to mock a function or a module.

In order to check whether we are in a test or not, we need to test if
`CONFIG_KUNIT` is set.
Unfortunately, we cannot rely only on this condition because some
distros compile KUnit in production kernels, so checking at runtime
that `current->kunit_test != NULL` is required.

Note that the C function `kunit_get_current_test()` can not be used
because it is not present in the current Rust tree yet. Once it is
available we might want to change our Rust wrapper to use it.

This patch adds a function to know whether we are in a KUnit test or
not and examples showing how to mock a function and a module.

Reviewed-by: David Gow <davidgow@google.com>
Signed-off-by: José Expósito <jose.exposito89@gmail.com>
---
 rust/kernel/kunit.rs | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index 44ea67028316..dcaac19bb108 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -40,6 +40,8 @@ pub fn info(args: fmt::Arguments<'_>) {
     }
 }
 
+use crate::task::Task;
+use core::ops::Deref;
 use macros::kunit_tests;
 
 /// Asserts that a boolean expression is `true` at runtime.
@@ -256,11 +258,87 @@ macro_rules! kunit_unsafe_test_suite {
     };
 }
 
+/// In some cases, you need to call test-only code from outside the test case, for example, to
+/// create a function mock. This function can be invoked to know whether we are currently running a
+/// KUnit test or not.
+///
+/// # Examples
+///
+/// This example shows how a function can be mocked to return a well-known value while testing:
+///
+/// ```
+/// # use kernel::kunit::in_kunit_test;
+/// #
+/// fn fn_mock_example(n: i32) -> i32 {
+///     if in_kunit_test() {
+///         100
+///     } else {
+///         n + 1
+///     }
+/// }
+///
+/// let mock_res = fn_mock_example(5);
+/// assert_eq!(mock_res, 100);
+/// ```
+///
+/// Sometimes, you don't control the code that needs to be mocked. This example shows how the
+/// `bindings` module can be mocked:
+///
+/// ```
+/// // Import our mock naming it as the real module.
+/// #[cfg(CONFIG_KUNIT)]
+/// use bindings_mock_example as bindings;
+///
+/// // This module mocks `bindings`.
+/// mod bindings_mock_example {
+///     use kernel::kunit::in_kunit_test;
+///     use kernel::bindings::u64_;
+///
+///     // Make the other binding functions available.
+///     pub(crate) use kernel::bindings::*;
+///
+///     // Mock `ktime_get_boot_fast_ns` to return a well-known value when running a KUnit test.
+///     pub(crate) unsafe fn ktime_get_boot_fast_ns() -> u64_ {
+///         if in_kunit_test() {
+///             1234
+///         } else {
+///             unsafe { kernel::bindings::ktime_get_boot_fast_ns() }
+///         }
+///     }
+/// }
+///
+/// // This is the function we want to test. Since `bindings` has been mocked, we can use its
+/// // functions seamlessly.
+/// fn get_boot_ns() -> u64 {
+///     unsafe { bindings::ktime_get_boot_fast_ns() }
+/// }
+///
+/// let time = get_boot_ns();
+/// assert_eq!(time, 1234);
+/// ```
+pub fn in_kunit_test() -> bool {
+    if cfg!(CONFIG_KUNIT) {
+        // SAFETY: By the type invariant, we know that `*Task::current().deref().0` is valid.
+        let test = unsafe { (*Task::current().deref().0.get()).kunit_test };
+        !test.is_null()
+    } else {
+        false
+    }
+}
+
 #[kunit_tests(rust_kernel_kunit)]
 mod tests {
+    use super::*;
+
     #[test]
     fn rust_test_kunit_kunit_tests() {
         let running = true;
         assert_eq!(running, true);
     }
+
+    #[test]
+    fn rust_test_kunit_in_kunit_test() {
+        let in_kunit = in_kunit_test();
+        assert_eq!(in_kunit, true);
+    }
 }

-- 
2.41.0.255.g8b1d071c50-goog


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] rust: kunit: add KUnit case and suite macros
  2023-07-20  6:38 ` [PATCH 1/3] rust: kunit: add KUnit case and suite macros David Gow
@ 2023-07-25 18:07   ` Boqun Feng
  0 siblings, 0 replies; 8+ messages in thread
From: Boqun Feng @ 2023-07-25 18:07 UTC (permalink / raw)
  To: David Gow
  Cc: Brendan Higgins, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
	Gary Guo, Benno Lossin, José Expósito,
	Björn Roy Baron, linux-kselftest, kunit-dev, rust-for-linux,
	linux-kernel

On Thu, Jul 20, 2023 at 02:38:52PM +0800, David Gow wrote:
> From: José Expósito <jose.exposito89@gmail.com>
> 
> Add a couple of Rust macros to allow to develop KUnit tests without
> relying on generated C code:
> 
>  - The `kunit_unsafe_test_suite!` Rust macro is similar to the
>    `kunit_test_suite` C macro.
>  - The `kunit_case!` Rust macro is similar to the `KUNIT_CASE` C macro.
>    It can be used with parameters to generate a test case or without
>    parameters to be used as delimiter in `kunit_test_suite!`.
> 
> While these macros can be used on their own, a future patch will
> introduce another macro to create KUnit tests using a user-space like
> syntax.
> 
> Co-developed-by: David Gow <davidgow@google.com>
> Signed-off-by: David Gow <davidgow@google.com>
> Signed-off-by: José Expósito <jose.exposito89@gmail.com>
> ---
>  rust/kernel/kunit.rs | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs   |  1 +
>  2 files changed, 93 insertions(+)
> 
> diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
> index 722655b2d62d..4cffc71e463b 100644
> --- a/rust/kernel/kunit.rs
> +++ b/rust/kernel/kunit.rs
> @@ -161,3 +161,95 @@ macro_rules! kunit_assert_eq {
>          $crate::kunit_assert!($name, $file, $diff, $left == $right);
>      }};
>  }
> +
> +/// Represents an individual test case.
> +///
> +/// The test case should have the signature
> +/// `unsafe extern "C" fn test_case(test: *mut crate::bindings::kunit)`.
> +///
> +/// The `kunit_unsafe_test_suite!` macro expects a NULL-terminated list of test cases. This macro
> +/// can be invoked without parameters to generate the delimiter.
> +#[macro_export]
> +macro_rules! kunit_case {

kunit_case doesn't need to be a macro, right? We can define it as a
const fn. Maybe one `kunit_case_null` and one `kunit_case`. Macros
should be avoided whenever possible.

Thoughts?

Regards,
Boqun

> +    () => {
> +        $crate::bindings::kunit_case {
> +            run_case: None,
> +            name: core::ptr::null_mut(),
> +            generate_params: None,
> +            status: $crate::bindings::kunit_status_KUNIT_SUCCESS,
> +            log: core::ptr::null_mut(),
> +        }
> +    };
> +    ($name:ident, $run_case:ident) => {
> +        $crate::bindings::kunit_case {
> +            run_case: Some($run_case),
> +            name: $crate::c_str!(core::stringify!($name)).as_char_ptr(),
> +            generate_params: None,
> +            status: $crate::bindings::kunit_status_KUNIT_SUCCESS,
> +            log: core::ptr::null_mut(),
> +        }
> +    };
> +}
> +
> +/// Registers a KUnit test suite.
> +///
> +/// # Safety
> +///
> +/// `test_cases` must be a NULL terminated array of test cases.
> +///
> +/// # Examples
> +///
> +/// ```ignore
> +/// unsafe extern "C" fn test_fn(_test: *mut crate::bindings::kunit) {
> +///     let actual = 1 + 1;
> +///     let expected = 2;
> +///     assert_eq!(actual, expected);
> +/// }
> +///
> +/// static mut KUNIT_TEST_CASE: crate::bindings::kunit_case = crate::kunit_case!(name, test_fn);
> +/// static mut KUNIT_NULL_CASE: crate::bindings::kunit_case = crate::kunit_case!();
> +/// static mut KUNIT_TEST_CASES: &mut[crate::bindings::kunit_case] = unsafe {
> +///     &mut[KUNIT_TEST_CASE, KUNIT_NULL_CASE]
> +/// };
> +/// crate::kunit_unsafe_test_suite!(suite_name, KUNIT_TEST_CASES);
> +/// ```
> +#[macro_export]
> +macro_rules! kunit_unsafe_test_suite {
> +    ($name:ident, $test_cases:ident) => {
> +        const _: () = {
> +            static KUNIT_TEST_SUITE_NAME: [i8; 256] = {
> +                let name_u8 = core::stringify!($name).as_bytes();
> +                let mut ret = [0; 256];
> +
> +                let mut i = 0;
> +                while i < name_u8.len() {
> +                    ret[i] = name_u8[i] as i8;
> +                    i += 1;
> +                }
> +
> +                ret
> +            };
> +
> +            // SAFETY: `test_cases` is valid as it should be static.
> +            static mut KUNIT_TEST_SUITE: core::cell::UnsafeCell<$crate::bindings::kunit_suite> =
> +                core::cell::UnsafeCell::new($crate::bindings::kunit_suite {
> +                    name: KUNIT_TEST_SUITE_NAME,
> +                    test_cases: unsafe { $test_cases.as_mut_ptr() },
> +                    suite_init: None,
> +                    suite_exit: None,
> +                    init: None,
> +                    exit: None,
> +                    status_comment: [0; 256usize],
> +                    debugfs: core::ptr::null_mut(),
> +                    log: core::ptr::null_mut(),
> +                    suite_init_err: 0,
> +                });
> +
> +            // SAFETY: `KUNIT_TEST_SUITE` is static.
> +            #[used]
> +            #[link_section = ".kunit_test_suites"]
> +            static mut KUNIT_TEST_SUITE_ENTRY: *const $crate::bindings::kunit_suite =
> +                unsafe { KUNIT_TEST_SUITE.get() };
> +        };
> +    };
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 3642cadc34b1..ec81fd28d71a 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -18,6 +18,7 @@
>  #![feature(new_uninit)]
>  #![feature(receiver_trait)]
>  #![feature(unsize)]
> +#![feature(const_mut_refs)]
>  
>  // Ensure conditional compilation based on the kernel configuration works;
>  // otherwise we may silently break things like initcall handling.
> 
> -- 
> 2.41.0.255.g8b1d071c50-goog
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] rust: kunit: allow to know if we are in a test
  2023-07-20  6:38 ` [PATCH 3/3] rust: kunit: allow to know if we are in a test David Gow
@ 2023-07-25 23:34   ` Boqun Feng
  0 siblings, 0 replies; 8+ messages in thread
From: Boqun Feng @ 2023-07-25 23:34 UTC (permalink / raw)
  To: David Gow
  Cc: Brendan Higgins, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
	Gary Guo, Benno Lossin, José Expósito,
	Björn Roy Baron, linux-kselftest, kunit-dev, rust-for-linux,
	linux-kernel

On Thu, Jul 20, 2023 at 02:38:54PM +0800, David Gow wrote:
> From: José Expósito <jose.exposito89@gmail.com>
> 
> In some cases, you need to call test-only code from outside the test
> case, for example, to mock a function or a module.
> 
> In order to check whether we are in a test or not, we need to test if
> `CONFIG_KUNIT` is set.
> Unfortunately, we cannot rely only on this condition because some
> distros compile KUnit in production kernels, so checking at runtime
> that `current->kunit_test != NULL` is required.
> 
> Note that the C function `kunit_get_current_test()` can not be used
> because it is not present in the current Rust tree yet. Once it is
> available we might want to change our Rust wrapper to use it.
> 
> This patch adds a function to know whether we are in a KUnit test or
> not and examples showing how to mock a function and a module.
> 
> Reviewed-by: David Gow <davidgow@google.com>
> Signed-off-by: José Expósito <jose.exposito89@gmail.com>
> ---
>  rust/kernel/kunit.rs | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 78 insertions(+)
> 
> diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
> index 44ea67028316..dcaac19bb108 100644
> --- a/rust/kernel/kunit.rs
> +++ b/rust/kernel/kunit.rs
> @@ -40,6 +40,8 @@ pub fn info(args: fmt::Arguments<'_>) {
>      }
>  }
>  
> +use crate::task::Task;
> +use core::ops::Deref;
>  use macros::kunit_tests;
>  
>  /// Asserts that a boolean expression is `true` at runtime.
> @@ -256,11 +258,87 @@ macro_rules! kunit_unsafe_test_suite {
>      };
>  }
>  
> +/// In some cases, you need to call test-only code from outside the test case, for example, to
> +/// create a function mock. This function can be invoked to know whether we are currently running a
> +/// KUnit test or not.
> +///
> +/// # Examples
> +///
> +/// This example shows how a function can be mocked to return a well-known value while testing:
> +///
> +/// ```
> +/// # use kernel::kunit::in_kunit_test;
> +/// #
> +/// fn fn_mock_example(n: i32) -> i32 {
> +///     if in_kunit_test() {
> +///         100
> +///     } else {
> +///         n + 1
> +///     }
> +/// }
> +///
> +/// let mock_res = fn_mock_example(5);
> +/// assert_eq!(mock_res, 100);
> +/// ```
> +///
> +/// Sometimes, you don't control the code that needs to be mocked. This example shows how the
> +/// `bindings` module can be mocked:
> +///
> +/// ```
> +/// // Import our mock naming it as the real module.
> +/// #[cfg(CONFIG_KUNIT)]
> +/// use bindings_mock_example as bindings;
> +///
> +/// // This module mocks `bindings`.
> +/// mod bindings_mock_example {
> +///     use kernel::kunit::in_kunit_test;
> +///     use kernel::bindings::u64_;
> +///
> +///     // Make the other binding functions available.
> +///     pub(crate) use kernel::bindings::*;
> +///
> +///     // Mock `ktime_get_boot_fast_ns` to return a well-known value when running a KUnit test.
> +///     pub(crate) unsafe fn ktime_get_boot_fast_ns() -> u64_ {
> +///         if in_kunit_test() {
> +///             1234
> +///         } else {
> +///             unsafe { kernel::bindings::ktime_get_boot_fast_ns() }
> +///         }
> +///     }
> +/// }
> +///
> +/// // This is the function we want to test. Since `bindings` has been mocked, we can use its
> +/// // functions seamlessly.
> +/// fn get_boot_ns() -> u64 {
> +///     unsafe { bindings::ktime_get_boot_fast_ns() }
> +/// }
> +///
> +/// let time = get_boot_ns();
> +/// assert_eq!(time, 1234);
> +/// ```
> +pub fn in_kunit_test() -> bool {
> +    if cfg!(CONFIG_KUNIT) {
> +        // SAFETY: By the type invariant, we know that `*Task::current().deref().0` is valid.
> +        let test = unsafe { (*Task::current().deref().0.get()).kunit_test };

Note here are two unsafe operations: `Task::current()` and the pointer
dereference. You can use the `current!()` macro here to avoid the first
unsafe operation here. Besides I think it'll be better if
in_kunit_test() is a safe method for `Task`? That will be easier for us
to track the usage of task_struct fields in Rust side. But I'm OK with
either way.

Regards,
Boqun

> +        !test.is_null()
> +    } else {
> +        false
> +    }
> +}
> +
>  #[kunit_tests(rust_kernel_kunit)]
>  mod tests {
> +    use super::*;
> +
>      #[test]
>      fn rust_test_kunit_kunit_tests() {
>          let running = true;
>          assert_eq!(running, true);
>      }
> +
> +    #[test]
> +    fn rust_test_kunit_in_kunit_test() {
> +        let in_kunit = in_kunit_test();
> +        assert_eq!(in_kunit, true);
> +    }
>  }
> 
> -- 
> 2.41.0.255.g8b1d071c50-goog
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] rust: macros: add macro to easily run KUnit tests
  2023-07-20  6:38 ` [PATCH 2/3] rust: macros: add macro to easily run KUnit tests David Gow
@ 2023-07-30 21:49   ` Boqun Feng
  2023-08-01 14:44   ` Miguel Ojeda
  1 sibling, 0 replies; 8+ messages in thread
From: Boqun Feng @ 2023-07-30 21:49 UTC (permalink / raw)
  To: David Gow
  Cc: Brendan Higgins, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
	Gary Guo, Benno Lossin, José Expósito,
	Björn Roy Baron, linux-kselftest, kunit-dev, rust-for-linux,
	linux-kernel

On Thu, Jul 20, 2023 at 02:38:53PM +0800, David Gow wrote:
> From: José Expósito <jose.exposito89@gmail.com>
> 
> Add a new procedural macro (`#[kunit_tests(kunit_test_suit_name)]`) to
> run KUnit tests using a user-space like syntax.
> 
> The macro, that should be used on modules, transforms every `#[test]`
> in a `kunit_case!` and adds a `kunit_unsafe_test_suite!` registering
> all of them.
> 
> The only difference with user-space tests is that instead of using
> `#[cfg(test)]`, `#[kunit_tests(kunit_test_suit_name)]` is used.
> 
> Note that `#[cfg(CONFIG_KUNIT)]` is added so the test module is not
> compiled when `CONFIG_KUNIT` is set to `n`.
> 
> Reviewed-by: David Gow <davidgow@google.com>
> Signed-off-by: José Expósito <jose.exposito89@gmail.com>
> ---
>  MAINTAINERS          |   1 +
>  rust/kernel/kunit.rs |  11 ++++
>  rust/macros/kunit.rs | 149 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  rust/macros/lib.rs   |  29 ++++++++++
>  4 files changed, 190 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2a942fe59144..c32ba6b29a96 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11373,6 +11373,7 @@ F:	Documentation/dev-tools/kunit/
>  F:	include/kunit/
>  F:	lib/kunit/
>  F:	rust/kernel/kunit.rs
> +F:	rust/macros/kunit.rs
>  F:	scripts/rustdoc_test_*
>  F:	tools/testing/kunit/
>  
> diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
> index 4cffc71e463b..44ea67028316 100644
> --- a/rust/kernel/kunit.rs
> +++ b/rust/kernel/kunit.rs
> @@ -40,6 +40,8 @@ pub fn info(args: fmt::Arguments<'_>) {
>      }
>  }
>  
> +use macros::kunit_tests;
> +
>  /// Asserts that a boolean expression is `true` at runtime.
>  ///
>  /// Public but hidden since it should only be used from generated tests.
> @@ -253,3 +255,12 @@ macro_rules! kunit_unsafe_test_suite {
>          };
>      };
>  }
> +
> +#[kunit_tests(rust_kernel_kunit)]
> +mod tests {
> +    #[test]
> +    fn rust_test_kunit_kunit_tests() {
> +        let running = true;
> +        assert_eq!(running, true);
> +    }
> +}
> diff --git a/rust/macros/kunit.rs b/rust/macros/kunit.rs
> new file mode 100644
> index 000000000000..69dac253232f
> --- /dev/null
> +++ b/rust/macros/kunit.rs
> @@ -0,0 +1,149 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Procedural macro to run KUnit tests using a user-space like syntax.
> +//!
> +//! Copyright (c) 2023 José Expósito <jose.exposito89@gmail.com>
> +
> +use proc_macro::{Delimiter, Group, TokenStream, TokenTree};
> +use std::fmt::Write;
> +
> +pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
> +    if attr.to_string().is_empty() {
> +        panic!("Missing test name in #[kunit_tests(test_name)] macro")
> +    }
> +
> +    let mut tokens: Vec<_> = ts.into_iter().collect();
> +
> +    // Scan for the "mod" keyword.
> +    tokens
> +        .iter()
> +        .find_map(|token| match token {
> +            TokenTree::Ident(ident) => match ident.to_string().as_str() {
> +                "mod" => Some(true),
> +                _ => None,
> +            },
> +            _ => None,
> +        })
> +        .expect("#[kunit_tests(test_name)] attribute should only be applied to modules");
> +
> +    // Retrieve the main body. The main body should be the last token tree.
> +    let body = match tokens.pop() {
> +        Some(TokenTree::Group(group)) if group.delimiter() == Delimiter::Brace => group,
> +        _ => panic!("cannot locate main body of module"),
> +    };
> +
> +    // Get the functions set as tests. Search for `[test]` -> `fn`.
> +    let mut body_it = body.stream().into_iter();
> +    let mut tests = Vec::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);
> +                }
> +                _ => continue,
> +            },
> +            _ => (),
> +        }
> +    }
> +
> +    // Add `#[cfg(CONFIG_KUNIT)]` before the module declaration.
> +    let config_kunit = "#[cfg(CONFIG_KUNIT)]".to_owned().parse().unwrap();
> +    tokens.insert(
> +        0,
> +        TokenTree::Group(Group::new(Delimiter::None, config_kunit)),
> +    );
> +
> +    // Generate the test KUnit test suite and a test case for each `#[test]`.
> +    // The code generated for the following test module:
> +    //
> +    // ```
> +    // #[kunit_tests(kunit_test_suit_name)]
> +    // mod tests {
> +    //     #[test]
> +    //     fn foo() {
> +    //         assert_eq!(1, 1);
> +    //     }
> +    //
> +    //     #[test]
> +    //     fn bar() {
> +    //         assert_eq!(2, 2);
> +    //     }
> +    // ```
> +    //
> +    // Looks like:
> +    //
> +    // ```
> +    // unsafe extern "C" fn kunit_rust_wrapper_foo(_test: *mut kernel::bindings::kunit) {
> +    //     foo();
> +    // }
> +    // static mut KUNIT_CASE_FOO: kernel::bindings::kunit_case =
> +    //     kernel::kunit_case!(foo, kunit_rust_wrapper_foo);
> +    //
> +    // unsafe extern "C" fn kunit_rust_wrapper_bar(_test: * mut kernel::bindings::kunit) {
> +    //     bar();
> +    // }
> +    // static mut KUNIT_CASE_BAR: kernel::bindings::kunit_case =
> +    //     kernel::kunit_case!(bar, kunit_rust_wrapper_bar);
> +    //
> +    // static mut KUNIT_CASE_NULL: kernel::bindings::kunit_case = kernel::kunit_case!();
> +    //
> +    // static mut TEST_CASES : &mut[kernel::bindings::kunit_case] = unsafe {
> +    //     &mut [KUNIT_CASE_FOO, KUNIT_CASE_BAR, KUNIT_CASE_NULL]
> +    // };
> +    //
> +    // kernel::kunit_unsafe_test_suite!(kunit_test_suit_name, TEST_CASES);
> +    // ```
> +    let mut kunit_macros = "".to_owned();
> +    let mut test_cases = "".to_owned();
> +    for test in tests {
> +        let kunit_wrapper_fn_name = format!("kunit_rust_wrapper_{}", test);
> +        let kunit_case_name = format!("KUNIT_CASE_{}", test.to_uppercase());
> +        let kunit_wrapper = format!(
> +            "unsafe extern \"C\" fn {}(_test: *mut kernel::bindings::kunit) {{ {}(); }}",
> +            kunit_wrapper_fn_name, test
> +        );
> +        let kunit_case = format!(
> +            "static mut {}: kernel::bindings::kunit_case = kernel::kunit_case!({}, {});",
> +            kunit_case_name, test, kunit_wrapper_fn_name
> +        );
> +        writeln!(kunit_macros, "{kunit_wrapper}").unwrap();
> +        writeln!(kunit_macros, "{kunit_case}").unwrap();
> +        writeln!(test_cases, "{kunit_case_name},").unwrap();
> +    }
> +
> +    writeln!(
> +        kunit_macros,
> +        "static mut KUNIT_CASE_NULL: kernel::bindings::kunit_case = kernel::kunit_case!();"
> +    )
> +    .unwrap();
> +
> +    writeln!(
> +        kunit_macros,
> +        "static mut TEST_CASES : &mut[kernel::bindings::kunit_case] = unsafe {{ &mut[{test_cases} KUNIT_CASE_NULL] }};"
> +    )
> +    .unwrap();
> +
> +    writeln!(
> +        kunit_macros,
> +        "kernel::kunit_unsafe_test_suite!({attr}, TEST_CASES);"
> +    )
> +    .unwrap();
> +
> +    let new_body: TokenStream = vec![body.stream(), kunit_macros.parse().unwrap()]
> +        .into_iter()
> +        .collect();
> +
> +    // Remove the `#[test]` macros.
> +    let new_body = new_body.to_string().replace("#[test]", "");

I've played this with some extra tests, one thing I notice is that
Span/code location information is lost if we do this, for example, if I
have a compile error in the test code (I introduced one on purpose in
the `rust_kernel_kunit` test), I will get information like:

	error[E0384]: cannot assign twice to immutable variable `running`
	   --> ../rust/kernel/kunit.rs:329:1
	    |
	329 | #[kunit_tests(rust_kernel_kunit)]
	    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
	    | |
	    | cannot assign twice to immutable variable
	    | help: consider making this binding mutable: `mut running`

the location information is not very usefull. However if we do the
following:

diff --git a/rust/macros/kunit.rs b/rust/macros/kunit.rs
index 69dac253232f..913879765d24 100644
--- a/rust/macros/kunit.rs
+++ b/rust/macros/kunit.rs
@@ -134,15 +134,29 @@ pub(crate) fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
     )
     .unwrap();
 
-    let new_body: TokenStream = vec![body.stream(), kunit_macros.parse().unwrap()]
-        .into_iter()
-        .collect();
-
     // Remove the `#[test]` macros.
-    let new_body = new_body.to_string().replace("#[test]", "");
+    let mut new_body = vec![];
+    let mut body_it = body.stream().into_iter();
+
+    while let Some(token) = body_it.next() {
+        match token {
+            TokenTree::Punct(ref c) if c.as_char() == '#' => {
+                match body_it.next() {
+                    Some(TokenTree::Group(g)) if g.to_string() == "[test]" => (),
+                    Some(next) => { new_body.extend([token, next]);},
+                    _ => {new_body.push(token);},
+                }
+            }
+            _ => { new_body.push(token); }
+        }
+    }
+
+    let mut new_body = TokenStream::from_iter(new_body);
+    new_body.extend::<TokenStream>(kunit_macros.parse().unwrap());
+
     tokens.push(TokenTree::Group(Group::new(
         Delimiter::Brace,
-        new_body.parse().unwrap(),
+        new_body
     )));
 
     tokens.into_iter().collect()


then we get better information:

	   --> ../rust/kernel/kunit.rs:335:13
	    |
	335 |         let running = true;
	    |             ^^^^^^^
	    |
	    = help: maybe it is overwritten before being read?
	    = note: `#[warn(unused_assignments)]` on by default

	error[E0384]: cannot assign twice to immutable variable `running`
	   --> ../rust/kernel/kunit.rs:336:9
	    |
	335 |         let running = true;
	    |             -------
	    |             |
	    |             first assignment to `running`
	    |             help: consider making this binding mutable: `mut running`
	336 |         running = false;
	    |         ^^^^^^^^^^^^^^^ cannot assign twice to immutable variable


Regards,
Boqun

> +    tokens.push(TokenTree::Group(Group::new(
> +        Delimiter::Brace,
> +        new_body.parse().unwrap(),
> +    )));
> +
> +    tokens.into_iter().collect()
> +}
> diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
> index 3fc74cb4ea19..cd0b720514ff 100644
> --- a/rust/macros/lib.rs
> +++ b/rust/macros/lib.rs
> @@ -6,6 +6,7 @@
>  mod quote;
>  mod concat_idents;
>  mod helpers;
> +mod kunit;
>  mod module;
>  mod pin_data;
>  mod pinned_drop;
> @@ -246,3 +247,31 @@ pub fn pin_data(inner: TokenStream, item: TokenStream) -> TokenStream {
>  pub fn pinned_drop(args: TokenStream, input: TokenStream) -> TokenStream {
>      pinned_drop::pinned_drop(args, input)
>  }
> +
> +/// Registers a KUnit test suite and its test cases using a user-space like syntax.
> +///
> +/// This macro should be used on modules. If `CONFIG_KUNIT` (in `.config`) is `n`, the target module
> +/// is ignored.
> +///
> +/// # Examples
> +///
> +/// ```ignore
> +/// # use macros::kunit_tests;
> +///
> +/// #[kunit_tests(kunit_test_suit_name)]
> +/// mod tests {
> +///     #[test]
> +///     fn foo() {
> +///         assert_eq!(1, 1);
> +///     }
> +///
> +///     #[test]
> +///     fn bar() {
> +///         assert_eq!(2, 2);
> +///     }
> +/// }
> +/// ```
> +#[proc_macro_attribute]
> +pub fn kunit_tests(attr: TokenStream, ts: TokenStream) -> TokenStream {
> +    kunit::kunit_tests(attr, ts)
> +}
> 
> -- 
> 2.41.0.255.g8b1d071c50-goog
> 

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] rust: macros: add macro to easily run KUnit tests
  2023-07-20  6:38 ` [PATCH 2/3] rust: macros: add macro to easily run KUnit tests David Gow
  2023-07-30 21:49   ` Boqun Feng
@ 2023-08-01 14:44   ` Miguel Ojeda
  1 sibling, 0 replies; 8+ messages in thread
From: Miguel Ojeda @ 2023-08-01 14:44 UTC (permalink / raw)
  To: David Gow, José Expósito
  Cc: Brendan Higgins, Miguel Ojeda, Alex Gaynor, Wedson Almeida Filho,
	Boqun Feng, Gary Guo, Benno Lossin, Björn Roy Baron,
	linux-kselftest, kunit-dev, rust-for-linux, linux-kernel

On Thu, Jul 20, 2023 at 8:40 AM David Gow <davidgow@google.com> wrote:
>
> The only difference with user-space tests is that instead of using
> `#[cfg(test)]`, `#[kunit_tests(kunit_test_suit_name)]` is used.

I may be missing something, but this does not appear to map the
`assert*!`s to the KUnit APIs, is that correct? (i.e. like we do for
`rustdoc`-tests).

I made an assertion fail, and it seems to use the standard library
macros, thus panicking and ending up in `BUG()` (rather than a failed
test):

    rust_kernel: panicked at 'assertion failed: `(left == right)`
      left: `true`,
     right: `false`', rust/kernel/kunit.rs:329:1
    ------------[ cut here ]------------
    kernel BUG at rust/helpers.c:34!

Then the test times out eventually and things break:

        # rust_test_kunit_kunit_tests: try timed out
    ------------[ cut here ]------------
    refcount_t: addition on 0; use-after-free.

> +    // Add `#[cfg(CONFIG_KUNIT)]` before the module declaration.
> +    let config_kunit = "#[cfg(CONFIG_KUNIT)]".to_owned().parse().unwrap();
> +    tokens.insert(
> +        0,
> +        TokenTree::Group(Group::new(Delimiter::None, config_kunit)),
> +    );

I wonder about compile-time here with this approach. As far as I
understand, having the `cfg` explicitly outside the proc macro would
avoid invoking it.

Do we know the potential compile-time impact, especially when we will
have many tests?
ventually it would be ideal to have an approach closer to the
`rustdoc` one, where the compiler finds the tests for us and we
generate the needed code in the build system, i.e. outside a proc
macro.

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-08-01 14:44 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-20  6:38 [PATCH 0/3] rust: kunit: Support KUnit tests with a user-space like syntax David Gow
2023-07-20  6:38 ` [PATCH 1/3] rust: kunit: add KUnit case and suite macros David Gow
2023-07-25 18:07   ` Boqun Feng
2023-07-20  6:38 ` [PATCH 2/3] rust: macros: add macro to easily run KUnit tests David Gow
2023-07-30 21:49   ` Boqun Feng
2023-08-01 14:44   ` Miguel Ojeda
2023-07-20  6:38 ` [PATCH 3/3] rust: kunit: allow to know if we are in a test David Gow
2023-07-25 23:34   ` Boqun Feng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.