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

[Ville Syrjälä]

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).

So why did CI not present that change in behaviour more
prominently?

Is it because we went from FAIL to apparent SUCCESS
and that's always considered sort of fine. I suppose flagging
the thing a failure because something seems to have gotten
fixed would be weird, but maybe it should at least present
such changes in behaviour at the top of the list.

Or is it because it attributed that change to these bugs
https://gitlab.freedesktop.org/drm/intel/-/issues/1149
https://gitlab.freedesktop.org/drm/intel/-/issues/315
Does CI consider everything with an associated bug a potential
ping pong and just basically hides any change it the result
in that big list of CI noise in the report (which this series 
hit a lot)? If so we need to get that fixed. Expected fail
should be handled differently from a known ping pong.

-- 
Ville Syrjälä
Intel