Bug 1655524 [wpt PR 24768] - [wptpr.live] Make sure to clean up old refs/prs-open/{pr}, a=testonly

Automatic update from web-platform-tests
[wptpr.live] Make sure to clean up old refs/prs-open/{pr} (#24768)

This expands the list of PRs that pr_preview.py looks at to include
those that have existing refs. That means that old PRs outside the
window will always have their deployments deleted, rather than being
kept forever.

Fixes https://github.com/web-platform-tests/wpt.live/issues/36
--

wpt-commits: d409a92831a5e9823d123489e606a308d4272648
wpt-pr: 24768

testing/web-platform/tests/tools/ci/pr_preview.py

@@ -103,6 +103,15 @@ class Project(object):
         self._host = host
         self._github_project = github_project
 
+    @guard('core')
+    def get_pull_request(self, number):
+        url = '{}/repos/{}/pulls/{}'.format(
+            self._host, self._github_project, number
+        )
+
+        logger.info('Fetching Pull Request %s', number)
+        return gh_request('GET', url)
+
     @guard('search')
     def get_pull_requests(self, updated_since):
         window_start = time.strftime('%Y-%m-%dT%H:%M:%SZ', updated_since)
@@ -121,20 +130,18 @@ class Project(object):
         if data['incomplete_results']:
             raise Exception('Incomplete results')
 
-        return data['items']
+        return {pr['number']: pr for pr in data['items']}
 
-    @guard('core')
     def pull_request_is_from_fork(self, pull_request):
         pr_number = pull_request['number']
-        url = '{}/repos/{}/pulls/{}'.format(
-            self._host, self._github_project, pr_number
-        )
-
         logger.info('Checking if pull request %s is from a fork', pr_number)
 
-        data = gh_request('GET', url)
+        # We may already have this information; if so no need to make another
+        # API call.
+        if 'head' not in pull_request:
+            pull_request = self.get_pull_request(pr_number)
 
-        repo_name = data['head']['repo']['full_name']
+        repo_name = pull_request['head']['repo']['full_name']
         is_fork = repo_name != self._github_project
 
         logger.info(
@@ -222,13 +229,17 @@ class Remote(object):
         # for pushing changes.
         self._token = os.environ['DEPLOY_TOKEN']
 
-    def get_revision(self, refspec):
-        output = subprocess.check_output([
+    def _git(self, command):
+        return subprocess.check_output([
             'git',
             '-c',
             'credential.username={}'.format(self._token),
             '-c',
             'core.askPass=true',
+        ] + command)
+
+    def get_revision(self, refspec):
+        output = self._git([
             'ls-remote',
             'origin',
             'refs/{}'.format(refspec)
@@ -244,18 +255,40 @@ class Remote(object):
 
         logger.info('Deleting ref "%s"', refspec)
 
-        subprocess.check_call([
-            'git',
-            '-c',
-            'credential.username={}'.format(self._token),
-            '-c',
-            'core.askPass=true',
+        self._git([
             'push',
             'origin',
             '--delete',
             full_ref
         ])
 
+    def get_pull_requests_with_open_ref(self):
+        '''Returns pull requests that have an open ref.
+
+        This checks for all refs that match the pattern 'refs/prs-open/{pr}',
+        and then returns them as a list of pull request numbers. Note that this
+        method does not query github; this is only the open pull requests as
+        far as WPT knows.
+        '''
+
+        refspec = 'refs/prs-open/*'
+
+        logger.info('Fetching all ref names matching "%s"', refspec)
+
+        output = self._git([
+            'ls-remote',
+            'origin',
+            refspec,
+        ])
+
+        if not output:
+            return []
+
+        ref_names = [line.split()[1] for line in output.decode('utf-8').splitlines()]
+        logger.info('%s ref names found', len(ref_names))
+        return [int(ref_name.split('/')[2]) for ref_name in ref_names]
+
+
 def is_open(pull_request):
     return not pull_request['closed_at']
 
@@ -301,7 +334,19 @@ def synchronize(host, github_project, window):
         time.gmtime(time.time() - window)
     )
 
-    for pull_request in pull_requests:
+    # It is possible we may miss some pull requests if this script breaks or is
+    # not run for a while and the PR falls outside the checked window. To
+    # ensure that closed pull requests are deleted, extend the list of pull
+    # requests to look at with any that have an existing refs/prs-open/{pr} ref
+    # in the repo.
+    existing_pr_numbers = remote.get_pull_requests_with_open_ref()
+    for pr_number in existing_pr_numbers:
+        if pr_number in pull_requests:
+            continue
+        pull_request = project.get_pull_request(pr_number)
+        pull_requests[pull_request['number']] = pull_request
+
+    for pull_request in pull_requests.values():
         logger.info('Processing Pull Request #%(number)d', pull_request)
 
         refspec_trusted = 'prs-trusted-for-preview/{number}'.format(

testing/web-platform/tests/tools/ci/tests/test_pr_preview.py

@@ -710,6 +710,44 @@ def test_synchronize_delete_collaborator():
     assert same_members(expected_traffic, actual_traffic)
     assert list(remote_refs) == ['refs/pull/23/head']
 
+
+def test_synchronize_delete_old_pr():
+    # No pull requests from the search, but one outstanding closed PR.
+    expected_traffic = [
+        (Requests.get_rate, Responses.no_limit),
+        (Requests.get_rate, Responses.no_limit),
+        (Requests.search, (200,
+            {
+                'items': [],
+                'incomplete_results': False
+            }
+        )),
+        (Requests.pr_details, (200,
+            {
+                'number': 23,
+                'labels': [],
+                'closed_at': '2019-10-28',
+                'head': {
+                    'repo': {
+                        'full_name': 'test-org/test-repo'
+                    }
+                }
+            }
+        )),
+    ]
+    refs = {
+        'refs/pull/23/head': 'HEAD',
+        'refs/prs-open/23': 'HEAD~',
+        'refs/prs-trusted-for-preview/23': 'HEAD~'
+    }
+
+    returncode, actual_traffic, remote_refs = synchronize(expected_traffic, refs)
+
+    assert returncode == 0
+    assert same_members(expected_traffic, actual_traffic)
+    assert list(remote_refs) == ['refs/pull/23/head']
+
+
 def test_detect_ignore_unknown_env():
     expected_github_traffic = []
     expected_preview_traffic = []