Fix GerritStatusPush 'startCB' not work

[stanzgy]

Fix GerritStatusPush 'startCB' not work with Gerrit+Git.
GerritStatusPush 'startCB' triggers before buildsteps start. But the condition
'startCB' decide to send code reviews is the property 'gerrit_branch' which is
set during the buildbot.steps.source.gerrit.Gerrit buildstep. So use property
'event.change.id' instead of 'gerrit_branch' to verify Gerrit source to ensure
'startCB' works properly.

The property 'got_revision' here also has same problem when 'startCB' triggers.

[sa2ajj]

Tests are failing:

[ERROR]
Traceback (most recent call last):
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/unit/test_status_gerrit.py", line 307, in check
    result)
  File "/home/travis/virtualenv/python2.6.9/lib/python2.6/site-packages/mock.py", line 845, in assert_called_once_with
    raise AssertionError(msg)
exceptions.AssertionError: Expected to be called once. Called 0 times.

buildbot.test.unit.test_status_gerrit.TestGerritStatusPush.test_buildsetComplete_failure_sends_review
buildbot.test.unit.test_status_gerrit.TestGerritStatusPush.test_buildsetComplete_success_sends_review
===============================================================================
[ERROR]
Traceback (most recent call last):
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/unit/test_status_gerrit.py", line 322, in check
    result)
  File "/home/travis/virtualenv/python2.6.9/lib/python2.6/site-packages/mock.py", line 845, in assert_called_once_with
    raise AssertionError(msg)
exceptions.AssertionError: Expected to be called once. Called 0 times.

buildbot.test.unit.test_status_gerrit.TestGerritStatusPush.test_buildsetComplete_failure_sends_review_legacy
buildbot.test.unit.test_status_gerrit.TestGerritStatusPush.test_buildsetComplete_success_sends_review_legacy
===============================================================================
[ERROR]
Traceback (most recent call last):
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/unit/test_status_gerrit.py", line 200, in check
    result)
  File "/home/travis/virtualenv/python2.6.9/lib/python2.6/site-packages/mock.py", line 845, in assert_called_once_with
    raise AssertionError(msg)
exceptions.AssertionError: Expected to be called once. Called 0 times.

buildbot.test.unit.test_status_gerrit.TestGerritStatusPush.test_buildsetComplete_failure_sends_summary_review
buildbot.test.unit.test_status_gerrit.TestGerritStatusPush.test_buildsetComplete_mixed_sends_summary_review
buildbot.test.unit.test_status_gerrit.TestGerritStatusPush.test_buildsetComplete_success_sends_summary_review
===============================================================================
[ERROR]
Traceback (most recent call last):
  File "/home/travis/build/buildbot/buildbot/master/buildbot/test/unit/test_status_gerrit.py", line 217, in check
    result)
  File "/home/travis/virtualenv/python2.6.9/lib/python2.6/site-packages/mock.py", line 845, in assert_called_once_with
    raise AssertionError(msg)
exceptions.AssertionError: Expected to be called once. Called 0 times.

buildbot.test.unit.test_status_gerrit.TestGerritStatusPush.test_buildsetComplete_failure_sends_summary_review_legacy
buildbot.test.unit.test_status_gerrit.TestGerritStatusPush.test_buildsetComplete_mixed_sends_summary_review_legacy
buildbot.test.unit.test_status_gerrit.TestGerritStatusPush.test_buildsetComplete_success_sends_summary_review_legacy

[tardyp]

This plugin is to be rewritten completely in buildbot nine.

Even if I'm not convinced that the startCB is a feature people should enable (that's too much email spam for your users), this still makes a good usecase for the new reporter framework.

So this has to be fixed in eight branch, not master.

[stanzgy]

@sa2ajj Testcase for startCB are added in #1575
@tardyp Already add a new pr on branch eight in #1575. I think startCB is useful when people want to know if buildbot starts running on a gerrit patchset. Maybe it's good to make it configurable and disable on default.

[tardyp]

People want that in the beginning then you will get all your developers
yell at you because of the amount of generated email traffic. :)

Le mar. 3 mars 2015 08:49, stanzgy notifications@github.com a écrit :

@sa2ajj https://github.com/sa2ajj Testcase for startCB are added in
#1575 #1575
@tardyp https://github.com/tardyp Already add a new pr on branch eight
in #1575 #1575. I think
startCB is useful when people want to know if buildbot starts running on a
gerrit patchset. Maybe it's good to make it configurable and disable on
default.

—
Reply to this email directly or view it on GitHub
#1570 (comment).

[stanzgy]

yeah, in most times the massive emails from gerrit CI bots are really
annoyed. But in some very case we indeed need this startCB. So it's a
tradeoff between massive email traffic and startCB function, let peopel
themselves decide.

On Tue, Mar 3, 2015 at 3:53 PM, Pierre Tardy notifications@github.com
wrote:

People want that in the beginning then you will get all your developers
yell at you because of the amount of generated email traffic. :)

Le mar. 3 mars 2015 08:49, stanzgy notifications@github.com a écrit :

@sa2ajj https://github.com/sa2ajj Testcase for startCB are added in
#1575 #1575
@tardyp https://github.com/tardyp Already add a new pr on branch eight
in #1575 #1575. I think
startCB is useful when people want to know if buildbot starts running on
a
gerrit patchset. Maybe it's good to make it configurable and disable on
default.

—
Reply to this email directly or view it on GitHub
#1570 (comment).

—
Reply to this email directly or view it on GitHub
#1570 (comment).

Best Regards,

Zhang Gengyuan

[sa2ajj]

As we agreed, this particular change is eight specific.