Attachment 343625 Details for Bug 187056 – Patch and unit test

[patch] Patch and unit test

bug-187056-20180626112438.patch (text/plain), 10.71 KB, created by Daniel Bates on 2018-06-26 11:24:39 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Daniel Bates

Created: 2018-06-26 11:24:39 PDT

Size: 10.71 KB

patch

obsolete

>Subversion Revision: 233189
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index a4e4382cc6bb1c54255dc6b1fd9677ac7c1d55d4..8899e03ddb67031ba860049abe93ce0021189b92 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,30 @@
>+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 NOBODY (OOPS!).
>+
>+        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-25  Daniel Bates  <dabates@apple.com>
> 
>         Log a message when fetching attachment data from the status server
>diff --git a/Tools/Scripts/webkitpy/common/net/statusserver_mock.py b/Tools/Scripts/webkitpy/common/net/statusserver_mock.py
>index a56b7ea214b118e2e592b99a574dab8d9ebe3699..8de0a5151c0c077ae18af1faae2b58f900f79d44 100644
>--- a/Tools/Scripts/webkitpy/common/net/statusserver_mock.py
>+++ b/Tools/Scripts/webkitpy/common/net/statusserver_mock.py
>@@ -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
>diff --git a/Tools/Scripts/webkitpy/tool/commands/download.py b/Tools/Scripts/webkitpy/tool/commands/download.py
>index 154452d5513dd8babf58d71487174a7a56c24593..9d8977181e4fd894fec001ac14146452e6823961 100644
>--- a/Tools/Scripts/webkitpy/tool/commands/download.py
>+++ b/Tools/Scripts/webkitpy/tool/commands/download.py
>@@ -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):
>diff --git a/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py b/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py
>index 24460aabdf7168a06f490de07049134c24b1cbed..bb2b071901396f3add61d1b3258fa366461c7e97 100644
>--- a/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py
>+++ b/Tools/Scripts/webkitpy/tool/commands/earlywarningsystem_unittest.py
>@@ -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)

Actions: View | Formatted Diff | Diff

Attachments on bug 187056: 343625 | 343633