Re: [igt-dev] [PATCH] core_getversion: Test for desired device

[Helen Koike <helen.koike@collabora.com>]

Hi,

On 25/09/2023 09:15, Kamil Konieczny wrote:
> Hi,
> 
> On 2023-09-22 at 14:35:01 +0200, Kamil Konieczny wrote:
>> Hi Rob,
>>
>> On 2023-09-21 at 15:44:22 -0700, Rob Clark wrote:
>>> From: Rob Clark <robdclark@chromium.org>
>>>
>>> We discovered in drm/ci that if the drm device fails to probe, all the
>>> tests come back as "Skip", and the job is considered successful.  So fix
>> -------------------------- ^^^^^ ---------------------------------^
>> No comma before "and", s/, and/ and/, also use only one space separator
>> after end of statement,  s/.  So/. So/. As a side note, it is better to
>> avoid starting with "So", better:
>>
>> Fixed getversion test ...
>>
>>> the getversion test to fail if there is no drm device, or if the drm
>> ------------------------------------------------------ ^
>> s/, or/ or/
>>
>>> device does not match the expected device.
>>
>> Please write also that you used new enviroment var IGT_REQUIRED_DRIVERS
>> for that purpose.
>>
>>>
>>> Signed-off-by: Rob Clark <robdclark@chromium.org>
>>> ---
>>>   tests/core_getversion.c | 6 +++++-
>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/core_getversion.c b/tests/core_getversion.c
>>> index 32cb976e4923..e5416993d7a3 100644
>>> --- a/tests/core_getversion.c
>>> +++ b/tests/core_getversion.c
>>> @@ -48,14 +48,18 @@ igt_simple_main
>>>   {
>>>   	int fd;
>>>   	drmVersionPtr v;
>>> +	const char *name = getenv("IGT_REQUIRED_DRIVERS");
>>>   
>>> -	fd = drm_open_driver(DRIVER_ANY);
>>> +	fd = __drm_open_driver(DRIVER_ANY);
>>> +	igt_assert_fd(fd);
>>>   	v = drmGetVersion(fd);
>>>   	igt_assert_neq(strlen(v->name), 0);
>>>   	igt_assert_neq(strlen(v->date), 0);
>>>   	igt_assert_neq(strlen(v->desc), 0);
>>>   	if (is_i915_device(fd))
>>>   		igt_assert_lte(1, v->version_major);
>>> +	if (name)
>>> +		igt_assert_eq(strcmp(name, v->name), 0);
>> ------------------ ^
>> Better for debug would be:
>> 		igt_assert_f(!strcmp(name, v->name), "Expected driver: %s but got: %s\n", name, v->name);
> -------------------- ^
> 
> This should be:
>   		igt_assert_f(strcmp(name, v->name), "Expected driver: %s but got: %s\n", name, v->name);


According to 
https://drm.pages.freedesktop.org/igt-gpu-tools/igt-gpu-tools-Core.html#igt-assert-f

igt_assert_f "Fails (sub-)test if the condition is not met."

If we want name and v->name to match, strcmp returns 0, so it should be 
igt_assert_f(!strcmp.. no? Unless I'm missing something.

Regards,
Helen

> 
> Regards,
> Kamil
> 
>>
>> Regards,
>> Kamil
>>
>>>   
>>>   	drmFree(v);
>>>   	drm_close_driver(fd);
>>> -- 
>>> 2.41.0
>>>