About merging patches without tests

[Itamar Heim]

On 05/21/2014 06:32 PM, David Caro wrote:
> On Wed 21 May 2014 04:28:10 PM CEST, Itamar Heim wrote:
>> On 05/21/2014 05:25 PM, Eyal Edri wrote:
>>>
>>>
>>> ----- Original Message -----
>>>> From: "Itamar Heim" <iheim at redhat.com>
>>>> To: "Sandro Bonazzola" <sbonazzo at redhat.com>, "David Caro"
>>>> <dcaroest at redhat.com>
>>>> Cc: "infra" <infra at ovirt.org>
>>>> Sent: Friday, May 16, 2014 9:48:19 PM
>>>> Subject: Re: About merging patches without tests
>>>>
>>>> On 05/16/2014 05:38 AM, Sandro Bonazzola wrote:
>>>>> Il 16/05/2014 10:37, David Caro ha scritto:
>>>>>> On Fri 16 May 2014 09:33:44 AM CEST, Sandro Bonazzola wrote:
>>>>>>> Il 15/05/2014 18:46, David Caro ha scritto:
>>>>>>>> Hi!
>>>>>>>>
>>>>>>>>    From time to time we have some patches that are merged to master
>>>>>>>>    branches
>>>>>>>> without having been tested mostly because the developer merges before
>>>>>>>> having any
>>>>>>>> response from the jenkins system.
>>>>>>>>
>>>>>>>> Merging one of those patches makes any following test run on that branch
>>>>>>>> to
>>>>>>>> fail, and creating a lot of noise and trouble around all patches and
>>>>>>>> jobs.
>>>>>>>>
>>>>>>>> So I wanted to stat a little discussion to bring up ideas on how to
>>>>>>>> prevent
>>>>>>>> that. Some random thoughts:
>>>>>>>>
>>>>>>>> * -1 the patch at jenkins job start, and reset to 0 on success or infra
>>>>>>>> failure,
>>>>>>>> and keep -1 if jenkins failure
>>>>>>>> * Only send a message to the patch with 'jenkins jobs started'
>>>>>>>
>>>>>>> I think that something like "jenkins jobs started, please don't merge
>>>>>>> until they finish" should be enough.
>>>>>>>
>>>>>>>> * Setup zuul as gateway, and make it block the patches if they do not
>>>>>>>> pass the tests
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Cheers!
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> Infra mailing list
>>>>>>>> Infra at ovirt.org
>>>>>>>> http://lists.ovirt.org/mailman/listinfo/infra
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> Usually the flow is reabase -> submit, that is done pretty easily from
>>>>>> the ui, and it does not give too much time to check any comments (the
>>>>>> rebase and submit buttons are on the top, and the comments show at the
>>>>>> end).
>>>>>> So doing that will not help on that case, as the developer would not
>>>>>> see the comment before submitting. I think we should block, at least
>>>>>> for a couple minutes, so he has to read the comments to be able to
>>>>>> submit.
>>>>>>
>>>>>> We had that before also, adding a comment when the jobs start, but we
>>>>>> removed it by developers request iirc
>>>>>
>>>>> Also recently you can just press submit and it automatically rebase before
>>>>> merging.
>>>>> So maybe yes, just a message isn't enough
>>>>
>>>> its not possible to wait for the job on a simple rebase usually - too
>>>> many collisions possible.
>>>
>>> we're mostly talking about jobs that fail to compile, i think in those cases,
>>> it worth to wait for jenkins job to finish.
>>
>> if the previous patch failed to compile - i agree.
>> if a rebase of a patch that passed previously, waiting for another round of
>> jobs isn't practical if other merge in the meantime. I'd say:
>> - wait for it if last patch is several days old.
>> - make sure to check the email from automation post the merge and revert/fix
>> if it broke in a short loop.
>>
>
> Maybe we can retake the initiative of testing 'on demand', meaning,
> that the high load jobs only run after a maintainer +1 and a verified
> +1, that added to the extra flag might help ease the load and keep
> master stable:

lets wait a bit till we have the extra hardware up and running?

>
> Devel sends patches/rebase/whatever, verifies +1, then a maintainer
> reviews +1 -> jenkins job starts and adds the 'tested' flag when
> finished -> maintainer can submit
>
> But e will need some control over the maintainers of each project (we
> have more or less that already as gerrit groups and permissions).
>
> --
> David Caro
>
> Red Hat S.L.
> Continuous Integration Engineer - EMEA ENG Virtualization R&D
>
> Email: dcaro at redhat.com
> Web: www.redhat.com
> RHT Global #: 82-62605
>