[Gluster-devel] Gerrit review, submit type and Jenkins testing

[Raghavendra Talur]

Hi,

While trying to understand how our gerrit+jenkins setup works, I realized
of a possibility of allowing bugs to get in.

Currently, our gerrit is setup to have cherry-pick as the submit type. Now
consider a case where:

Dev1 sends a commit B with parent commit A(A is already merged).
Dev2 sends a commit C with parent commit A(A is already merged).

Both the patches get +2 from Jenkins.

Maintainer merges commit B from Dev1.
Another maintainer merges commit C from Dev2.

If the two commits B and C changed code which had no merge conflicts but
were conflicting in logic,
then we have a master which has bugs.

If Dev3 now sends a commit D with re-based master as parent, we have the
following cases:

1. If bug introduced above is not racy, we have tests always failing for
Dev3 on commit D. Tests that fail would be from components that commit B
and C changed. Dev3 has no idea on how to fix them and has to enlist help
from Dev1 and Dev2.

2. If bug introduced above is racy, then there is a probability that Dev3
escapes from this trouble and someone else will bear it later. Even if the
racy code is hit and test fails, Dev3 will probably re-trigger the tests
given that they failed for a component which is not related to his/her code
and the bug stays in code longer.

The most obvious but not practical solution to the above problem is to
change the submit type in gerrit to "fast-forward only". It would then
ensure that once commit B is merged, Dev2 has to re-base and re-run the
tests on commit C with commit B as parent, before it could be merged. It is
not practical because it will cause all patches in review to get re-based
and re-triggered whenever a patch is merged.

A little modification to the above solution would be to

   - change submit type to fast-forward only
   - don't run any jenkins job on patches till they get +2 from reviewers
   - once a +2 is given, run jenkins job on patch and automatically submit
   it if test passes.
   - automatically rebase all patches on review with new master and mark
   conflict if merge conflict arises.

As a side effect of this, Dev would now be forced to run a complete
regression on dev machine before sending a patch for review.

Any thoughts on the above solutions or other suggestions?

Thanks,
Raghavendra Talur
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.gluster.org/pipermail/gluster-devel/attachments/20151110/75047748/attachment-0001.html>