On Tue, Jun 21, 2022 at 1:10 PM Maíra Canal <maira.canal@xxxxxx> wrote:
>
> Considering the current adoption of the KUnit framework, convert the
> DRM format selftest to the KUnit API.
>
> Tested-by: David Gow <davidgow@xxxxxxxxxx>
> Signed-off-by: Maíra Canal <maira.canal@xxxxxx>
Acked-by: Daniel Latypov <dlatypov@xxxxxxxxxx>
Overall looks good from the KUnit side, just a few general suggestions below.
FYI, the warning email from kernel-test-robot is basically saying that
the compiler is not optimizing away the temporary variables internally
created in KUNIT_EXPECT_*.
So having too many KUNIT_EXPECT_.* in a single function is the trigger.
The main workaround you'd have is to split up the test into more test functions.
(I don't know if that's actually worth doing)
> +static void igt_check_drm_format_block_width(struct kunit *test)
> +{
> + const struct drm_format_info *info = NULL;
> +
> + /* Test invalid arguments */
> + KUNIT_EXPECT_FALSE(test, drm_format_info_block_width(info, 0));
> + KUNIT_EXPECT_FALSE(test, drm_format_info_block_width(info, -1));
> + KUNIT_EXPECT_FALSE(test, drm_format_info_block_width(info, 1));
Hmm, I think one of these two would be clearer here:
KUNIT_EXPECT_EQ(test, drm_format_info_block_width(info, 0), 0);
KUNIT_EXPECT_EQ(test, 0, drm_format_info_block_width(info, 0));
I think this helps test readability (giving hints about the types) and
gives better error messages, more on that below.
The problem with using the boolean expectations is that given
int foo = 2;
KUNIT_EXPECT_FALSE(test, foo);
KUnit will only print out
Expected foo to be false, but is true
Using EXPECT_EQ(foo, 0), we'd get
Expected foo == 0, but
foo == 2
Knowing exactly what the offending return value was can help debug
test failures a bit faster.
> +
> + /* Test 1 plane format */
> + info = drm_format_info(DRM_FORMAT_XRGB4444);
> + KUNIT_EXPECT_TRUE(test, info);
FYI, you can now instead write
KUNIT_EXPECT_NOT_NULL(test, info);
this new macro was merged into 5.19-rc1.
