Commit 7faa001a authored by Jeff Carpenter's avatar Jeff Carpenter Committed by Commit Bot

[WPT Export] Favor Change-Id over Commit-Position when finding PRs

From time to time the exporter is unable to find and merge an upstream PR
once its been landed. This is caused by the exporter first searching by
Commit-Position, which in the case of a landed commit does not correspond
to what's on the PR. However, this can lead to false positives where the
Commit-Position in a commit points to another, unrelated upstream PR. This
change fixes that problem by first looking for PRs by Change-Id, which never
changes.

Bug: 745880
Change-Id: I36b12a486fcfce9190ef8d2a9320b723b359ba9b
Reviewed-on: https://chromium-review.googlesource.com/575785
Commit-Queue: Jeff Carpenter <jeffcarp@chromium.org>
Reviewed-by: default avatarQuinten Yearsley <qyearsley@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487646}
parent cc908983
...@@ -253,14 +253,14 @@ class TestExporter(object): ...@@ -253,14 +253,14 @@ class TestExporter(object):
Returns the pull_request if found, else returns None. Returns the pull_request if found, else returns None.
""" """
# Check for PRs created by commits on master.
pull_request = self.wpt_github.pr_with_position(exportable_commit.position)
if pull_request:
return pull_request
# Check for PRs created by open Gerrit CLs. # Check for PRs created by open Gerrit CLs.
change_id = exportable_commit.change_id() change_id = exportable_commit.change_id()
if change_id: if change_id:
return self.wpt_github.pr_with_change_id(change_id) return self.wpt_github.pr_with_change_id(change_id)
# Check for PRs created by commits on master.
pull_request = self.wpt_github.pr_with_position(exportable_commit.position)
if pull_request:
return pull_request
return None return None
...@@ -80,15 +80,12 @@ class TestExporterTest(unittest.TestCase): ...@@ -80,15 +80,12 @@ class TestExporterTest(unittest.TestCase):
'pr_with_change_id', 'pr_with_change_id',
'pr_with_change_id', 'pr_with_change_id',
'pr_with_change_id', 'pr_with_change_id',
'pr_with_position',
'pr_with_change_id', 'pr_with_change_id',
'create_pr', 'create_pr',
'add_label "chromium-export"', 'add_label "chromium-export"',
'pr_with_position',
'pr_with_change_id', 'pr_with_change_id',
'create_pr', 'create_pr',
'add_label "chromium-export"', 'add_label "chromium-export"',
'pr_with_position',
'pr_with_change_id', 'pr_with_change_id',
'create_pr', 'create_pr',
'add_label "chromium-export"', 'add_label "chromium-export"',
...@@ -294,7 +291,6 @@ class TestExporterTest(unittest.TestCase): ...@@ -294,7 +291,6 @@ class TestExporterTest(unittest.TestCase):
test_exporter.run() test_exporter.run()
self.assertEqual(test_exporter.wpt_github.calls, [ self.assertEqual(test_exporter.wpt_github.calls, [
'pr_with_position',
'pr_with_change_id', 'pr_with_change_id',
'remove_label "do not merge yet"', 'remove_label "do not merge yet"',
'get_pr_branch', 'get_pr_branch',
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment