Re: [PATCH v3 01/10] clk: Add Kunit tests for rate

[Daniel Latypov <dlatypov@google.com>]

On Thu, Jan 20, 2022 at 1:31 PM Stephen Boyd <sboyd@kernel.org> wrote:
> I was thinking this would be more generic so that one file tests clk.c
> and all the code in there, but I guess there may be config dependencies
> like CONFIG_OF that we may want to extract out and depend on
> differently. I'm not sure how kunit will handle testing different paths
> depending on build configuration so this approach may head off future
> problems. If it doesn't then we can always slam the test together.

KUnit doesn't have hard technical limitations in this regard.

You could have something like this

static void my_optional_kunit_test(struct kunit *test)
{
#ifdef CONFIG_OPTIONAL_FEATURE

# else
  kunit_skip(test, "CONFIG_OPTIONAL_FEATURE is not enabled");
#endif
}

I think it's just a matter of what's least confusing to users.

> > +}
> > +
> > +/*
> > + * Test that the actual rate matches what is returned by clk_get_rate()
> > + */
> > +static void clk_rate_test_get_rate(struct kunit *test)
> > +{
> > +       struct clk_dummy_rate_context *ctx = test->priv;
> > +       struct clk_hw *hw = &ctx->hw;
> > +       struct clk *clk = hw->clk;
> > +       unsigned long rate;
> > +
> > +       rate = clk_get_rate(clk);
> > +       KUNIT_ASSERT_TRUE(test, rate > 0);

KUNIT_EXPECT_GT(test, rate, 0);

> > +       KUNIT_ASSERT_EQ(test, rate, ctx->rate);
>
> These should be KUNIT_EXPECT_*() as we don't want to stop the test if
> the rate is wrong, we want to check that the rate is what we expected it
> to be. Assertions are about making sure things are sane and if not we
> should stop testing, whereas expectations are about testing the code. A
> test must have an EXPECT while it can have an ASSERT.
>
> Maybe kunit should check that there was an EXPECT on return from the
> test. Daniel?

Sorry, I'm not sure I understand the question.

Are you saying you want kunit to flag cases like
  static void empty_test(struct kunit *) {}
?