[igt-dev] [PATCH i-g-t] tests/kms_color: fix crc assert condition

[Melissa Wen]

On 09/21, Ville Syrjälä wrote:
> On Tue, Sep 20, 2022 at 03:32:39PM +0530, Modem, Bhanuprakash wrote:
> > On Mon-19-09-2022 09:24 pm, Melissa Wen wrote:
> > > On 09/01, Modem, Bhanuprakash wrote:
> > >> On Wed-31-08-2022 03:41 pm, Melissa Wen wrote:
> > >>> In test_pipe_degamma/gamma/ctm, igt_assert_crc_equal() was replaced by
> > >>>
> > >>> ret = !igt_skip_crc_compare || igt_check_crc_equal()
> > >>> and then igt_assert(ret)
> > >>>
> > >>> where igt_check_crc_equal returns !mismatch, and therefore we can
> > >>> translate as:
> > >>>
> > >>> ret = !igt_skip_crc_compare || !mismatch
> > >>>
> > >>> However, the original igt_assert_crc_equal() assertion does:
> > >>>
> > >>> igt_assert(!mismatch || igt_skip_crc_compare)
> > >>>
> > >>> That means, the replacement changes the original assertion. Moreover,
> > >>> negating `igt_skip_crc_compare` makes the test assertion to be always
> > >>> true (sucessful) by default and reverses the logic of
> > >>> --skip-crc-compare.
> > >>>
> > >>> Fixes: d61e4598142 ("tests/kms_color: Convert tests to dynamic")
> > >>> Signed-off-by: Melissa Wen <mwen at igalia.com>
> > >>
> > >> LGTM
> > >> Reviewed-by: Bhanuprakash Modem <bhanuprakash.modem at intel.com>
> > >>
> > >> - Bhanu
> > > 
> > > Thanks for reviewing, Bhanu.
> > > 
> > > Siqueira,
> > > 
> > > Is it okay from AMD side? If so, can you apply it?
> > 
> > Applied.
> 
> It did _not_ pass CI and so should not have been pushed!

Hi Ville,

So, this change "not passed" because the test was not actually assessing
the CRC results (i.e. !igt_skip_crc_compare makes the assertion always
successful - a false PASS). In this context, I think CI testlist should be
updated, instead of not applying this change, right?

BR,

Melissa

> 
> -- 
> Ville Syrjälä
> Intel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/igt-dev/attachments/20220921/68878fa7/attachment.sig>