webkit-patch: Make attachment commands work with status-server hosted attachments
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 26 Jun 2018 20:54:50 +0000 (20:54 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 26 Jun 2018 20:54:50 +0000 (20:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187056

Reviewed by Per Arne Vollan.

Allow the EWS bots to apply, build, test, check-style, and (in the future) land
attachments hosted on the status server. We only download an attachment from the
status server if we do not have sufficient permission to download it from Bugzilla
(e.g. security-sensitive patches).

A valid status server API key is required to run these commands by hand. Otherwise,
the status server will not provide attachment data.

* Scripts/webkitpy/common/net/statusserver_mock.py:
(MockStatusServer.fetch_attachment): Log a message for testing purposes.
* Scripts/webkitpy/tool/commands/download.py:
(ProcessAttachmentsMixin._fetch_list_of_patches_to_process): Fetch the attachment
from the status server if we do not have permission to fetch it from Bugzilla.
* Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py:
(EarlyWarningSystemTest._default_expected_logs): Update expected result when
using a custom work item and when fetching an attachment from the status server.
(_test_ews): Modified to take use_security_sensitive_patch (defaults to False) as
to whether to use a security-sensitive patch when testing.
(test_ewses_with_security_sensitive_patch): Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@233219 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/Scripts/webkitpy/common/net/statusserver_mock.py
Tools/Scripts/webkitpy/tool/commands/download.py
Tools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py

index ed0c1a8..d544b86 100644 (file)
@@ -1,5 +1,32 @@
 2018-06-26  Daniel Bates  <dabates@apple.com>
 
+        webkit-patch: Make attachment commands work with status-server hosted attachments
+        https://bugs.webkit.org/show_bug.cgi?id=187056
+
+        Reviewed by Per Arne Vollan.
+
+        Allow the EWS bots to apply, build, test, check-style, and (in the future) land
+        attachments hosted on the status server. We only download an attachment from the
+        status server if we do not have sufficient permission to download it from Bugzilla
+        (e.g. security-sensitive patches).
+
+        A valid status server API key is required to run these commands by hand. Otherwise,
+        the status server will not provide attachment data.
+
+        * Scripts/webkitpy/common/net/statusserver_mock.py:
+        (MockStatusServer.fetch_attachment): Log a message for testing purposes.
+        * Scripts/webkitpy/tool/commands/download.py:
+        (ProcessAttachmentsMixin._fetch_list_of_patches_to_process): Fetch the attachment
+        from the status server if we do not have permission to fetch it from Bugzilla.
+        * Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py:
+        (EarlyWarningSystemTest._default_expected_logs): Update expected result when
+        using a custom work item and when fetching an attachment from the status server.
+        (_test_ews): Modified to take use_security_sensitive_patch (defaults to False) as
+        to whether to use a security-sensitive patch when testing.
+        (test_ewses_with_security_sensitive_patch): Added.
+
+2018-06-26  Daniel Bates  <dabates@apple.com>
+
         contributors.json fails to parse after r233209
 
         Removing trailing ',' that caused this file to be malformed. Also ran
index a56b7ea..8de0a51 100644 (file)
@@ -78,6 +78,7 @@ class MockStatusServer(object):
         return "http://dummy_url"
 
     def fetch_attachment(self, attachment_id):
+        _log.info('MOCK: fetch_attachment: {}'.format(attachment_id))
         attachment = Attachment({'id': 10008}, None)
         attachment.content = lambda: 'Patch'
         return attachment
index 154452d..9d89771 100644 (file)
@@ -1,5 +1,5 @@
 # Copyright (c) 2009, 2011 Google Inc. All rights reserved.
-# Copyright (c) 2009, 2017 Apple Inc. All rights reserved.
+# Copyright (c) 2009, 2017-2018 Apple Inc. All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions are
@@ -33,6 +33,7 @@ from webkitpy.tool import steps
 
 from webkitpy.common.checkout.changelog import ChangeLog
 from webkitpy.common.config import urls
+from webkitpy.common.net.bugzilla import Bugzilla
 from webkitpy.common.system.executive import ScriptError
 from webkitpy.tool.commands.abstractsequencedcommand import AbstractSequencedCommand
 from webkitpy.tool.commands.stepsequence import StepSequence
@@ -217,7 +218,16 @@ class AbstractPatchSequencingCommand(AbstractPatchProcessingCommand):
 
 class ProcessAttachmentsMixin(object):
     def _fetch_list_of_patches_to_process(self, options, args, tool):
-        return filter(None, map(lambda patch_id: tool.bugs.fetch_attachment(patch_id), args))
+        patches = []
+        for patch_id in args:
+            try:
+                patch = tool.bugs.fetch_attachment(patch_id, throw_on_access_error=True)
+            except Bugzilla.AccessError as e:
+                if e.error_code == Bugzilla.AccessError.NOT_PERMITTED:
+                    patch = tool.status_server.fetch_attachment(patch_id)
+            if patch:
+                patches.append(patch)
+        return patches
 
 
 class ProcessBugsMixin(object):
index 24460aa..bb2b071 100644 (file)
@@ -122,43 +122,50 @@ class MockAbstractEarlyWarningSystemForInconclusiveJSCResults(AbstractEarlyWarni
 
 
 class EarlyWarningSystemTest(QueuesTest):
-    def _default_expected_logs(self, ews, conclusive):
+    def _default_expected_logs(self, ews, conclusive, work_item, will_fetch_from_status_server=False):
         string_replacements = {
             "name": ews.name,
             "port": ews.port_name,
             "architecture": " --architecture=%s" % ews.architecture if ews.architecture else "",
             "build_style": ews.build_style(),
             "group": ews.group(),
+            'patch_id': work_item.id() if work_item else QueuesTest.mock_work_item.id(),
         }
 
+        if will_fetch_from_status_server:
+            status_server_fetch_line = 'MOCK: fetch_attachment: %(patch_id)s\n' % string_replacements
+        else:
+            status_server_fetch_line = ''
+        string_replacements['status_server_fetch_line'] = status_server_fetch_line
+
         if ews.should_build:
-            build_line = "Running: webkit-patch --status-host=example.com build --no-clean --no-update --build-style=%(build_style)s --group=%(group)s --port=%(port)s%(architecture)s\nMOCK: update_status: %(name)s Built patch\n" % string_replacements
+            build_line = "%(status_server_fetch_line)sRunning: webkit-patch --status-host=example.com build --no-clean --no-update --build-style=%(build_style)s --group=%(group)s --port=%(port)s%(architecture)s\nMOCK: update_status: %(name)s Built patch\n" % string_replacements
         else:
             build_line = ""
         string_replacements['build_line'] = build_line
 
         if ews.run_tests:
-            run_tests_line = "Running: webkit-patch --status-host=example.com build-and-test --no-clean --no-update --test --non-interactive --build-style=%(build_style)s --group=%(group)s --port=%(port)s%(architecture)s\nMOCK: update_status: %(name)s Passed tests\n" % string_replacements
+            run_tests_line = "%(status_server_fetch_line)sRunning: webkit-patch --status-host=example.com build-and-test --no-clean --no-update --test --non-interactive --build-style=%(build_style)s --group=%(group)s --port=%(port)s%(architecture)s\nMOCK: update_status: %(name)s Passed tests\n" % string_replacements
         else:
             run_tests_line = ""
         string_replacements['run_tests_line'] = run_tests_line
 
         if conclusive:
-            result_lines = "MOCK: update_status: %(name)s Pass\nMOCK: release_work_item: %(name)s 10000\n" % string_replacements
+            result_lines = "MOCK: update_status: %(name)s Pass\nMOCK: release_work_item: %(name)s %(patch_id)s\n" % string_replacements
         else:
-            result_lines = "MOCK: release_lock: %(name)s 10000\n" % string_replacements
+            result_lines = "MOCK: release_lock: %(name)s %(patch_id)s\n" % string_replacements
         string_replacements['result_lines'] = result_lines
 
         expected_logs = {
             "begin_work_queue": self._default_begin_work_queue_logs(ews.name),
             "process_work_item": """MOCK: update_status: %(name)s Started processing patch
-Running: webkit-patch --status-host=example.com clean --port=%(port)s%(architecture)s
+%(status_server_fetch_line)sRunning: webkit-patch --status-host=example.com clean --port=%(port)s%(architecture)s
 MOCK: update_status: %(name)s Cleaned working directory
-Running: webkit-patch --status-host=example.com update --port=%(port)s%(architecture)s
+%(status_server_fetch_line)sRunning: webkit-patch --status-host=example.com update --port=%(port)s%(architecture)s
 MOCK: update_status: %(name)s Updated working directory
-Running: webkit-patch --status-host=example.com apply-attachment --no-update --non-interactive 10000 --port=%(port)s%(architecture)s
+%(status_server_fetch_line)sRunning: webkit-patch --status-host=example.com apply-attachment --no-update --non-interactive %(patch_id)s --port=%(port)s%(architecture)s
 MOCK: update_status: %(name)s Applied patch
-Running: webkit-patch --status-host=example.com check-patch-relevance --quiet --group=%(group)s --port=%(port)s%(architecture)s
+%(status_server_fetch_line)sRunning: webkit-patch --status-host=example.com check-patch-relevance --quiet --group=%(group)s --port=%(port)s%(architecture)s
 MOCK: update_status: %(name)s Checked relevance of patch
 %(build_line)s%(run_tests_line)s%(result_lines)s""" % string_replacements,
             "handle_unexpected_error": "Mock error message\n",
@@ -166,13 +173,14 @@ MOCK: update_status: %(name)s Checked relevance of patch
         }
         return expected_logs
 
-    def _test_ews(self, ews, results_are_conclusive=True):
+    def _test_ews(self, ews, results_are_conclusive=True, use_security_sensitive_patch=False):
         ews.bind_to_tool(MockTool())
         ews.host = MockHost()
         options = Mock()
         options.port = None
         options.run_tests = ews.run_tests
-        self.assert_queue_outputs(ews, expected_logs=self._default_expected_logs(ews, results_are_conclusive), options=options)
+        work_item = MockTool().bugs.fetch_attachment(10008) if use_security_sensitive_patch else None
+        self.assert_queue_outputs(ews, work_item=work_item, expected_logs=self._default_expected_logs(ews, results_are_conclusive, work_item, will_fetch_from_status_server=bool(use_security_sensitive_patch)), options=options)
 
     def test_ewses(self):
         classes = AbstractEarlyWarningSystem.load_ews_classes()
@@ -181,6 +189,13 @@ MOCK: update_status: %(name)s Checked relevance of patch
         for ews_class in classes:
             self._test_ews(ews_class())
 
+    def test_ewses_with_security_sensitive_patch(self):
+        classes = AbstractEarlyWarningSystem.load_ews_classes()
+        self.assertTrue(classes)
+        self.maxDiff = None
+        for ews_class in classes:
+            self._test_ews(ews_class(), use_security_sensitive_patch=True)
+
     def test_inconclusive_jsc_test_results(self):
         classes = MockAbstractEarlyWarningSystemForInconclusiveJSCResults.load_ews_classes()
         self.assertTrue(classes)