Merge pull request #5827 from p12tic/trigger-fix-crash-patch-body

Relax a too-eager assertion of sourcestamp properties
buildbot · Mar 1, 2021 · 9327f70 · 9327f70
2 parents 6984bd2 + 4392d9e
commit 9327f70
Show file tree

Hide file tree

Showing 3 changed files with 101 additions and 5 deletions.
diff --git a/master/buildbot/newsfragments/trigger-step-patch-body.bugfix b/master/buildbot/newsfragments/trigger-step-patch-body.bugfix
@@ -0,0 +1 @@
+Fixed a crash when a trigger step is used in a build with patch body data passed via the try scheduler (:issue:`5165`).
diff --git a/master/buildbot/process/buildrequest.py b/master/buildbot/process/buildrequest.py
@@ -98,6 +98,14 @@ class TempSourceStamp:
 
     ATTRS = ('branch', 'revision', 'repository', 'project', 'codebase')
 
+    PATCH_ATTRS = (
+        ('patch_level', 'level'),
+        ('patch_body', 'body'),
+        ('patch_subdir', 'subdir'),
+        ('patch_author', 'author'),
+        ('patch_comment', 'comment')
+    )
+
     def __init__(self, ssdict):
         self._ssdict = ssdict
 
@@ -118,8 +126,6 @@ def __getattr__(self, attr):
     def asSSDict(self):
         return self._ssdict
 
-    PATCH_ATTRS = ('level', 'body', 'subdir', 'author', 'comment')
-
     def asDict(self):
         # This return value should match the kwargs to
         # SourceStampsConnectorComponent.findSourceStampId
@@ -128,11 +134,11 @@ def asDict(self):
             result[attr] = self._ssdict.get(attr)
 
         patch = self._ssdict.get('patch') or {}
-        for attr in self.PATCH_ATTRS:
-            result['patch_{}'.format(attr)] = patch.get(attr)
+        for patch_attr, attr in self.PATCH_ATTRS:
+            result[patch_attr] = patch.get(attr)
 
         assert all(
-            isinstance(val, (str, int, type(None)))
+            isinstance(val, (str, int, bytes, type(None)))
             for attr, val in result.items()
         ), result
         return result

diff --git a/master/buildbot/test/unit/process/test_buildrequest.py b/master/buildbot/test/unit/process/test_buildrequest.py
@@ -13,6 +13,8 @@
 #
 # Copyright Buildbot Team Members
 
+import datetime
+
 import mock
 
 from twisted.internet import defer
@@ -287,6 +289,93 @@ def collapseRequests_fn(master, builder, brdict1, brdict2):
         yield self.do_request_collapse(rows, [21], [20])
 
 
+class TestSourceStamp(unittest.TestCase):
+    def test_asdict_minimal(self):
+        ssdatadict = {
+            'ssid': '123',
+            'branch': None,
+            'revision': None,
+            'patch': None,
+            'repository': 'testrepo',
+            'codebase': 'testcodebase',
+            'project': 'testproject',
+            'created_at': datetime.datetime(2019, 4, 1, 23, 38, 33, 154354),
+        }
+        ss = buildrequest.TempSourceStamp(ssdatadict)
+
+        self.assertEqual(ss.asDict(), {
+            'branch': None,
+            'codebase': 'testcodebase',
+            'patch_author': None,
+            'patch_body': None,
+            'patch_comment': None,
+            'patch_level': None,
+            'patch_subdir': None,
+            'project': 'testproject',
+            'repository': 'testrepo',
+            'revision': None
+        })
+
+    def test_asdict_no_patch(self):
+        ssdatadict = {
+            'ssid': '123',
+            'branch': 'testbranch',
+            'revision': 'testrev',
+            'patch': None,
+            'repository': 'testrepo',
+            'codebase': 'testcodebase',
+            'project': 'testproject',
+            'created_at': datetime.datetime(2019, 4, 1, 23, 38, 33, 154354),
+        }
+        ss = buildrequest.TempSourceStamp(ssdatadict)
+
+        self.assertEqual(ss.asDict(), {
+            'branch': 'testbranch',
+            'codebase': 'testcodebase',
+            'patch_author': None,
+            'patch_body': None,
+            'patch_comment': None,
+            'patch_level': None,
+            'patch_subdir': None,
+            'project': 'testproject',
+            'repository': 'testrepo',
+            'revision': 'testrev',
+        })
+
+    def test_asdict_with_patch(self):
+        ssdatadict = {
+            'ssid': '123',
+            'branch': 'testbranch',
+            'revision': 'testrev',
+            'patch': {
+                'patchid': 1234,
+                'body': b'testbody',
+                'level': 2,
+                'author': 'testauthor',
+                'comment': 'testcomment',
+                'subdir': 'testsubdir',
+            },
+            'repository': 'testrepo',
+            'codebase': 'testcodebase',
+            'project': 'testproject',
+            'created_at': datetime.datetime(2019, 4, 1, 23, 38, 33, 154354),
+        }
+        ss = buildrequest.TempSourceStamp(ssdatadict)
+
+        self.assertEqual(ss.asDict(), {
+            'branch': 'testbranch',
+            'codebase': 'testcodebase',
+            'patch_author': 'testauthor',
+            'patch_body': b'testbody',
+            'patch_comment': 'testcomment',
+            'patch_level': 2,
+            'patch_subdir': 'testsubdir',
+            'project': 'testproject',
+            'repository': 'testrepo',
+            'revision': 'testrev'
+        })
+
+
 class TestBuildRequest(TestReactorMixin, unittest.TestCase):
 
     def setUp(self):