2023-07-20 07:07:08

by David Gow

[permalink] [raw]
Subject: [PATCH 0/3] rust: kunit: Support KUnit tests with a user-space like syntax

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/[email protected]/
- Add rust/macros/kunit.rs to the KUnit MAINTAINERS entry

Signed-off-by: David Gow <[email protected]>
---
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 <[email protected]>



2023-07-20 07:11:50

by David Gow

[permalink] [raw]
Subject: [PATCH 1/3] rust: kunit: add KUnit case and suite macros

From: José Expósito <[email protected]>

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 <[email protected]>
Signed-off-by: David Gow <[email protected]>
Signed-off-by: José Expósito <[email protected]>
---
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


2023-07-20 08:10:12

by David Gow

[permalink] [raw]
Subject: [PATCH 3/3] rust: kunit: allow to know if we are in a test

From: José Expósito <[email protected]>

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 <[email protected]>
Signed-off-by: José Expósito <[email protected]>
---
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


2023-07-25 18:35:42

by Boqun Feng

[permalink] [raw]
Subject: Re: [PATCH 1/3] rust: kunit: add KUnit case and suite macros

On Thu, Jul 20, 2023 at 02:38:52PM +0800, David Gow wrote:
> From: Jos? Exp?sito <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: David Gow <[email protected]>
> Signed-off-by: Jos? Exp?sito <[email protected]>
> ---
> 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
>

2023-07-26 01:57:45

by Boqun Feng

[permalink] [raw]
Subject: Re: [PATCH 3/3] rust: kunit: allow to know if we are in a test

On Thu, Jul 20, 2023 at 02:38:54PM +0800, David Gow wrote:
> From: Jos? Exp?sito <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Jos? Exp?sito <[email protected]>
> ---
> 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
>