OSDir

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [VOTE] Stricter commit guidelines


+1. Can we also add something about revert-first policy if some patch
breaks tests?
So that it’s ok to revert if the tests aren’t fixed quickly with a
follow-up.

On 18/5/15, 13:08, "Jesus Camacho Rodriguez" <jcamacho@xxxxxxxxxx> wrote:

>I was hoping that by being stricter, we are going to make an effort to
>fix the flaky
>tests. The reason is precisely what you mention: if you cannot commit,
>you need
>to fix this situation. That is what I meant with providing an additional
>incentive to
>make testing more robust: currently there is no incentive. I do not think
>that
>improving tests is a responsibility of a single developer, but rather a
>responsibility
>of all of us. Disabling the tests is a one-time solution to get to a
>clean run, trying to
>accelerate the process to get to it, as we did not want to block
>development for
>weeks. Then flaky tests should just not go in, and if they do, we can
>just revert
>the patch (this is what [2] says btw).
>
>The first thing we need to do is identifying why a test is flaky. After
>examining runs
>for the last few days, I saw many of them fall in following categories:
>- Many of them are flaky because of estimations such as data size. One
>possible
>solution is to mask data size for those tests, as we already mask some
>environment
>dependent information.
>- Some of them are flaky because environment issues, e.g., I see this a
>lot with
>TestTriggersMoveWorkloadManager. If their logic cannot be rewritten, a
>possible
>solution is to add a max number of retries selectively for those tests
>(surefire has
>a rerunFailingTestsCount option that I am not familiar with), expecting
>that they
>pass at least once.
>
>Not sure if you have other ideas?
>
>-Jesús
>
>
>On 5/15/18, 11:59 AM, "Vihang Karajgaonkar" <vihang@xxxxxxxxxxxx> wrote:
>
>    Can we also define a standard process to identify a flaky test and
>thereby
>    making it eligible to be disabled? I am worried that the intermittent
>the
>    flaky ones will stall the patches when we restart allowing the
>commits.
>    
>    On Tue, May 15, 2018 at 10:50 AM, Vineet Garg <vgarg@xxxxxxxxxxxxxxx>
>wrote:
>    
>    > +1
>    >
>    > > On May 15, 2018, at 9:13 AM, Alan Gates <alanfgates@xxxxxxxxx>
>wrote:
>    > >
>    > > +1.
>    > >
>    > > Alan.
>    > >
>    > > On Tue, May 15, 2018 at 9:12 AM, Sergio Pena
><sergio.pena@xxxxxxxxxxxx>
>    > > wrote:
>    > >
>    > >> +1
>    > >>
>    > >> On Tue, May 15, 2018 at 11:05 AM, Gunther Hagleitner <
>    > >> ghagleitner@xxxxxxxxxxxxxxx> wrote:
>    > >>
>    > >>> +1
>    > >>> ________________________________________
>    > >>> From: Sankar Hariappan <shariappan@xxxxxxxxxxxxxxx>
>    > >>> Sent: Tuesday, May 15, 2018 9:03 AM
>    > >>> To: dev@xxxxxxxxxxxxxxx
>    > >>> Subject: Re: [VOTE] Stricter commit guidelines
>    > >>>
>    > >>> +1
>    > >>>
>    > >>>
>    > >>> On 15/05/18, 9:30 PM, "Sahil Takiar" <takiar.sahil@xxxxxxxxx>
>wrote:
>    > >>>
>    > >>>> +1
>    > >>>>
>    > >>>> On Tue, May 15, 2018 at 10:56 AM, Owen O'Malley <
>    > owen.omalley@xxxxxxxxx
>    > >>>
>    > >>>> wrote:
>    > >>>>
>    > >>>>> +1
>    > >>>>>
>    > >>>>> On Tue, May 15, 2018 at 8:55 AM, Peter Vary
><pvary@xxxxxxxxxxxx>
>    > >> wrote:
>    > >>>>>
>    > >>>>>> +1 - Hoping for something like this for a long while! Thanks
>for
>    > >>> taking
>    > >>>>>> this up all!
>    > >>>>>>
>    > >>>>>>> On May 15, 2018, at 5:44 PM, Jesus Camacho Rodriguez <
>    > >>>>>> jcamacho@xxxxxxxxxx> wrote:
>    > >>>>>>>
>    > >>>>>>> Forgot to mention the length of the vote in original
>message.
>    > >>>>>>>
>    > >>>>>>> Let's leave the vote open for a shorter period than usual,
>for
>    > >>> instance
>    > >>>>>> 48 hours, i.e., till Wednesday 10pm PST. Situation can only
>get
>    > >> worse
>    > >>>>> than
>    > >>>>>> it is now if we do not take action for a longer period.
>    > >>>>>>>
>    > >>>>>>> As Alan suggested, vote passes if there is a lazy majority
>(at
>    > >>> least 3
>    > >>>>>> votes, more +1s than -1s).
>    > >>>>>>>
>    > >>>>>>> Thanks,
>    > >>>>>>> Jesús
>    > >>>>>>>
>    > >>>>>>>
>    > >>>>>>> On 5/15/18, 8:37 AM, "Andrew Sherman"
><asherman@xxxxxxxxxxxx>
>    > >>> wrote:
>    > >>>>>>>
>    > >>>>>>>   +1
>    > >>>>>>>
>    > >>>>>>>   On Tue, May 15, 2018 at 2:34 AM Rui Li
><lirui.fudan@xxxxxxxxx>
>    > >>>>> wrote:
>    > >>>>>>>
>    > >>>>>>>> +1
>    > >>>>>>>>
>    > >>>>>>>> On Tue, May 15, 2018 at 2:24 PM, Prasanth Jayachandran <
>    > >>>>>>>> pjayachandran@xxxxxxxxxxxxxxx> wrote:
>    > >>>>>>>>
>    > >>>>>>>>> +1
>    > >>>>>>>>>
>    > >>>>>>>>>
>    > >>>>>>>>>
>    > >>>>>>>>> Thanks
>    > >>>>>>>>> Prasanth
>    > >>>>>>>>>
>    > >>>>>>>>>
>    > >>>>>>>>>
>    > >>>>>>>>> On Mon, May 14, 2018 at 10:44 PM -0700, "Jesus Camacho
>    > >> Rodriguez"
>    > >>> <
>    > >>>>>>>>> jcamacho@xxxxxxxxxx<mailto:jcamacho@xxxxxxxxxx>> wrote:
>    > >>>>>>>>>
>    > >>>>>>>>>
>    > >>>>>>>>> After work has been done to ignore most of the tests that
>were
>    > >>>>> failing
>    > >>>>>>>>> consistently/intermittently [1], I wanted to start this
>vote to
>    > >>>>> gather
>    > >>>>>>>>> support from the community to be stricter wrt committing
>patches
>    > >>> to
>    > >>>>>> Hive.
>    > >>>>>>>>> The committers guide [2] already specifies that a +1
>should be
>    > >>>>> obtained
>    > >>>>>>>>> before committing, but there is another clause that allows
>    > >>> committing
>    > >>>>>>>> under
>    > >>>>>>>>> the presence of flaky tests (clause 4). Flaky tests are
>as good
>    > >> as
>    > >>>>>> having
>    > >>>>>>>>> no tests, hence I propose to remove clause 4 and enforce
>the +1
>    > >>> from
>    > >>>>>>>>> testing infra before committing.
>    > >>>>>>>>>
>    > >>>>>>>>>
>    > >>>>>>>>>
>    > >>>>>>>>> As I see it, by enforcing that we always get a +1 from the
>    > >> testing
>    > >>>>>> infra
>    > >>>>>>>>> before committing, 1) we will have a more stable project,
>and 2)
>    > >>> we
>    > >>>>>> will
>    > >>>>>>>>> have another incentive as a community to create a more
>robust
>    > >>> testing
>    > >>>>>>>>> infra, e.g., replacing flaky tests for similar unit tests
>that
>    > >> are
>    > >>>>> not
>    > >>>>>>>>> flaky, trying to decrease running time for tests, etc.
>    > >>>>>>>>>
>    > >>>>>>>>>
>    > >>>>>>>>>
>    > >>>>>>>>> Please, share your thoughts about this.
>    > >>>>>>>>>
>    > >>>>>>>>>
>    > >>>>>>>>>
>    > >>>>>>>>> Here is my +1.
>    > >>>>>>>>>
>    > >>>>>>>>>
>    > >>>>>>>>>
>    > >>>>>>>>> Thanks,
>    > >>>>>>>>>
>    > >>>>>>>>> Jes?s
>    > >>>>>>>>>
>    > >>>>>>>>>
>    > >>>>>>>>>
>    > >>>>>>>>> [1]
>http://mail-archives.apache.org/mod_mbox/hive-dev/201805.
>    > >>>>>>>>> 
>mbox/%3C63023673-AEE5-41A9-BA52-5A5DFB2078B6%40apache.org%3E
>    > >>>>>>>>>
>    > >>>>>>>>> [2] https://cwiki.apache.org/confluence/display/Hive/
>    > >>>>>>>>> HowToCommit#HowToCommit-PreCommitruns,andcommittingpatches
>    > >>>>>>>>>
>    > >>>>>>>>>
>    > >>>>>>>>>
>    > >>>>>>>>>
>    > >>>>>>>>>
>    > >>>>>>>>
>    > >>>>>>>>
>    > >>>>>>>> --
>    > >>>>>>>> Best regards!
>    > >>>>>>>> Rui Li
>    > >>>>>>>>
>    > >>>>>>>
>    > >>>>>>>
>    > >>>>>>>
>    > >>>>>>
>    > >>>>>>
>    > >>>>>
>    > >>>>
>    > >>>>
>    > >>>>
>    > >>>> --
>    > >>>> Sahil Takiar
>    > >>>> Software Engineer
>    > >>>> takiar.sahil@xxxxxxxxx | (510) 673-0309
>    > >>>
>    > >>>
>    > >>>
>    > >>
>    >
>    >
>    
>
>