| Message ID | 20231024191002.1620-7-gcarlos@disroot.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Increase coverage on drm_framebuffer.c | expand |
Hi, On Tue, Oct 24, 2023 at 04:09:57PM -0300, Carlos Eduardo Gallo Filho wrote: > Add a single KUnit test case for the drm_framebuffer_cleanup function. > > Signed-off-by: Carlos Eduardo Gallo Filho <gcarlos@disroot.org> > --- > v2: > - Reorder kunit cases alphabetically. > - Rely on drm_kunit_helper_alloc_device() for mock initialization. > --- > drivers/gpu/drm/tests/drm_framebuffer_test.c | 37 ++++++++++++++++++++ > 1 file changed, 37 insertions(+) > > diff --git a/drivers/gpu/drm/tests/drm_framebuffer_test.c b/drivers/gpu/drm/tests/drm_framebuffer_test.c > index 7b51862fb0f2..a63f30985f75 100644 > --- a/drivers/gpu/drm/tests/drm_framebuffer_test.c > +++ b/drivers/gpu/drm/tests/drm_framebuffer_test.c > @@ -490,8 +490,45 @@ check_src_coords_test_to_desc(const struct drm_framebuffer_check_src_coords_case > KUNIT_ARRAY_PARAM(check_src_coords, drm_framebuffer_check_src_coords_cases, > check_src_coords_test_to_desc); > > +static void drm_test_framebuffer_cleanup(struct kunit *test) Again, we should document *what* we are testing here. Your commit says that you're testing drm_framebuffer_cleanup(), but even after reading that test I have no idea which tests have been covered or not. > +{ > + struct drm_framebuffer_test_priv *priv = test->priv; > + struct drm_device *dev = &priv->dev; > + struct list_head *fb_list = &dev->mode_config.fb_list; > + struct drm_framebuffer fb1 = { .dev = dev }; > + struct drm_framebuffer fb2 = { .dev = dev }; > + > + /* This must result on [fb_list] -> fb1 -> fb2 */ > + list_add_tail(&fb1.head, fb_list); > + list_add_tail(&fb2.head, fb_list); > + dev->mode_config.num_fb = 2; > + > + KUNIT_ASSERT_PTR_EQ(test, fb_list->prev, &fb2.head); > + KUNIT_ASSERT_PTR_EQ(test, fb_list->next, &fb1.head); > + KUNIT_ASSERT_PTR_EQ(test, fb1.head.prev, fb_list); > + KUNIT_ASSERT_PTR_EQ(test, fb1.head.next, &fb2.head); > + KUNIT_ASSERT_PTR_EQ(test, fb2.head.prev, &fb1.head); > + KUNIT_ASSERT_PTR_EQ(test, fb2.head.next, fb_list); > + > + drm_framebuffer_cleanup(&fb1); I think that for that test to be meaningful, we should first have init the framebuffer using drm_framebuffer_init. Otherwise we will just chasing our tail trying to mock drm_framebuffer_init. Plus, we don't want to test if drm_framebuffer_cleanup works if we hand-crafted a framebuffer. Maxime
diff --git a/drivers/gpu/drm/tests/drm_framebuffer_test.c b/drivers/gpu/drm/tests/drm_framebuffer_test.c index 7b51862fb0f2..a63f30985f75 100644 --- a/drivers/gpu/drm/tests/drm_framebuffer_test.c +++ b/drivers/gpu/drm/tests/drm_framebuffer_test.c @@ -490,8 +490,45 @@ check_src_coords_test_to_desc(const struct drm_framebuffer_check_src_coords_case KUNIT_ARRAY_PARAM(check_src_coords, drm_framebuffer_check_src_coords_cases, check_src_coords_test_to_desc); +static void drm_test_framebuffer_cleanup(struct kunit *test) +{ + struct drm_framebuffer_test_priv *priv = test->priv; + struct drm_device *dev = &priv->dev; + struct list_head *fb_list = &dev->mode_config.fb_list; + struct drm_framebuffer fb1 = { .dev = dev }; + struct drm_framebuffer fb2 = { .dev = dev }; + + /* This must result on [fb_list] -> fb1 -> fb2 */ + list_add_tail(&fb1.head, fb_list); + list_add_tail(&fb2.head, fb_list); + dev->mode_config.num_fb = 2; + + KUNIT_ASSERT_PTR_EQ(test, fb_list->prev, &fb2.head); + KUNIT_ASSERT_PTR_EQ(test, fb_list->next, &fb1.head); + KUNIT_ASSERT_PTR_EQ(test, fb1.head.prev, fb_list); + KUNIT_ASSERT_PTR_EQ(test, fb1.head.next, &fb2.head); + KUNIT_ASSERT_PTR_EQ(test, fb2.head.prev, &fb1.head); + KUNIT_ASSERT_PTR_EQ(test, fb2.head.next, fb_list); + + drm_framebuffer_cleanup(&fb1); + + /* Now [fb_list] -> fb2 */ + KUNIT_ASSERT_PTR_EQ(test, fb_list->prev, &fb2.head); + KUNIT_ASSERT_PTR_EQ(test, fb_list->next, &fb2.head); + KUNIT_ASSERT_PTR_EQ(test, fb2.head.prev, fb_list); + KUNIT_ASSERT_PTR_EQ(test, fb2.head.next, fb_list); + KUNIT_ASSERT_EQ(test, dev->mode_config.num_fb, 1); + + drm_framebuffer_cleanup(&fb2); + + /* Now fb_list is empty */ + KUNIT_ASSERT_TRUE(test, list_empty(fb_list)); + KUNIT_ASSERT_EQ(test, dev->mode_config.num_fb, 0); +} + static struct kunit_case drm_framebuffer_tests[] = { KUNIT_CASE_PARAM(drm_test_framebuffer_check_src_coords, check_src_coords_gen_params), + KUNIT_CASE(drm_test_framebuffer_cleanup), KUNIT_CASE_PARAM(drm_test_framebuffer_create, drm_framebuffer_create_gen_params), KUNIT_CASE(drm_test_framebuffer_modifiers_not_supported), { }
Add a single KUnit test case for the drm_framebuffer_cleanup function. Signed-off-by: Carlos Eduardo Gallo Filho <gcarlos@disroot.org> --- v2: - Reorder kunit cases alphabetically. - Rely on drm_kunit_helper_alloc_device() for mock initialization. --- drivers/gpu/drm/tests/drm_framebuffer_test.c | 37 ++++++++++++++++++++ 1 file changed, 37 insertions(+)