Re: [PATCH v2 1/2] drm/cmdline-parser: Merge negative tests

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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),
>>>   	{}
>>>   };



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux