On Thu, Aug 18, 2022 at 11:15:39AM -0300, Maíra Canal wrote:
>
>
> On 8/17/22 18:12, Michał Winiarski wrote:
> > Negative tests can be expressed as a single parameterized test case,
> > which highlights that we're following the same test logic (passing
> > invalid cmdline and expecting drm_mode_parse_command_line_for_connector
> > to fail), which improves readability.
> >
>
> In order to be consistent to the change on the testcases, you could
> s/negative/invalid on the commit message also.
Already did that - s/passing negative cmdline/passing invalid cmdline.
The tests are still "negative tests" - in other words, tests that pass invalid
data, and expect specific error condition to happen. We can't use "invalid
tests" here, as that has different meaning (broken test).
We could expand it into:
"Tests that pass invalid data can be expressed as a single parameterized test
case (...)"
Would that work? Or should we keep it as "negative tests"?
-Michał
>
> Best Regards,
> - Maíra Canal
>
> > v2: s/negative/invalid to be consistent with other testcases in DRM
> >
> > Signed-off-by: Michał Winiarski <michal.winiarski@xxxxxxxxx>
> > Tested-by: Maíra Canal <mairacanal@xxxxxxxxxx>
> > ---
> > .../gpu/drm/tests/drm_cmdline_parser_test.c | 293 ++++++------------
> > 1 file changed, 103 insertions(+), 190 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/tests/drm_cmdline_parser_test.c b/drivers/gpu/drm/tests/drm_cmdline_parser_test.c
> > index 59b29cdfdd35..3a46c7d6f2aa 100644
> > --- a/drivers/gpu/drm/tests/drm_cmdline_parser_test.c
> > +++ b/drivers/gpu/drm/tests/drm_cmdline_parser_test.c
> > @@ -109,24 +109,6 @@ static void drm_cmdline_test_force_d_only(struct kunit *test)
> > KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_OFF);
> > }
> > -static void drm_cmdline_test_margin_only(struct kunit *test)
> > -{
> > - struct drm_cmdline_mode mode = { };
> > - const char *cmdline = "m";
> > -
> > - KUNIT_EXPECT_FALSE(test, drm_mode_parse_command_line_for_connector(cmdline,
> > - &no_connector, &mode));
> > -}
> > -
> > -static void drm_cmdline_test_interlace_only(struct kunit *test)
> > -{
> > - struct drm_cmdline_mode mode = { };
> > - const char *cmdline = "i";
> > -
> > - KUNIT_EXPECT_FALSE(test, drm_mode_parse_command_line_for_connector(cmdline,
> > - &no_connector, &mode));
> > -}
> > -
> > static void drm_cmdline_test_res(struct kunit *test)
> > {
> > struct drm_cmdline_mode mode = { };
> > @@ -149,42 +131,6 @@ static void drm_cmdline_test_res(struct kunit *test)
> > KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
> > }
> > -static void drm_cmdline_test_res_missing_x(struct kunit *test)
> > -{
> > - struct drm_cmdline_mode mode = { };
> > - const char *cmdline = "x480";
> > -
> > - KUNIT_EXPECT_FALSE(test, drm_mode_parse_command_line_for_connector(cmdline,
> > - &no_connector, &mode));
> > -}
> > -
> > -static void drm_cmdline_test_res_missing_y(struct kunit *test)
> > -{
> > - struct drm_cmdline_mode mode = { };
> > - const char *cmdline = "1024x";
> > -
> > - KUNIT_EXPECT_FALSE(test, drm_mode_parse_command_line_for_connector(cmdline,
> > - &no_connector, &mode));
> > -}
> > -
> > -static void drm_cmdline_test_res_bad_y(struct kunit *test)
> > -{
> > - struct drm_cmdline_mode mode = { };
> > - const char *cmdline = "1024xtest";
> > -
> > - KUNIT_EXPECT_FALSE(test, drm_mode_parse_command_line_for_connector(cmdline,
> > - &no_connector, &mode));
> > -}
> > -
> > -static void drm_cmdline_test_res_missing_y_bpp(struct kunit *test)
> > -{
> > - struct drm_cmdline_mode mode = { };
> > - const char *cmdline = "1024x-24";
> > -
> > - KUNIT_EXPECT_FALSE(test, drm_mode_parse_command_line_for_connector(cmdline,
> > - &no_connector, &mode));
> > -}
> > -
> > static void drm_cmdline_test_res_vesa(struct kunit *test)
> > {
> > struct drm_cmdline_mode mode = { };
> > @@ -274,15 +220,6 @@ static void drm_cmdline_test_res_bpp(struct kunit *test)
> > KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
> > }
> > -static void drm_cmdline_test_res_bad_bpp(struct kunit *test)
> > -{
> > - struct drm_cmdline_mode mode = { };
> > - const char *cmdline = "720x480-test";
> > -
> > - KUNIT_EXPECT_FALSE(test, drm_mode_parse_command_line_for_connector(cmdline,
> > - &no_connector, &mode));
> > -}
> > -
> > static void drm_cmdline_test_res_refresh(struct kunit *test)
> > {
> > struct drm_cmdline_mode mode = { };
> > @@ -306,15 +243,6 @@ static void drm_cmdline_test_res_refresh(struct kunit *test)
> > KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
> > }
> > -static void drm_cmdline_test_res_bad_refresh(struct kunit *test)
> > -{
> > - struct drm_cmdline_mode mode = { };
> > - const char *cmdline = "720x480@refresh";
> > -
> > - KUNIT_EXPECT_FALSE(test, drm_mode_parse_command_line_for_connector(cmdline,
> > - &no_connector, &mode));
> > -}
> > -
> > static void drm_cmdline_test_res_bpp_refresh(struct kunit *test)
> > {
> > struct drm_cmdline_mode mode = { };
> > @@ -411,15 +339,6 @@ static void drm_cmdline_test_res_bpp_refresh_force_off(struct kunit *test)
> > KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_OFF);
> > }
> > -static void drm_cmdline_test_res_bpp_refresh_force_on_off(struct kunit *test)
> > -{
> > - struct drm_cmdline_mode mode = { };
> > - const char *cmdline = "720x480-24@60de";
> > -
> > - KUNIT_EXPECT_FALSE(test, drm_mode_parse_command_line_for_connector(cmdline,
> > - &no_connector, &mode));
> > -}
> > -
> > static void drm_cmdline_test_res_bpp_refresh_force_on(struct kunit *test)
> > {
> > struct drm_cmdline_mode mode = { };
> > @@ -563,24 +482,6 @@ static void drm_cmdline_test_res_vesa_margins(struct kunit *test)
> > KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
> > }
> > -static void drm_cmdline_test_res_invalid_mode(struct kunit *test)
> > -{
> > - struct drm_cmdline_mode mode = { };
> > - const char *cmdline = "720x480f";
> > -
> > - KUNIT_EXPECT_FALSE(test, drm_mode_parse_command_line_for_connector(cmdline,
> > - &no_connector, &mode));
> > -}
> > -
> > -static void drm_cmdline_test_res_bpp_wrong_place_mode(struct kunit *test)
> > -{
> > - struct drm_cmdline_mode mode = { };
> > - const char *cmdline = "720x480e-24";
> > -
> > - KUNIT_EXPECT_FALSE(test, drm_mode_parse_command_line_for_connector(cmdline,
> > - &no_connector, &mode));
> > -}
> > -
> > static void drm_cmdline_test_name(struct kunit *test)
> > {
> > struct drm_cmdline_mode mode = { };
> > @@ -608,42 +509,6 @@ static void drm_cmdline_test_name_bpp(struct kunit *test)
> > KUNIT_EXPECT_EQ(test, mode.bpp, 24);
> > }
> > -static void drm_cmdline_test_name_bpp_refresh(struct kunit *test)
> > -{
> > - struct drm_cmdline_mode mode = { };
> > - const char *cmdline = "NTSC-24@60";
> > -
> > - KUNIT_EXPECT_FALSE(test, drm_mode_parse_command_line_for_connector(cmdline,
> > - &no_connector, &mode));
> > -}
> > -
> > -static void drm_cmdline_test_name_refresh(struct kunit *test)
> > -{
> > - struct drm_cmdline_mode mode = { };
> > - const char *cmdline = "NTSC@60";
> > -
> > - KUNIT_EXPECT_FALSE(test, drm_mode_parse_command_line_for_connector(cmdline,
> > - &no_connector, &mode));
> > -}
> > -
> > -static void drm_cmdline_test_name_refresh_wrong_mode(struct kunit *test)
> > -{
> > - struct drm_cmdline_mode mode = { };
> > - const char *cmdline = "NTSC@60m";
> > -
> > - KUNIT_EXPECT_FALSE(test, drm_mode_parse_command_line_for_connector(cmdline,
> > - &no_connector, &mode));
> > -}
> > -
> > -static void drm_cmdline_test_name_refresh_invalid_mode(struct kunit *test)
> > -{
> > - struct drm_cmdline_mode mode = { };
> > - const char *cmdline = "NTSC@60f";
> > -
> > - KUNIT_EXPECT_FALSE(test, drm_mode_parse_command_line_for_connector(cmdline,
> > - &no_connector, &mode));
> > -}
> > -
> > static void drm_cmdline_test_name_option(struct kunit *test)
> > {
> > struct drm_cmdline_mode mode = { };
> > @@ -762,33 +627,6 @@ static void drm_cmdline_test_rotate_270(struct kunit *test)
> > KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
> > }
> > -static void drm_cmdline_test_rotate_multiple(struct kunit *test)
> > -{
> > - struct drm_cmdline_mode mode = { };
> > - const char *cmdline = "720x480,rotate=0,rotate=90";
> > -
> > - KUNIT_EXPECT_FALSE(test, drm_mode_parse_command_line_for_connector(cmdline,
> > - &no_connector, &mode));
> > -}
> > -
> > -static void drm_cmdline_test_rotate_invalid_val(struct kunit *test)
> > -{
> > - struct drm_cmdline_mode mode = { };
> > - const char *cmdline = "720x480,rotate=42";
> > -
> > - KUNIT_EXPECT_FALSE(test, drm_mode_parse_command_line_for_connector(cmdline,
> > - &no_connector, &mode));
> > -}
> > -
> > -static void drm_cmdline_test_rotate_truncated(struct kunit *test)
> > -{
> > - struct drm_cmdline_mode mode = { };
> > - const char *cmdline = "720x480,rotate=";
> > -
> > - KUNIT_EXPECT_FALSE(test, drm_mode_parse_command_line_for_connector(cmdline,
> > - &no_connector, &mode));
> > -}
> > -
> > static void drm_cmdline_test_hmirror(struct kunit *test)
> > {
> > struct drm_cmdline_mode mode = { };
> > @@ -885,15 +723,6 @@ static void drm_cmdline_test_multiple_options(struct kunit *test)
> > KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
> > }
> > -static void drm_cmdline_test_invalid_option(struct kunit *test)
> > -{
> > - struct drm_cmdline_mode mode = { };
> > - const char *cmdline = "720x480,test=42";
> > -
> > - KUNIT_EXPECT_FALSE(test, drm_mode_parse_command_line_for_connector(cmdline,
> > - &no_connector, &mode));
> > -}
> > -
> > static void drm_cmdline_test_bpp_extra_and_option(struct kunit *test)
> > {
> > struct drm_cmdline_mode mode = { };
> > @@ -1006,64 +835,148 @@ static void drm_cmdline_test_panel_orientation(struct kunit *test)
> > KUNIT_EXPECT_EQ(test, mode.force, DRM_FORCE_UNSPECIFIED);
> > }
> > +struct drm_cmdline_invalid_test {
> > + const char *name;
> > + const char *cmdline;
> > +};
> > +
> > +static void drm_cmdline_test_invalid(struct kunit *test)
> > +{
> > + const struct drm_cmdline_invalid_test *params = test->param_value;
> > + struct drm_cmdline_mode mode = { };
> > +
> > + KUNIT_EXPECT_FALSE(test, drm_mode_parse_command_line_for_connector(params->cmdline,
> > + &no_connector,
> > + &mode));
> > +}
> > +
> > +static const struct drm_cmdline_invalid_test drm_cmdline_invalid_tests[] = {
> > + {
> > + .name = "margin_only",
> > + .cmdline = "m",
> > + },
> > + {
> > + .name = "interlace_only",
> > + .cmdline = "i",
> > + },
> > + {
> > + .name = "res_missing_x",
> > + .cmdline = "x480",
> > + },
> > + {
> > + .name = "res_missing_y",
> > + .cmdline = "1024x",
> > + },
> > + {
> > + .name = "res_bad_y",
> > + .cmdline = "1024xtest",
> > + },
> > + {
> > + .name = "res_missing_y_bpp",
> > + .cmdline = "1024x-24",
> > + },
> > + {
> > + .name = "res_bad_bpp",
> > + .cmdline = "720x480-test",
> > + },
> > + {
> > + .name = "res_bad_refresh",
> > + .cmdline = "720x480@refresh",
> > + },
> > + {
> > + .name = "res_bpp_refresh_force_on_off",
> > + .cmdline = "720x480-24@60de",
> > + },
> > + {
> > + .name = "res_invalid_mode",
> > + .cmdline = "720x480f",
> > + },
> > + {
> > + .name = "res_bpp_wrong_place_mode",
> > + .cmdline = "720x480e-24",
> > + },
> > + {
> > + .name = "name_bpp_refresh",
> > + .cmdline = "NTSC-24@60",
> > + },
> > + {
> > + .name = "name_refresh",
> > + .cmdline = "NTSC@60",
> > + },
> > + {
> > + .name = "name_refresh_wrong_mode",
> > + .cmdline = "NTSC@60m",
> > + },
> > + {
> > + .name = "name_refresh_invalid_mode",
> > + .cmdline = "NTSC@60f",
> > + },
> > + {
> > + .name = "rotate_multiple",
> > + .cmdline = "720x480,rotate=0,rotate=90",
> > + },
> > + {
> > + .name = "rotate_invalid_val",
> > + .cmdline = "720x480,rotate=42",
> > + },
> > + {
> > + .name = "rotate_truncated",
> > + .cmdline = "720x480,rotate=",
> > + },
> > + {
> > + .name = "invalid_option",
> > + .cmdline = "720x480,test=42",
> > + },
> > +};
> > +
> > +static void drm_cmdline_invalid_desc(const struct drm_cmdline_invalid_test *t,
> > + char *desc)
> > +{
> > + sprintf(desc, "%s", t->name);
> > +}
> > +
> > +KUNIT_ARRAY_PARAM(drm_cmdline_invalid, drm_cmdline_invalid_tests, drm_cmdline_invalid_desc);
> > +
> > static struct kunit_case drm_cmdline_parser_tests[] = {
> > KUNIT_CASE(drm_cmdline_test_force_d_only),
> > KUNIT_CASE(drm_cmdline_test_force_D_only_dvi),
> > KUNIT_CASE(drm_cmdline_test_force_D_only_hdmi),
> > KUNIT_CASE(drm_cmdline_test_force_D_only_not_digital),
> > KUNIT_CASE(drm_cmdline_test_force_e_only),
> > - KUNIT_CASE(drm_cmdline_test_margin_only),
> > - KUNIT_CASE(drm_cmdline_test_interlace_only),
> > KUNIT_CASE(drm_cmdline_test_res),
> > - KUNIT_CASE(drm_cmdline_test_res_missing_x),
> > - KUNIT_CASE(drm_cmdline_test_res_missing_y),
> > - KUNIT_CASE(drm_cmdline_test_res_bad_y),
> > - KUNIT_CASE(drm_cmdline_test_res_missing_y_bpp),
> > KUNIT_CASE(drm_cmdline_test_res_vesa),
> > KUNIT_CASE(drm_cmdline_test_res_vesa_rblank),
> > KUNIT_CASE(drm_cmdline_test_res_rblank),
> > KUNIT_CASE(drm_cmdline_test_res_bpp),
> > - KUNIT_CASE(drm_cmdline_test_res_bad_bpp),
> > KUNIT_CASE(drm_cmdline_test_res_refresh),
> > - KUNIT_CASE(drm_cmdline_test_res_bad_refresh),
> > KUNIT_CASE(drm_cmdline_test_res_bpp_refresh),
> > KUNIT_CASE(drm_cmdline_test_res_bpp_refresh_interlaced),
> > KUNIT_CASE(drm_cmdline_test_res_bpp_refresh_margins),
> > KUNIT_CASE(drm_cmdline_test_res_bpp_refresh_force_off),
> > - KUNIT_CASE(drm_cmdline_test_res_bpp_refresh_force_on_off),
> > KUNIT_CASE(drm_cmdline_test_res_bpp_refresh_force_on),
> > KUNIT_CASE(drm_cmdline_test_res_bpp_refresh_force_on_analog),
> > KUNIT_CASE(drm_cmdline_test_res_bpp_refresh_force_on_digital),
> > KUNIT_CASE(drm_cmdline_test_res_bpp_refresh_interlaced_margins_force_on),
> > KUNIT_CASE(drm_cmdline_test_res_margins_force_on),
> > KUNIT_CASE(drm_cmdline_test_res_vesa_margins),
> > - KUNIT_CASE(drm_cmdline_test_res_invalid_mode),
> > - KUNIT_CASE(drm_cmdline_test_res_bpp_wrong_place_mode),
> > KUNIT_CASE(drm_cmdline_test_name),
> > KUNIT_CASE(drm_cmdline_test_name_bpp),
> > - KUNIT_CASE(drm_cmdline_test_name_refresh),
> > - KUNIT_CASE(drm_cmdline_test_name_bpp_refresh),
> > - KUNIT_CASE(drm_cmdline_test_name_refresh_wrong_mode),
> > - KUNIT_CASE(drm_cmdline_test_name_refresh_invalid_mode),
> > KUNIT_CASE(drm_cmdline_test_name_option),
> > KUNIT_CASE(drm_cmdline_test_name_bpp_option),
> > KUNIT_CASE(drm_cmdline_test_rotate_0),
> > KUNIT_CASE(drm_cmdline_test_rotate_90),
> > KUNIT_CASE(drm_cmdline_test_rotate_180),
> > KUNIT_CASE(drm_cmdline_test_rotate_270),
> > - KUNIT_CASE(drm_cmdline_test_rotate_multiple),
> > - KUNIT_CASE(drm_cmdline_test_rotate_invalid_val),
> > - KUNIT_CASE(drm_cmdline_test_rotate_truncated),
> > KUNIT_CASE(drm_cmdline_test_hmirror),
> > KUNIT_CASE(drm_cmdline_test_vmirror),
> > KUNIT_CASE(drm_cmdline_test_margin_options),
> > KUNIT_CASE(drm_cmdline_test_multiple_options),
> > - KUNIT_CASE(drm_cmdline_test_invalid_option),
> > KUNIT_CASE(drm_cmdline_test_bpp_extra_and_option),
> > KUNIT_CASE(drm_cmdline_test_extra_and_option),
> > KUNIT_CASE(drm_cmdline_test_freestanding_options),
> > KUNIT_CASE(drm_cmdline_test_freestanding_force_e_and_options),
> > KUNIT_CASE(drm_cmdline_test_panel_orientation),
> > + KUNIT_CASE_PARAM(drm_cmdline_test_invalid, drm_cmdline_invalid_gen_params),
> > {}
> > };
