[ovirt-devel] Creating a new gerrit flag

Wed Dec 10 16:44:02 UTC 2014

----- Original Message -----
> From: "David Caro" <dcaroest at redhat.com>
> To: "Nir Soffer" <nsoffer at redhat.com>
> Cc: "Oved Ourfali" <ovedo at redhat.com>, devel at ovirt.org, "infra" <infra at ovirt.org>
> Sent: Wednesday, December 10, 2014 4:59:36 PM
> Subject: Re: [ovirt-devel] Creating a new gerrit flag
> 
> On 12/10, Nir Soffer wrote:
> > 
> > 
> > ----- Original Message -----
> > > From: "Eyal Edri" <eedri at redhat.com>
> > > To: devel at ovirt.org
> > > Cc: "Oved Ourfali" <ovedo at redhat.com>, "infra" <infra at ovirt.org>
> > > Sent: Wednesday, December 10, 2014 10:40:47 AM
> > > Subject: Re: [ovirt-devel] Creating a new gerrit flag
> > > 
> > > 
> > > 
> > > ----- Original Message -----
> > > > From: "Oved Ourfali" <ovedo at redhat.com>
> > > > To: "David Caro" <dcaroest at redhat.com>
> > > > Cc: devel at ovirt.org
> > > > Sent: Wednesday, December 10, 2014 8:30:30 AM
> > > > Subject: Re: [ovirt-devel] Creating a new gerrit flag
> > > > 
> > > > 
> > > > 
> > > > ----- Original Message -----
> > > > > From: "David Caro" <dcaroest at redhat.com>
> > > > > To: "Oved Ourfali" <ovedo at redhat.com>
> > > > > Cc: devel at ovirt.org
> > > > > Sent: Tuesday, December 9, 2014 7:02:44 PM
> > > > > Subject: Re: [ovirt-devel] Creating a new gerrit flag
> > > > > 
> > > > > On 12/09, Oved Ourfali wrote:
> > > > > > 
> > > > > > 
> > > > > > ----- Original Message -----
> > > > > > > From: "David Caro" <dcaroest at redhat.com>
> > > > > > > To: "Oved Ourfali" <ovedo at redhat.com>
> > > > > > > Cc: "Sven Kieske" <s.kieske at mittwald.de>, devel at ovirt.org
> > > > > > > Sent: Tuesday, December 9, 2014 3:40:30 PM
> > > > > > > Subject: Re: [ovirt-devel] Creating a new gerrit flag
> > > > > > > 
> > > > > > > On 12/09, Oved Ourfali wrote:
> > > > > > > > 
> > > > > > > > 
> > > > > > > > ----- Original Message -----
> > > > > > > > > From: "Sven Kieske" <s.kieske at mittwald.de>
> > > > > > > > > To: devel at ovirt.org
> > > > > > > > > Sent: Tuesday, December 9, 2014 3:21:43 PM
> > > > > > > > > Subject: Re: [ovirt-devel] Creating a new gerrit flag
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > On 09/12/14 13:47, Oved Ourfali wrote:
> > > > > > > > > > safe up to 95% or so.
> > > > > > > > > 
> > > > > > > > > You just made up that number.
> > > > > > > > > I don't really understand why you would want
> > > > > > > > > to downgrade your code quality by circumventing tests.
> > > > > > > > > 
> > > > > > > > > Maybe someone can elaborate on this a bit?
> > > > > > > > > 
> > > > > > > > 
> > > > > > > > It doesn't downgrade the code quality.
> > > > > > > > It is just a way to ensure developers can both merge changes,
> > > > > > > > and
> > > > > > > > do
> > > > > > > > it
> > > > > > > > as
> > > > > > > > safely as possible without relying on post-submit tools.
> > > > > > > > The number is indeed invented... as I don't have real
> > > > > > > > statistics,
> > > > > > > > but
> > > > > > > > it
> > > > > > > > comes to say that it would be safe most of the time.
> > > > > > > > After the patch is merged, if CI will fail, it is the
> > > > > > > > responsibility
> > > > > > > > of
> > > > > > > > the
> > > > > > > > developer to check that out and fix that.
> > > > > > > 
> > > > > > > This thread was started to avoid getting to that point, as
> > > > > > > getting a
> > > > > > > failed patch inside the code means breaking all the other tests
> > > > > > > that
> > > > > > > run on top of it and that blocks all the development, not only
> > > > > > > that
> > > > > > > specific patch.
> > > > > > > 
> > > > > > 
> > > > > > The issue that started the discussion was an issue in which there
> > > > > > was a
> > > > > > Tests "-1" flag, and it was ignored.
> > > > > > My suggestion will enforce that it won't be ignored.
> > > > > > In more rare cases, in which the rebase is the source of the tests
> > > > > > issue,
> > > > > > then you'll find about it later.
> > > > > 
> > > > > I started the discussion, and I started it because a developer
> > > > > complained about not being able to merge a patch because it was
> > > > > failing the tests due to an already merged patch that was making all
> > > > > the builds to fail. And was trying to get a solution to avoid getting
> > > > > to that point where a patch is merged while breaking the tests.
> > > > > 
> > > > > 
> > > > > So in summary, you are suggestion this:
> > > > > 
> > > > > Creating a new flag 'tested' with values +1, 0 and -1 that only
> > > > > jenkins
> > > > > and managers can set
> > > > > 
> > > > > Block form submitting any patches that have a -1
> > > > > 
> > > > > Carry the value of that flag to following patches only if the flag
> > > > > was
> > > > > -1
> > > > > 
> > > > 
> > > 
> > > +1, we need a way to block bad patches from being merged, even with a
> > > rebase
> > > in gerrit.
> > > going forward we're planning a few changes to the way jenkins jobs are
> > > run on
> > > ovirt ci, which will help
> > > reduce noise and imrove resources usages.
> > > 
> > > 1. moving into a flow process, where critical jobs like unit
> > > tests/checkstyle
> > > will run first and only then other heavy jobs will run
> > > (integration/rpms/findbugs)
> > 
> > This is already implemented in vdsm for few months - running "make check"
> > will run the fast tests first and will not run the slower tests if a fast
> > test
> > failed.
> 
> Please change to be able to run only fast tests or only slow tests,
> that way we can separate the job into two and give feedback about the
> fast tests before the slows have finished running.
> 
> Actually what eyal is talking about is not inside the project flow,
> but jenkins build pipeline. Thar ranges from static checks, unit
> tests, functional tests, builds and deployments (in the future).
> 
> So instead of having one job for each step and running all of them in
> parallel, you'll run in a hierarchical manner, to avoid having to wat
> for all the tests to get feedback or failing before starting the most
> complex long-running tests.

+1 for preventing failing test changes to be merged into master.
I also agree that the biggest barrier for achieving that are the test that run ages
and making them finish in a shorter time would enable us implement the proposed change.
We could've achieve that by:
1. Splitting our repo into several smaller ones (according to the projects structure) 
   so only the very relevant part of the project tests would be run on on every change merge.
2. Creating only fast running unit tests. 
2.1. All test that run longer than 2 seconds should be reviewed and rewritten/eliminated.
2.2. Avoid creating the unit tests that check more than one class functionality. 
     Using DI in our code should help us with that.
3. If we insist on long running tests, I guess they aren't proper unit tests, 
   we should extract them into a separate test set and run them periodically (nightly).
> 
> 
> > 
> > Nir
> > _______________________________________________
> > Infra mailing list
> > Infra at ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/infra
> 
> --
> David Caro
> 
> Red Hat S.L.
> Continuous Integration Engineer - EMEA ENG Virtualization R&D
> 
> Tel.: +420 532 294 605
> Email: dcaro at redhat.com
> Web: www.redhat.com
> RHT Global #: 82-62605
> 
> _______________________________________________
> Devel mailing list
> Devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel