Re: [PATCH i-g-t v2] tests/kms_3d: Add more logs to skips/failure

["Modem, Bhanuprakash" <bhanuprakash.modem@intel.com>]

Hi Pranay,

On 28-08-2024 10:03 am, Pranay Samala wrote:
> Having some logs for test failures and skips would
> make debugging much easier.
> 
> Signed-off-by: Pranay Samala <pranay.samala@intel.com>
> ---
>   tests/kms_3d.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/kms_3d.c b/tests/kms_3d.c
> index ad6c45e9b..66c2ddfc1 100644
> --- a/tests/kms_3d.c
> +++ b/tests/kms_3d.c
> @@ -55,7 +55,8 @@ igt_simple_main
>   	res = drmModeGetResources(drm_fd);
>   	igt_require(res);
>   
> -	igt_assert(drmSetClientCap(drm_fd, DRM_CLIENT_CAP_STEREO_3D, 1) >= 0);
> +	igt_assert_f(drmSetClientCap(drm_fd, DRM_CLIENT_CAP_STEREO_3D, 1) >= 0,
> +		     "Failed to enable STEREO_3D capability\n");

I can feel this log is not required, but still it's upto you.

>   
>   	/* find an hdmi connector */
>   	for (int i = 0; i < res->count_connectors; i++) {
> @@ -65,7 +66,7 @@ igt_simple_main
>   		drmModeFreeConnector(connector);
>   		connector = NULL;
>   	}
> -	igt_require(connector);
> +	igt_require_f(connector, "Unable to get HDMI Connector\n");

This message is misleading like "system is unable to get the HDMI 
connector event though there is one connected". Please update this log 
something like "NO HDMI connector found".

>   
>   	kmstest_unset_all_crtcs(drm_fd, res);
>   
> @@ -85,7 +86,7 @@ igt_simple_main
>   			mode_count++;
>   	}
>   
> -	igt_assert(mode_count);
> +	igt_assert_f(mode_count, "3D modes not detected\n");
-------------------------------------------------------^
Please add a period '.' at the end of every sentence.

>   
>   	/* set 3D modes */
>   	igt_info("Testing:\n");
> @@ -95,8 +96,10 @@ igt_simple_main
>   		int crtc_mask = -1;
>   		int ret;
>   
> -		if (!(connector->modes[i].flags & DRM_MODE_FLAG_3D_MASK))
> +		if (!(connector->modes[i].flags & DRM_MODE_FLAG_3D_MASK)) {
> +			igt_info("%i: Doesn't support 3D MASK\n", res->connectors[i]);
---------------------------------^
Adding the connector name to the log is more user friendly.
igt_info("%s-%d: <your msg>\n", connector_type, connector_type_id);

- Bhanu

>   			continue;
> +		}
>   
>   		/* create a configuration */
>   		ret = kmstest_get_connector_config(drm_fd, connector_id,