On Tue, Jun 21, 2022 at 1:10 PM Maíra Canal maira.canal@usp.br wrote:
Considering the current adoption of the KUnit framework, convert the DRM format selftest to the KUnit API.
Tested-by: David Gow davidgow@google.com Signed-off-by: Maíra Canal maira.canal@usp.br
Acked-by: Daniel Latypov dlatypov@google.com
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.