[PATCH v2] drm/tests: Add KUnit tests for drm_mode_create_dvi_i_properties()

[Dipam Turkar]

I needed some help regarding introducing a separate test for testing if the
function still works if called a second time as suggested. Wouldn't we need
to call it on the same object we called in the first time. So, that will
bring redundancy in the two functions. Is this correct? Or am I
understanding this wrong?

Thanks and regards
Dipam Turkar

On Tue, Nov 28, 2023 at 8:39 PM Maxime Ripard <mripard at kernel.org> wrote:

> Hi,
>
> On Sat, Nov 11, 2023 at 12:54:53AM +0530, Dipam Turkar wrote:
> > Introduce unit tests for the drm_mode_create_dvi_i_properties() function
> to ensure
> > the proper creation of DVI-I specific connector properties.
> >
> > Signed-off-by: Dipam Turkar <dipamt1729 at gmail.com>
> > ---
> >  drivers/gpu/drm/tests/drm_connector_test.c | 38 ++++++++++++++++++++++
> >  1 file changed, 38 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/tests/drm_connector_test.c
> b/drivers/gpu/drm/tests/drm_connector_test.c
> > index c66aa2dc8d9d..9ac1fd32c579 100644
> > --- a/drivers/gpu/drm/tests/drm_connector_test.c
> > +++ b/drivers/gpu/drm/tests/drm_connector_test.c
> > @@ -4,6 +4,9 @@
> >   */
> >
> >  #include <drm/drm_connector.h>
> > +#include <drm/drm_device.h>
> > +#include <drm/drm_drv.h>
> > +#include <drm/drm_kunit_helpers.h>
> >
> >  #include <kunit/test.h>
> >
> > @@ -58,6 +61,30 @@ static void
> drm_test_get_tv_mode_from_name_truncated(struct kunit *test)
> >       KUNIT_EXPECT_LT(test, ret, 0);
> >  };
> >
> > +/*
> > + * Test that drm_mode_create_dvi_i_properties() succeeds and
> > + * DVI-I subconnector and select subconectors properties have
> > + * been created.
> > + */
> > +static void drm_test_mode_create_dvi_i_properties(struct kunit *test)
> > +{
> > +     struct drm_device *drm;
> > +     struct device *dev;
> > +
> > +     dev = drm_kunit_helper_alloc_device(test);
> > +     KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev);
> > +
> > +     drm = __drm_kunit_helper_alloc_drm_device(test, dev, sizeof(*drm),
> 0, DRIVER_MODESET);
> > +     KUNIT_ASSERT_NOT_ERR_OR_NULL(test, drm);
> > +
> > +     KUNIT_EXPECT_EQ(test, drm_mode_create_dvi_i_properties(drm), 0);
> > +     KUNIT_EXPECT_NOT_ERR_OR_NULL(test,
> drm->mode_config.dvi_i_select_subconnector_property);
> > +     KUNIT_EXPECT_NOT_ERR_OR_NULL(test,
> drm->mode_config.dvi_i_subconnector_property);
> > +
> > +     // Expect the function to return 0 if called twice.
>
> This is not the proper comment format
>
> > +     KUNIT_EXPECT_EQ(test, drm_mode_create_dvi_i_properties(drm), 0);
>
> This should be in a separate test, with a separate description. We want
> to test two things: that the function works well, and that the function
> still works if we call it a second time.
>
> > +}
> > +
> >  static struct kunit_case drm_get_tv_mode_from_name_tests[] = {
> >       KUNIT_CASE_PARAM(drm_test_get_tv_mode_from_name_valid,
> >                        drm_get_tv_mode_from_name_valid_gen_params),
> > @@ -70,7 +97,18 @@ static struct kunit_suite
> drm_get_tv_mode_from_name_test_suite = {
> >       .test_cases = drm_get_tv_mode_from_name_tests,
> >  };
>
> The test should be next to the test suite definition
>
> > +static struct kunit_case drm_connector_tests[] = {
> > +     KUNIT_CASE(drm_test_mode_create_dvi_i_properties),
> > +     { }
> > +};
> > +
> > +static struct kunit_suite drm_connector_test_suite = {
> > +     .name = "drm_connector",
>
> That's too generic, the test suite is only about
> drm_mode_create_dvi_i_properties(), not drm_connector in general.
>
> > +     .test_cases = drm_connector_tests,
> > +};
> > +
> >  kunit_test_suite(drm_get_tv_mode_from_name_test_suite);
> > +kunit_test_suite(drm_connector_test_suite);
>
> kunit_test_suites
>
> Maxime
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20231223/6fa31b24/attachment.htm>