On Wed, Aug 21, 2024 at 6:00 PM Michal Wajdeczko
<[email protected]> wrote:
>
>
>
> On 21.08.2024 23:22, Rae Moar wrote:
> > On Wed, Aug 21, 2024 at 10:43 AM Michal Wajdeczko
> > <[email protected]> wrote:
> >>
> >> Add example how to use KUNIT_FIXED_STUB_REDIRECT and compare its
> >> usage with the KUNIT_STATIC_STUB_REDIRECT. Also show how the
> >> DECLARE_IF_KUNIT macro could be helpful in declaring test data in
> >> the non-test data structures.
> >>
> >> Signed-off-by: Michal Wajdeczko <[email protected]>
> >
> > Hello!
> >
> > I really like this test. It provides a great overview of this patch
> > series. I just have a couple comments below.
> >
> > Otherwise,
> > Reviewed-by: Rae Moar <[email protected]>
> >
> > Thanks!
> > -Rae
> >
> >> ---
> >> Cc: David Gow <[email protected]>
> >> Cc: Daniel Latypov <[email protected]>
> >> Cc: Lucas De Marchi <[email protected]>
> >> ---
> >> lib/kunit/kunit-example-test.c | 63 ++++++++++++++++++++++++++++++++++
> >> 1 file changed, 63 insertions(+)
> >>
> >> diff --git a/lib/kunit/kunit-example-test.c
> >> b/lib/kunit/kunit-example-test.c
> >> index 3056d6bc705d..120e08d8899b 100644
> >> --- a/lib/kunit/kunit-example-test.c
> >> +++ b/lib/kunit/kunit-example-test.c
> >> @@ -6,8 +6,10 @@
> >> * Author: Brendan Higgins <[email protected]>
> >> */
> >>
> >> +#include <linux/workqueue.h>
> >> #include <kunit/test.h>
> >> #include <kunit/static_stub.h>
> >> +#include <kunit/visibility.h>
> >>
> >> /*
> >> * This is the most fundamental element of KUnit, the test case. A test
> >> case
> >> @@ -221,6 +223,66 @@ static void
> >> example_static_stub_using_fn_ptr_test(struct kunit *test)
> >> KUNIT_EXPECT_EQ(test, add_one(1), 2);
> >> }
> >>
> >> +/* This could be a location of various fixed stub functions. */
> >> +static struct {
> >> + DECLARE_IF_KUNIT(int (*add_two)(int i));
> >
> > Is the use of DECLARE_IF_KUNIT useful here? KUnit should always be
> > enabled if this file is being compiled/run. Is the idea to show an
> > example here that could be used outside of kunit test files?
>
> yes, the idea was to show that 'stubs' declarations could be placed
> anywhere, without any cost if compiled without KUNIT (I was trying to
> mention that in commit message)
>
> >
> > Additionally, would it make sense to call this add_two_stub instead to
> > make it clear that this is not a definition of the add_two function?
> > Or is it helpful for people to see this as an example of how to handle
> > multiple stubs: struct of stubs with exact names? Let me know what you
> > think.
>
> the 'add_two' above is just a member name, and IMO we shouldn't repeat
> that this is about 'stub' since the whole struct is for 'stubs'
>
> and yes, the idea was also to show that if applicable, other function
> stubs declarations could be either placed together
Happy with this as is then! Thanks for your response.
-Rae
>
> >
> >> +} stubs;
> >> +
> >> +/* This is a function we'll replace with stubs. */
> >> +static int add_two(int i)
> >> +{
> >> + /* This will trigger the stub if active. */
> >> + KUNIT_STATIC_STUB_REDIRECT(add_two, i);
> >> + KUNIT_FIXED_STUB_REDIRECT(stubs.add_two, i);
> >> +
> >> + return i + 2;
> >> +}
> >> +
> >> +struct add_two_async_work {
> >> + struct work_struct work;
> >> + int param;
> >> + int result;
> >> +};
> >> +
> >> +static void add_two_async_func(struct work_struct *work)
> >> +{
> >> + struct add_two_async_work *w = container_of(work, typeof(*w),
> >> work);
> >> +
> >> + w->result = add_two(w->param);
> >> +}
> >> +
> >> +static int add_two_async(int i)
> >> +{
> >> + struct add_two_async_work w = { .param = i };
> >> +
> >> + INIT_WORK_ONSTACK(&w.work, add_two_async_func);
> >> + schedule_work(&w.work);
> >> + flush_work(&w.work);
> >> + destroy_work_on_stack(&w.work);
> >> +
> >> + return w.result;
> >> +}
> >> +
> >> +/*
> >> + */
> >
> > It looks like the method description is missing here.
> >
>
> ha, I missed to copy commit message here
>
> >
> >
> >
> >> +static void example_fixed_stub_test(struct kunit *test)
> >> +{
> >> + /* static stub redirection works only for KUnit thread */
> >> + kunit_activate_static_stub(test, add_two, subtract_one);
> >> + KUNIT_EXPECT_EQ(test, add_two(1), subtract_one(1));
> >> + KUNIT_EXPECT_NE_MSG(test, add_two_async(1), subtract_one(1),
> >> + "stub shouldn't be active outside KUnit
> >> thread!");
> >> + kunit_deactivate_static_stub(test, add_two);
> >> + KUNIT_EXPECT_EQ(test, add_two(1), add_two(1));
> >> +
> >> + /* fixed stub redirection works for KUnit and other threads */
> >> + kunit_activate_fixed_stub(test, stubs.add_two, subtract_one);
> >> + KUNIT_EXPECT_EQ(test, add_two(1), subtract_one(1));
> >> + KUNIT_EXPECT_EQ(test, add_two_async(1), subtract_one(1));
> >> + kunit_deactivate_fixed_stub(test, stubs.add_two);
> >> + KUNIT_EXPECT_EQ(test, add_two(1), add_two(1));
> >> +}
> >> +
> >> static const struct example_param {
> >> int value;
> >> } example_params_array[] = {
> >> @@ -294,6 +356,7 @@ static struct kunit_case example_test_cases[] = {
> >> KUNIT_CASE(example_all_expect_macros_test),
> >> KUNIT_CASE(example_static_stub_test),
> >> KUNIT_CASE(example_static_stub_using_fn_ptr_test),
> >> + KUNIT_CASE(example_fixed_stub_test),
> >> KUNIT_CASE(example_priv_test),
> >> KUNIT_CASE_PARAM(example_params_test, example_gen_params),
> >> KUNIT_CASE_SLOW(example_slow_test),
> >> --
> >> 2.43.0
> >>
> >> --
> >> You received this message because you are subscribed to the Google Groups
> >> "KUnit Development" group.
> >> To unsubscribe from this group and stop receiving emails from it, send an
> >> email to [email protected].
> >> To view this discussion on the web visit
> >> https://groups.google.com/d/msgid/kunit-dev/20240821144305.1958-5-michal.wajdeczko%40intel.com.
