Attachment 342905 Details for Bug 186748 – Patch and unit tests

[patch] Patch and unit tests

bug-186748-20180616222940.patch (text/plain), 6.90 KB, created by Daniel Bates on 2018-06-16 22:29:41 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Daniel Bates

Created: 2018-06-16 22:29:41 PDT

Size: 6.90 KB

patch

obsolete

>Subversion Revision: 232910
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 8eb5a2b5864617045972bab24772dd15ad60fb46..d4da28349d592393ae47611afbeefa82650d0013 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,35 @@
>+2018-06-16  Daniel Bates  <dabates@apple.com>
>+
>+        QueueStatusServer: "'NoneType' object has no attribute 'message'" in ReleasePatch.get()
>+        when attachment is skipped by queue
>+        https://bugs.webkit.org/show_bug.cgi?id=186748
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Fixes an issue where releasing a patch that was skipped by a queue would cause an AttributeError
>+        in ReleasePatch.get().
>+
>+        When removing a patch from a queue we update queue statistics. As part of this process
>+        we record the last message posted to the status server for the patch. Currently if a patch
>+        is skipped by the queue when picking the next patch to process (say, we failed to fetch the
>+        attachment) then the queue does not post a message to the status server before it asks the
>+        status server to remove the patch from the list of patches the queue needs to process. Instead
>+        the queue should tell the status server that it chose to skip the patch before asking for the
>+        patch to be removed from its list of patches to process.
>+
>+        * Scripts/webkitpy/common/net/bugzilla/attachment.py:
>+        (Attachment.bug_id): Return None if we do not have a bug ID. This can happen if we failed
>+        to fetch the attachment from Bugzilla (say, Bugzilla's markup changed) or we do not have
>+        have permission to view the Bugzilla bug.
>+        * Scripts/webkitpy/common/net/bugzilla/attachment_unittest.py: Added.
>+        (AttachmentTest):
>+        (AttachmentTest.test_no_bug_id):
>+        * Scripts/webkitpy/tool/commands/queues.py:
>+        (AbstractPatchQueue._next_patch): Call AbstractPatchQueue._did_skip() to post a status
>+        update to the status server and then release the work item.
>+        * Scripts/webkitpy/tool/commands/queues_unittest.py:
>+        (AbstractPatchQueueTest.test_next_patch): Update expected result.
>+
> 2018-06-16  Leo Balter  <leonardo.balter@gmail.com>
> 
>         [test262-runner] Handle items from the config list with inline comments
>diff --git a/Tools/Scripts/webkitpy/common/net/bugzilla/attachment.py b/Tools/Scripts/webkitpy/common/net/bugzilla/attachment.py
>index c749a1512d76af7313546c732f7ca715f0bccd86..d209e1479ec355d50ab31dff7d9454c099c4e161 100644
>--- a/Tools/Scripts/webkitpy/common/net/bugzilla/attachment.py
>+++ b/Tools/Scripts/webkitpy/common/net/bugzilla/attachment.py
>@@ -63,7 +63,13 @@ class Attachment(object):
>         return self._bug
> 
>     def bug_id(self):
>-        return int(self._attachment_dictionary.get("bug_id"))
>+        bug_id_string = self._attachment_dictionary.get('bug_id')
>+        if bug_id_string:
>+            return int(bug_id_string)
>+        # We may not know the associated bug ID. This can happen if we do not have
>+        # permission to view the attachment or we failed to fetch it from Bugzilla
>+        # for some other reason (see AbstractPatchQueue._next_patch()).
>+        return None
> 
>     def is_patch(self):
>         return not not self._attachment_dictionary.get("is_patch")
>diff --git a/Tools/Scripts/webkitpy/common/net/bugzilla/attachment_unittest.py b/Tools/Scripts/webkitpy/common/net/bugzilla/attachment_unittest.py
>new file mode 100644
>index 0000000000000000000000000000000000000000..bf9a5beb9b88411905a08f65109d69abb85ba4cf
>--- /dev/null
>+++ b/Tools/Scripts/webkitpy/common/net/bugzilla/attachment_unittest.py
>@@ -0,0 +1,30 @@
>+# Copyright (C) 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 met:
>+# 1.  Redistributions of source code must retain the above copyright
>+#     notice, this list of conditions and the following disclaimer.
>+# 2.  Redistributions in binary form must reproduce the above copyright
>+#     notice, this list of conditions and the following disclaimer in the
>+#     documentation and/or other materials provided with the distribution.
>+#
>+# THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' AND
>+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
>+# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
>+# DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS BE LIABLE FOR
>+# ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
>+# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
>+# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
>+# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
>+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>+
>+import unittest
>+
>+from .attachment import Attachment
>+
>+
>+class AttachmentTest(unittest.TestCase):
>+    def test_no_bug_id(self):
>+        self.assertEqual(Attachment({'id': 12345}, None).bug_id(), None)
>diff --git a/Tools/Scripts/webkitpy/tool/commands/queues.py b/Tools/Scripts/webkitpy/tool/commands/queues.py
>index 77c2d8827bcc010df684800e6d46d3651b018f87..a8d7a16c557523af7087bdf7625b0260eb719397 100644
>--- a/Tools/Scripts/webkitpy/tool/commands/queues.py
>+++ b/Tools/Scripts/webkitpy/tool/commands/queues.py
>@@ -224,7 +224,7 @@ class AbstractPatchQueue(AbstractQueue):
>                 # mostly we just need to remove this bogus patch from our queue.
>                 # If for some reason bugzilla is just down, then it will be re-fed later.
>                 fake_patch = Attachment({'id': patch_id}, None)
>-                self._release_work_item(fake_patch)
>+                self._did_skip(fake_patch)
>         return patch
> 
>     def _release_work_item(self, patch):
>diff --git a/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py b/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py
>index 6d722e8b9a5007da8e2d6c88ea8734b9b6a42467..699621e1152716becfdc830a22fc73e9e7954b0e 100644
>--- a/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py
>+++ b/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py
>@@ -161,7 +161,9 @@ class AbstractPatchQueueTest(CommandsTest):
>         self.assertIsNone(queue._next_patch())
>         tool.status_server = MockStatusServer(work_items=[2, 10000, 10001])
>         expected_stdout = "MOCK: fetch_attachment: 2 is not a known attachment id\n"  # A mock-only message to prevent us from making mistakes.
>-        expected_logs = "MOCK: release_work_item: None 2\n"
>+        expected_logs = """MOCK: update_status: None Skip
>+MOCK: release_work_item: None 2
>+"""
>         patch = OutputCapture().assert_outputs(self, queue._next_patch, expected_stdout=expected_stdout, expected_logs=expected_logs)
>         # The patch.id() == 2 is ignored because it doesn't exist.
>         self.assertEqual(patch.id(), 10000)

Actions: View | Formatted Diff | Diff

Attachments on bug 186748: 342902 | 342905