From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTPS id C229F10E09D for ; Thu, 22 Sep 2022 06:18:10 +0000 (UTC) Date: Thu, 22 Sep 2022 09:18:06 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Bhanuprakash Modem Message-ID: References: <20220922051142.2003118-1-bhanuprakash.modem@intel.com> <20220922051142.2003118-9-bhanuprakash.modem@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Subject: Re: [igt-dev] [i-g-t V2 8/8] tests/kms_color: Fix crc compare check in CTM tests List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: igt-dev@lists.freedesktop.org, "Sarvela, Tomi P" , Petri Latvala Errors-To: igt-dev-bounces@lists.freedesktop.org Sender: "igt-dev" List-ID: 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