[Gluster-infra] Zuul?

[Nigel Babu]

On Sun, Sep 04, 2016 at 07:43:04PM +0200, Niels de Vos wrote:
> On Fri, Sep 02, 2016 at 12:12:00PM -0400, Jeff Darcy wrote:
> > > We already only merge after NetBSD-regression and CentOS-regression have
> > > voted
> > > back. All I'm changing is that you don't need to do the merge manually or do
> > > Verified +1 for regression to run.. Zuul will run the tests after you get
> > > Code-Review +2 and merge it for you with patches ordered correctly.
> >
> > The problem is that some reviewers (including myself) might not even look at
> > a patch until it already has CentOS+1 and NetBSD+1.  Reviewing code, having
> > it fail regressions, reviewing a substantially new version, having *that*
> > fail regressions, etc. tends to be very frustrating for both authors and
> > reviewers.  Fighting with the regression tests *prior* to review can still
> > be very frustrating for authors, but at least it doesn't frustrate reviewers
> > as much and doesn't contribute to author/reviewer animosity (apparently a
> > real problem in this group) as much.
>
> This is the main problem I see with this proposal too. There are quite
> regular patches posted that break things in related regression tests.
> Those corner cases can be very difficult to spot in code review, but
> automated testing catches them. It is one of the reasons I prefer to do
> code reviews for changes that are known to be (mostly) correct. Other
> times changes (they should!) add test-cases, and these fail with the
> initial versions...

This is a problem I hadn't realized we had. This makes it mostly a no-go to do
regressions after code review.

>
> > That said, it would be nice to have *something* as a gate between +2 and
> > merge - certainly a build, and at least a few basic tests (more than smoke
> > does IMO).  If Zuul can help us avoid broken builds due to improper merge
> > order, which seem to be the most common kind of broken builds, I'm all for
> > it.
>
> Our regression test suite allows running tests in a subdirectory, so
> changes to the tests should not really be needed. In addition to the
> simple smoke test, this might do:
>
>   # ./run-tests.sh tests/basic/

Perhaps, we could do build + smoke + tests/basic pre-merge so we catch *some*
of the edge cases.

>
> Niels



--
nigelb