588756 - Use full-format json pushlog for hg poller

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P3)

Description

[Chris AtLee [:catlee]]

I think it'll be faster, and it includes branch information.

Comment 1

[Chris AtLee [:catlee]]

Attachment: Use full json pushlog, and ignore off-default branch pushes

Comment 2

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 468922 [details] [diff] [review]
Use full json pushlog, and ignore off-default branch pushes

The code changes look OK but ...

>diff --git a/misc.py b/misc.py
>@@ -553,17 +551,17 @@ def generateBranchObjects(config, name):
>             binaryURL=l10n_binaryURL
>         ))
> 
>     # change sources - if try is enabled, tipsOnly will be true which  makes 
>     # every push only show up as one changeset
>     branchObjects['change_source'].append(HgPoller(
>         hgURL=config['hgurl'],
>         branch=config['repo_path'],
>-        pushlogUrlOverride='%s/%s/pushlog' % (config['hgurl'],
>+        pushlogUrlOverride='%s/%s/json-pushes?full=1' % (config['hgurl'],
>                                               config['repo_path']),

pushlogUrlOverride looks like it can be removed based on my reading of the the poller class and testSimpleUrl. Same for other places we set in this patch. 

>diff --git a/test/test_hgpoller.py b/test/test_hgpoller.py
>+    def testOverrideUrl(self):
>+        correctUrl = 'http://hg.mozilla.org/mozilla-central/json-pushes?full=1&fromchange=123456'
>+        poller = BaseHgPoller(hgURL='http://hg.mozilla.org', branch='mozilla-central',
>+            pushlogUrlOverride='http://hg.mozilla.org/mozilla-central/json-pushes?full=1')
>+        poller.lastChangeset = '123456'
>+        url = poller._make_url()
>+        self.failUnlessEqual(url, correctUrl)

I think you should set pushlogUrlOverride to something else, otherwise you can't tell the difference from the default value.

> class PushlogParsing(unittest.TestCase):
...
>+        self.failUnlessEqual(changes[0]['changeset'], 'ee6fb954cbc3de0f76e84cad6bdff452116e1b03')
>+        self.failUnlessEqual(changes[0]['updated'], 1282362551)
>+        self.failUnlessEqual(len(changes[0]['files']), 7)

If changes are in chronologically increasing order shouldn't this be 4c23e51a484f077ea27af3ea4a4ee13da5aeb5e6, 1282358416, and 4 respectively ? Plus fallout for the checks on the other two changes. 

>diff --git a/test/utils.py b/test/utils.py
>-from buildbot.broken_test.runutils import RunMixin
> 
>-class BuildTestCase(RunMixin, unittest.TestCase):

Did this get deprecated ?

Comment 3

[Chris AtLee [:catlee]]

Attachment: Use full json pushlog, and ignore off-default branch pushes

Comment 4

[Chris AtLee [:catlee]]

> >diff --git a/misc.py b/misc.py
> >@@ -553,17 +551,17 @@ def generateBranchObjects(config, name):
> >             binaryURL=l10n_binaryURL
> >         ))
> > 
> >     # change sources - if try is enabled, tipsOnly will be true which  makes 
> >     # every push only show up as one changeset
> >     branchObjects['change_source'].append(HgPoller(
> >         hgURL=config['hgurl'],
> >         branch=config['repo_path'],
> >-        pushlogUrlOverride='%s/%s/pushlog' % (config['hgurl'],
> >+        pushlogUrlOverride='%s/%s/json-pushes?full=1' % (config['hgurl'],
> >                                               config['repo_path']),
> 
> pushlogUrlOverride looks like it can be removed based on my reading of the the
> poller class and testSimpleUrl. Same for other places we set in this patch. 

I think you're right.  I've removed all but one case, where I think it might be needed (in generateCCBranchObjects)

> 
> >diff --git a/test/test_hgpoller.py b/test/test_hgpoller.py
> >+    def testOverrideUrl(self):
> >+        correctUrl = 'http://hg.mozilla.org/mozilla-central/json-pushes?full=1&fromchange=123456'
> >+        poller = BaseHgPoller(hgURL='http://hg.mozilla.org', branch='mozilla-central',
> >+            pushlogUrlOverride='http://hg.mozilla.org/mozilla-central/json-pushes?full=1')
> >+        poller.lastChangeset = '123456'
> >+        url = poller._make_url()
> >+        self.failUnlessEqual(url, correctUrl)
> 
> I think you should set pushlogUrlOverride to something else, otherwise you
> can't tell the difference from the default value.

Done

> > class PushlogParsing(unittest.TestCase):
> ...
> >+        self.failUnlessEqual(changes[0]['changeset'], 'ee6fb954cbc3de0f76e84cad6bdff452116e1b03')
> >+        self.failUnlessEqual(changes[0]['updated'], 1282362551)
> >+        self.failUnlessEqual(len(changes[0]['files']), 7)
> 
> If changes are in chronologically increasing order shouldn't this be
> 4c23e51a484f077ea27af3ea4a4ee13da5aeb5e6, 1282358416, and 4 respectively ? Plus
> fallout for the checks on the other two changes. 

I failed to re-run the tests after fixing up the change sorting in the code.  The tests are now fixed.

> >diff --git a/test/utils.py b/test/utils.py
> >-from buildbot.broken_test.runutils import RunMixin
> > 
> >-class BuildTestCase(RunMixin, unittest.TestCase):
> 
> Did this get deprecated ?

Yes.

Comment 5

[Rail Aliiev [:rail]]

Looks like it'll be easier to fix bug 466931 after this patch.

Comment 6

[Chris AtLee [:catlee]]

Comment on attachment 469065 [details] [diff] [review]
Use full json pushlog, and ignore off-default branch pushes

changeset:   903:28f430e413ad