[igt-dev] [i-g-t V2 8/8] tests/kms_color: Fix crc compare check in CTM tests

[Modem, Bhanuprakash]

On Fri-23-09-2022 01:10 am, Ville Syrjälä wrote:
> On Thu, Sep 22, 2022 at 03:06:22PM -0400, Lyude Paul wrote:
>> On Thu, 2022-09-22 at 09:18 +0300, Ville Syrjälä wrote:
>>> On Thu, Sep 22, 2022 at 08:49:50AM +0300, Ville Syrjälä wrote:
>>>> On Thu, Sep 22, 2022 at 10:41:42AM +0530, Bhanuprakash Modem wrote:
>>>>> !igt_skip_crc_compare || igt_check_crc_equal() is always true
>>>>> which is not correct.
>>>>>
>>>>> Fixes: 1a42910d
>>>>
>>>> Hang on. Now you're saying the regression in detecting actual failures
>>>> was introduced in commit 1a42910d4f8b ("tests/kms_color: Don't opencode
>>>> igt_check_crc_equal()") rather than the dynamic subtest conversion
>>>> commit (which is what the previous Fixes line claimed)?
>>>>
>>>> So apparently the test may have been broken for 1.5 years now and
>>>> no one realize it.
>>>
>>> Looks like the tests were in fact failing (at least on some hw)
>>> before and CI did notice the change in the behaviour:
>>> https://patchwork.freedesktop.org/series/88075/#rev2
>>>
>>> But it did not raise any kind of stink about that, and still
>>> flagged the whole thing as an overall success. I presume
>>> Lyude neglected to look at the individual results in any detail
>>> once the overall succees was indicated (kinda dangerous thing
>>> to do with our CI it seems).
>>
>> sheesh, I'll definitely keep this in mind for the future. Glad it at least
>> wasn't a clear cut case of me just missing the CI output entirely lol…
>>
>> fwiw, feel fine to revert this change. it was a minor nitpick
> 
> I guess some kind of mass revert is still needed, or someone
> needs to just figure out why the test is now broken. And I'm
> not talking about the failures on icl+ (or maybe glk+) since
> those are apparently expected. But the idle runs are now
> showing the test failing on absolutely all machines, so
> clearly a real regression has managed to sneak in.

 From this series (or the same series in try-bot [1]), it is clear that 
this regression is not due to the dynamic subtests.

I'll try to figure out why the test is failing.

[1] https://patchwork.freedesktop.org/series/108837/

- Bhanu

>