[igt-dev] [PATCH i-g-t 3/3] lib/amdgpu:fix the error when skip hotplug test

[Kamil Konieczny]

Hi Jesse,

one small nit about subject, put space after ':',

[PATCH i-g-t 3/3] lib/amdgpu:fix the error when skip hotplug test
----------------------------^^
s/amdgpu:fix/amdgpu: fix/

should be:
[PATCH i-g-t 3/3] lib/amdgpu: fix the error when skip hotplug test

On 2023-11-09 at 10:42:29 +0800, Jesse Zhang wrote:
> For skipped cases, we should print "SKIP"
> rather than "failure." For example
> 
> IGT-Version: 1.28-g4b3e1dc37 (x86_64) (Linux: 6.3.7+ x86_64)
> Starting subtest: amdgpu_hotunplug_simple
> SKIP ... more than 1 GPU is required for this test
> (amd_pci_unplug:70496) amdgpu/amd_pci_unplug-CRITICAL: Test assertion failure function amdgpu_hotunplug_test, file ../lib/amdgpu/amd_pci_unplug.c:305:
> (amd_pci_unplug:70496) amdgpu/amd_pci_unplug-CRITICAL: Failed assertion: r == 1
> (amd_pci_unplug:70496) amdgpu/amd_pci_unplug-CRITICAL: Last errno: 22, Invalid argument
> (amd_pci_unplug:70496) amdgpu/amd_pci_unplug-CRITICAL: error: 0 != 1
> Stack trace:
>   #0 ../lib/igt_core.c:1988 __igt_fail_assert()
>   #1 ../lib/amdgpu/amd_pci_unplug.c:305 amdgpu_hotunplug_test.constprop.0.isra.0()
--------- ^^^ -------------------------------------------- ^^^^

Did you write test inside library? Is it called from other tests?

Regards,
Kamil

>   #2 [amdgpu_hotunplug_simple+0x29]
>   #3 ../tests/amdgpu/amd_pci_unplug.c:44 __igt_unique____real_main34()
>   #4 ../tests/amdgpu/amd_pci_unplug.c:34 main()
>   #5 ../sysdeps/nptl/libc_start_call_main.h:58 __libc_start_call_main()
>   #6 ../csu/libc-start.c:128 __libc_start_main@@GLIBC_2.34()
>   #7 [_start+0x25]
> Subtest amdgpu_hotunplug_simple failed.
> 
> V2:
>   - replace igt_skip_on with igt_skip_f(Kamil)
> 
> Cc: Vitaly Prosyak <vitaly.prosyak at amd.com>
> Cc: Luben Tuikov <luben.tuikov at amd.com>
> Cc: Alex Deucher <alexander.deucher at amd.com>
> Cc: Christian Koenig <christian.koenig at amd.com>
> Cc: Tim Huang <tim.huang at amd.com>
> Cc: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> 
> Signed-off-by: Jesse Zhang <Jesse.Zhang at amd.com>
> Reviewed-by: Vitaly Prosyak <vitaly.prosyak at amd.com>
> ---
>  lib/amdgpu/amd_pci_unplug.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/amdgpu/amd_pci_unplug.c b/lib/amdgpu/amd_pci_unplug.c
> index b7af95dec..8346aeb98 100644
> --- a/lib/amdgpu/amd_pci_unplug.c
> +++ b/lib/amdgpu/amd_pci_unplug.c
> @@ -101,8 +101,7 @@ amdgpu_hotunplug_setup_test(bool render_mode, const struct amd_pci_unplug_setup
>  
>  	if (setup->open_device && setup->open_device2 && unplug->num_devices < 2) {
>  		/*Not enough board for the test*/
> -		printf("SKIP ... more than 1 GPU is required for this test\n");
> -		goto release;
> +		igt_skip_on_f(ret, "SKIP ... more than 1 GPU is required for this test\n");
>  	}
>  
>  	tmp_str = amdgpu_get_device_from_fd(unplug->drm_amdgpu_fds[0]);
> @@ -117,7 +116,9 @@ amdgpu_hotunplug_setup_test(bool render_mode, const struct amd_pci_unplug_setup
>  		goto release;
>  
>  	if (minor_version < setup->minor_version_req)
> -		goto release;
> +		igt_skip_on_f(minor_version < setup->minor_version_req,
> +			"It has minor_version: %d but it is required: %d for this test\n",
> +			minor_version, setup->minor_version_req);
>  
>  	/* skip hotplug test on APU (Accelerated Processing Unit CPU + iGPU) */
>  	r = amdgpu_query_gpu_info(unplug->device_handle, &gpu_info);
> @@ -138,7 +139,9 @@ amdgpu_hotunplug_setup_test(bool render_mode, const struct amd_pci_unplug_setup
>  		if (r != 0)
>  			goto release;
>  		if (minor_version < setup->minor_version_req)
> -			goto release;
> +			igt_skip_on_f(minor_version < setup->minor_version_req,
> +				"It has minor_version: %d but it is required: %d for this test\n",
> +				minor_version, setup->minor_version_req);
>  
>  		r = amdgpu_query_gpu_info(unplug->device_handle2, &gpu_info);
>  		igt_assert_eq(r, 0);
> -- 
> 2.25.1
>