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@xxxxxxxxxx> 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@xxxxxxxxx>
> ---
> 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
