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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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.

-- 
Ville Syrjälä
Intel