+1 I vehemently agree On Thu, Jan 6, 2022 at 2:53 PM Aizhamal Nurmamat kyzy <[email protected]> wrote:
> +1 > > On Thu, Jan 6, 2022 at 6:31 AM Kaxil Naik <[email protected]> wrote: > >> +1 - Agree with the Proposal, will take care of it myself too >> >> On Thu, Jan 6, 2022 at 5:21 PM Ash Berlin-Taylor <[email protected]> wrote: >> >>> Given the work you and the Outreachy interns?) have put in to fix the >>> previous flaky tests I 100% agree. >>> >>> Main is now in much better state with greatly reduced number of flaky >>> tests/false negatives, so yes, if we see a build fail it should be treated >>> as a real failure. >>> >>> On 6 January 2022 10:39:49 GMT, Jarek Potiuk <[email protected]> wrote: >>>> >>>> Hey everyone, >>>> >>>> I know we had quite a long period of flaky tests and accepting the >>>> fact that we merge PRs with some tests failing because of the >>>> flakiness. >>>> >>>> However I think over a couple of months or so we have invested heavily >>>> into fixing it - a number of people tracked and fixed a big number of >>>> flaky tests and what we have now is mostly "Green". >>>> >>>> Yeah - sometimes it happens we - by mistake merge a change that causes >>>> "main" failure (for example because our test harness is not perfect) >>>> but we should fix those cases quickly (mostly by reverting the >>>> offending commit and redoing it). >>>> >>>> But I think we should (and I am talking about committers) stop the >>>> case of merging "failed" PRs if we are not absolutely sure that the >>>> failure is already fixed in PR (or being fixed) . >>>> >>>> We had some changes merged recently (and I was as guilty as others) >>>> where we merged a "real" failure without properly investigating the >>>> root cause. The effect of that is the "broken window" effect - once >>>> such PR gets merged, it fails other PRs (until fixed) and it makes >>>> people impatient to merge PR with the failure because this is >>>> "normal". It should be normal to only merge "green" PRs. >>>> >>>> I propose that we change our approach and whenever we see a "red" >>>> build every committer's approach should be : >>>> >>>> * investigate the root cause >>>> * if it's main - attempt to fix it in main first before merging (could >>>> be by reverting the failed commit) >>>> * or discuss it in #development /devlist if it is not easy to find >>>> * and generally only merge a failed PR if you are absolutely sure the >>>> failure has already been fixed (or you know someone works on fixing >>>> it) - and ALWAYS comment about it in the PR explaining why you merge >>>> failed PR >>>> >>>> This is a proposal, happy to discuss it if others think differently. WDYT ? >>>> >>>> J. >>>> >>>> -- Vikram Koka *SVP Engineering* Email: [email protected] Mobile: +1 408 966 2203
