On 8/18/22 13:19, Michał Winiarski wrote:
> 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"?
As Latypov pointed out, maybe "negative tests" are easier to understand.
So I guess you can keep it as it is.
Reviewed-by: Maíra Canal <mairacanal@xxxxxxxxxx>
Best Regards,
- Maíra Canal
>
> -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),
>>> {}
>>> };
