Hi,


On 8/2/2018 4:13 PM, Maarten Lankhorst wrote:
> Op 02-08-18 om 12:42 schreef Maarten Lankhorst:
>> Op 02-07-18 om 13:27 schreef Maarten Lankhorst:
>>> Op 02-07-18 om 13:16 schreef Mahesh Kumar:
>>>> Now crc-core framework verifies the source string passed by the user.
>>>> So setting bad-source will fail. Expect file write to fail in bad-source
>>>> subtest of kms_pipe_crc_basic.
>>>>
>>>> Signed-off-by: Mahesh Kumar <mahesh1.kumar@intel.com>
>>>> ---
>>>>   tests/kms_pipe_crc_basic.c | 3 +--
>>>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>>>
>>>> diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
>>>> index 235fdc38..2d4dfee8 100644
>>>> --- a/tests/kms_pipe_crc_basic.c
>>>> +++ b/tests/kms_pipe_crc_basic.c
>>>> @@ -48,8 +48,7 @@ static struct {
>>>>   
>>>>   static void test_bad_source(data_t *data)
>>>>   {
>>>> -	igt_assert(igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo"));
>>>> -	igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) == -1);
>>>> +	igt_assert(!igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo"));
>>>>   }
>>>>   
>>>>   #define N_CRCS	3
>>> New behavior makes more sense.
>>>
>>> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>
>>> Do you have igt commit rights?
>>>
>> Any objections if we change this to allow both behaviors?
>>
>> diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
>> index 235fdc386ba2..91909fa42f2f 100644
>> --- a/tests/kms_pipe_crc_basic.c
>> +++ b/tests/kms_pipe_crc_basic.c
>> @@ -48,8 +48,11 @@ static struct {
>>   
>>   static void test_bad_source(data_t *data)
>>   {
>> -	igt_assert(igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo"));
>> -	igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) == -1);
>> +	errno = 0;
>> +	if (igt_sysfs_set(data->debugfs, "crtc-0/crc/control", "foo"))
>> +		igt_assert(openat(data->debugfs, "crtc-0/crc/data", O_WRONLY) == -1);
>> +	else
>> +		igt_assert_eq(errno, EINVAL);
>>   }
>>   
>>   #define N_CRCS	3
>>
> Hm without the else, errno should be EINVAL in any case..
agree, with this change
Reviewed-by: Mahesh Kumar <mahesh1.kumar@intel.com>