Skip to content

Commit

Permalink
Fix BitbucketPullrequestPoller change detection
Browse files Browse the repository at this point in the history 
Currently the BitbucketPullrequestPoller treats all pull requests as
if they have been updated, rather than filtering them properly. This is
because the existing check compares the short hash of the incoming
changeset with the full hash of the current revision.

This patch changes the behaviour to compare the short hashes of the
incoming and current revisions.

The Bitbucket API returns short hashes for a number of queries. The
test data has been updated to reflect this.

fixes #3597

Signed-off-by: Sam Bristow <sam.bristow@gmail.com>

Explicitly use short hashes when comparing PR revisions

This makes the version comparision more robust in case Atlassian changes the
Bitbucket PR API response to return full hashes in future.

Signed-off-by: Sam Bristow <sam.bristow@gmail.com>
  • Loading branch information
@sam-bristow @tardyp
sam-bristow authored and tardyp committed Aug 23, 2016
1 parent eeb683c commit b997f06
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 20 deletions.
3 changes: 2 additions & 1 deletion master/buildbot/changes/bitbucket.py
View file
Expand Up @@ -102,7 +102,8 @@ def _processChanges(self, page):
if not self.branch or branch in self.branch:
current = yield self._getCurrentRev(nr)

if not current or current != revision:
# compare _short_ hashes to check if the PR has been updated
if not current or current[0:12] != revision[0:12]:
# parse pull request api page (required for the filter)
page = yield client.getPage(str(pr['links']['self']['href']))
pr_json = json.loads(page, encoding=self.encoding)
Expand Down
45 changes: 26 additions & 19 deletions master/buildbot/test/unit/test_changes_bitbucket.py
View file
Expand Up @@ -26,13 +26,14 @@


class SourceRest():
"""https://bitbucket.org/!api/2.0/repositories/{owner}/{slug}"""
template = """\
{
"hash": "%(hash)s",
"links": {
"html": {
"href": "https://bitbucket.org/%(owner)s/%(slug)s/commits/%(hash)s"
"href": "https://bitbucket.org/%(owner)s/%(slug)s/commits/%(short_hash)s"
}
},
"repository": {
Expand Down Expand Up @@ -67,6 +68,7 @@ def request(self):
"owner": self.owner,
"slug": self.slug,
"hash": self.hash,
"short_hash": self.hash[0:12],
"date": self.date,
}

Expand All @@ -78,6 +80,7 @@ def repo_request(self):


class PullRequestRest():
"""https://bitbucket.org/!api/2.0/repositories/{owner}/{slug}/pullrequests/{pull_request_id}"""
template = """\
{
Expand Down Expand Up @@ -124,6 +127,7 @@ def request(self):
"description": self.description,
"title": self.title,
"hash": self.source.hash,
"short_hash": self.source.hash[0:12],
"owner": self.source.owner,
"slug": self.source.slug,
"display_name": self.display_name,
Expand All @@ -134,6 +138,7 @@ def request(self):


class PullRequestListRest():
"""https://bitbucket.org/api/2.0/repositories/{owner}/{slug}/pullrequests"""
template = """\
{
"description": "%(description)s",
Expand All @@ -151,10 +156,10 @@ class PullRequestListRest():
"title": "%(title)s",
"source": {
"commit": {
"hash": "%(hash)s",
"hash": "%(short_hash)s",
"links": {
"self": {
"href": "https://bitbucket.org/!api/2.0/repositories/%(src_owner)s/%(src_slug)s/commit/%(hash)s"
"href": "https://bitbucket.org/!api/2.0/repositories/%(src_owner)s/%(src_slug)s/commit/%(short_hash)s"
}
}
},
Expand Down Expand Up @@ -200,6 +205,7 @@ def request(self):
"display_name": pr.display_name,
"title": pr.title,
"hash": pr.source.hash,
"short_hash": pr.source.hash[0:12],
"src_owner": pr.source.owner,
"src_slug": pr.source.slug,
"created_on": pr.created_on,
Expand Down Expand Up @@ -260,7 +266,7 @@ def setUp(self):
src = SourceRest(
owner="contributor",
slug="slug",
hash="000000000000000000000000000001",
hash="1111111111111111111111111111111111111111",
date=self.date,
)
pr = PullRequestRest(
Expand All @@ -280,7 +286,7 @@ def setUp(self):
src = SourceRest(
owner="contributor",
slug="slug",
hash="000000000000000000000000000002",
hash="2222222222222222222222222222222222222222",
date=self.date,
)
pr = PullRequestRest(
Expand Down Expand Up @@ -367,8 +373,8 @@ def test_poll_new_pull_requests(self):
'project': u'',
'properties': {},
'repository': u'https://bitbucket.org/contributor/slug',
'revision': u'000000000000000000000000000001',
'revlink': u'https://bitbucket.org/contributor/slug/commits/000000000000000000000000000001',
'revision': u'1111111111111111111111111111111111111111',
'revlink': u'https://bitbucket.org/contributor/slug/commits/111111111111',
'src': u'bitbucket',
'when_timestamp': 1381869500,
}])
Expand All @@ -392,8 +398,8 @@ def test_poll_no_updated_pull_request(self):
'project': u'',
'properties': {},
'repository': u'https://bitbucket.org/contributor/slug',
'revision': u'000000000000000000000000000001',
'revlink': u'https://bitbucket.org/contributor/slug/commits/000000000000000000000000000001',
'revision': u'1111111111111111111111111111111111111111',
'revlink': u'https://bitbucket.org/contributor/slug/commits/111111111111',
'src': u'bitbucket',
'when_timestamp': 1381869500,
}])
Expand All @@ -420,8 +426,9 @@ def test_poll_updated_pull_request(self):
'project': u'',
'properties': {},
'repository': u'https://bitbucket.org/contributor/slug',
'revision': u'000000000000000000000000000001',
'revlink': u'https://bitbucket.org/contributor/slug/commits/000000000000000000000000000001',

'revision': u'1111111111111111111111111111111111111111',
'revlink': u'https://bitbucket.org/contributor/slug/commits/111111111111',
'src': u'bitbucket',
'when_timestamp': 1381869500,
}])
Expand All @@ -439,8 +446,8 @@ def test_poll_updated_pull_request(self):
'project': u'',
'properties': {},
'repository': u'https://bitbucket.org/contributor/slug',
'revision': u'000000000000000000000000000001',
'revlink': u'https://bitbucket.org/contributor/slug/commits/000000000000000000000000000001',
'revision': u'1111111111111111111111111111111111111111',
'revlink': u'https://bitbucket.org/contributor/slug/commits/111111111111',
'src': u'bitbucket',
'when_timestamp': 1381869500,
},
Expand All @@ -454,8 +461,8 @@ def test_poll_updated_pull_request(self):
'project': u'',
'properties': {},
'repository': u'https://bitbucket.org/contributor/slug',
'revision': u'000000000000000000000000000002',
'revlink': u'https://bitbucket.org/contributor/slug/commits/000000000000000000000000000002',
'revision': u'2222222222222222222222222222222222222222',
'revlink': u'https://bitbucket.org/contributor/slug/commits/222222222222',
'src': u'bitbucket',
'when_timestamp': 1381869500,
}
Expand Down Expand Up @@ -499,8 +506,8 @@ def test_poll_pull_request_filter_True(self):
'project': u'',
'properties': {},
'repository': u'https://bitbucket.org/contributor/slug',
'revision': u'000000000000000000000000000001',
'revlink': u'https://bitbucket.org/contributor/slug/commits/000000000000000000000000000001',
'revision': u'1111111111111111111111111111111111111111',
'revlink': u'https://bitbucket.org/contributor/slug/commits/111111111111',
'src': u'bitbucket',
'when_timestamp': 1381869500,
}])
Expand Down Expand Up @@ -528,8 +535,8 @@ def test_poll_pull_request_not_useTimestamps(self):
'project': u'',
'properties': {},
'repository': u'https://bitbucket.org/contributor/slug',
'revision': u'000000000000000000000000000001',
'revlink': u'https://bitbucket.org/contributor/slug/commits/000000000000000000000000000001',
'revision': u'1111111111111111111111111111111111111111',
'revlink': u'https://bitbucket.org/contributor/slug/commits/111111111111',
'src': u'bitbucket',
'when_timestamp': 1396825656,
}])

0 comments on commit b997f06

Please sign in to comment.