Message-ID: <CAJ-ks9kwVz4sPdmqfTLVK-Z2C7WmXHpBhbe5_ozWBZnwxJ8HpA@mail.gmail.com>
Date: Fri, 14 Feb 2025 09:41:02 -0500
From: Tamir Duberstein <tamird@...il.com>
To: David Gow <davidgow@...gle.com>
Cc: Miguel Ojeda <ojeda@...nel.org>, José Expósito <jose.exposito89@...il.com>, 
	Rae Moar <rmoar@...gle.com>, Boqun Feng <boqun.feng@...il.com>, 
	Alex Gaynor <alex.gaynor@...il.com>, Gary Guo <gary@...yguo.net>, 
	Benno Lossin <benno.lossin@...ton.me>, Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Alice Ryhl <aliceryhl@...gle.com>, Matt Gilbride <mattgilbride@...gle.com>, 
	Brendan Higgins <brendan.higgins@...ux.dev>, kunit-dev@...glegroups.com, 
	linux-kselftest@...r.kernel.org, rust-for-linux@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 3/3] rust: kunit: allow to know if we are in a test

On Fri, Feb 14, 2025 at 2:42 AM David Gow <davidgow@...gle.com> wrote:
>
> From: José Expósito <jose.exposito89@...il.com>
>
> In some cases, we 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:
> - a test could be running in another thread,
> - some distros compile KUnit in production kernels, so checking at runtime
>   that `current->kunit_test != NULL` is required.
>
> Forturately, KUnit provides an optimised check in
> `kunit_get_current_test()`, which checks CONFIG_KUNIT, a global static
> key, and then the current thread's running KUnit test.
>
> Add a safe wrapper function around this to know whether or not we are in
> a KUnit test and examples showing how to mock a function and a module.
>
> Signed-off-by: José Expósito <jose.exposito89@...il.com>
> Co-developed-by: David Gow <davidgow@...gle.com>
> Signed-off-by: David Gow <davidgow@...gle.com>
> Co-developed-by: Miguel Ojeda <ojeda@...nel.org>
> Signed-off-by: Miguel Ojeda <ojeda@...nel.org>
> ---
>
> Changes since v5:
> https://lore.kernel.org/all/20241213081035.2069066-4-davidgow@google.com/
> - Greatly improved documentation, which is both clearer and better
>   matches the rustdoc norm. (Thanks, Miguel)
> - The examples and safety comments are also both more idiomatic an
>   cleaner. (Thanks, Miguel)
> - More things sit appropriately behind CONFIG_KUNIT (Thanks, Miguel)
>
> Changes since v4:
> https://lore.kernel.org/linux-kselftest/20241101064505.3820737-4-davidgow@google.com/
> - Rebased against 6.13-rc1
> - Fix some missing safety comments, and remove some unneeded 'unsafe'
>   blocks. (Thanks Boqun)
>
> Changes since v3:
> https://lore.kernel.org/linux-kselftest/20241030045719.3085147-8-davidgow@google.com/
> - The example test has been updated to no longer use assert_eq!() with
>   a constant bool argument (fixes a clippy warning).
>
> No changes since v2:
> https://lore.kernel.org/linux-kselftest/20241029092422.2884505-4-davidgow@google.com/
>
> Changes since v1:
> https://lore.kernel.org/lkml/20230720-rustbind-v1-3-c80db349e3b5@google.com/
> - Rebased on top of rust-next.
> - Use the `kunit_get_current_test()` C function, which wasn't previously
>   available, instead of rolling our own.
> - (Thanks also to Boqun for suggesting a nicer way of implementing this,
>   which I tried, but the `kunit_get_current_test()` version obsoleted.)
> ---
>  rust/kernel/kunit.rs | 66 ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 66 insertions(+)
>
> diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
> index 9e27b74a605b..3aad7a281b6d 100644
> --- a/rust/kernel/kunit.rs
> +++ b/rust/kernel/kunit.rs
> @@ -286,11 +286,77 @@ macro_rules! kunit_unsafe_test_suite {
>      };
>  }
>
> +/// Returns whether we are currently running a KUnit test.
> +///
> +/// In some cases, you need to call test-only code from outside the test case, for example, to
> +/// create a function mock. This function allows to change behavior depending on 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() {
> +///         return 100;
> +///     }
> +///
> +///     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:

[`bindings`] here, please. There are two more instances below but
those aren't doc comments, so I don't think bracketing them will do
anything.

> +///
> +/// ```
> +/// // Import our mock naming it as the real module.
> +/// #[cfg(CONFIG_KUNIT)]
> +/// use bindings_mock_example as bindings;
> +/// #[cfg(not(CONFIG_KUNIT))]
> +/// use kernel::bindings;
> +///
> +/// // This module mocks `bindings`.
> +/// #[cfg(CONFIG_KUNIT)]
> +/// mod bindings_mock_example {
> +///     /// Mock `ktime_get_boot_fast_ns` to return a well-known value when running a KUnit test.
> +///     pub(crate) fn ktime_get_boot_fast_ns() -> u64 {
> +///         1234
> +///     }
> +/// }
> +///
> +/// // This is the function we want to test. Since `bindings` has been mocked, we can use its
> +/// // functions seamlessly.
> +/// fn get_boot_ns() -> u64 {
> +///     // SAFETY: `ktime_get_boot_fast_ns()` is always safe to call.
> +///     unsafe { bindings::ktime_get_boot_fast_ns() }
> +/// }
> +///
> +/// let time = get_boot_ns();
> +/// assert_eq!(time, 1234);
> +/// ```

Isn't this swapping out the bindings module at compile time, and for
the whole build? In other words cfg(CONFIG_KUNIT) will apply to all
code, both test and non-test.

> +pub fn in_kunit_test() -> bool {
> +    // SAFETY: `kunit_get_current_test()` is always safe to call (it has fallbacks for
> +    // when KUnit is not enabled).
> +    unsafe { !bindings::kunit_get_current_test().is_null() }

Nit if you care about reducing unsafe blocks:

!unsafe { bindings::kunit_get_current_test() }.is_null()


> +}
> +
>  #[kunit_tests(rust_kernel_kunit)]
>  mod tests {
> +    use super::*;
> +
>      #[test]
>      fn rust_test_kunit_example_test() {
>          #![expect(clippy::eq_op)]
>          assert_eq!(1 + 1, 2);
>      }
> +
> +    #[test]
> +    fn rust_test_kunit_in_kunit_test() {
> +        assert!(in_kunit_test());
> +    }
>  }
> --
> 2.48.1.601.g30ceb7b040-goog
>
>

Powered by blists - more mailing lists