Simplify UpdateReadyForTesting, publish it on edits that change builds and on update creation

bodhi-messages/bodhi/messages/schemas/update.py

@@ -1028,3 +1028,118 @@ def __str__(self) -> str:
             f"{self.body['update']['user']['name']}'s Bodhi update is ready for testing\n"
             f"Builds:\n"
             f"{new_line.join([b['nvr'] for b in self.body['artifact']['builds']])} ")
+
+
+class UpdateReadyForTestingV3(UpdateMessage):
+    """
+    Sent when an update is ready to be tested. Simplified version.
+
+    Specifically, this message is sent:
+
+    * When an update is created
+    * When an update is edited and its builds change
+    * When a "re-trigger tests" request is made via the web UI or API
+
+    These are the points where we expect that automated systems will
+    test the update.
+
+    Inherits from UpdateMessage and only contains as much extra
+    information (in the 'artifact' dict) as the Fedora CI schedulers
+    actually need.
+    """
+
+    body_schema = {
+        'id': f'{SCHEMA_URL}/v1/bodhi.update.status.testing#',
+        '$schema': 'http://json-schema.org/draft-04/schema#',
+        'description': 'Schema for message sent when an update is ready for testing',
+        'type': 'object',
+        'properties': {
+            'agent': {
+                'type': 'string',
+                'description': 'Re-trigger request: name of requester, trigger on push: "bodhi".',
+            },
+            'artifact': {
+                'description': 'Details about the builds to test.',
+                'type': 'object',
+                'properties': {
+                    'type': {
+                        'description': 'Artifact type, in this case "koji-build-group".',
+                        'type': 'string',
+                    },
+                    'builds': {
+                        'type': 'array',
+                        'description': 'A list of builds included in this group',
+                        'items': {'$ref': '#/definitions/artifactbuild'}
+                    },
+                },
+                'required': ['type', 'builds'],
+            },
+            're-trigger': {
+                'type': 'boolean',
+                'description': 'This flag is True if the message is sent to re-trigger tests'
+            },
+            'update': UpdateV1.schema(),
+        },
+        'required': ['agent', 'artifact', 'update'],
+        'definitions': {
+            'artifactbuild': {
+                'description': 'Details about a build that are not in the update builds dict',
+                'type': 'object',
+                'properties': {
+                    'type': {
+                        'description': 'Artifact type, in this case "koji-build"',
+                        'type': 'string',
+                    },
+                    'id': {
+                        'description': 'Build ID of the koji build.',
+                        'type': 'integer',
+                    },
+                    'task_id': {
+                        'description': 'Task ID of the koji build.',
+                        'type': ['null', 'integer'],
+                    },
+                    'nvr': {
+                        'description': 'Name-version-release of the artifact.',
+                        'type': 'string',
+                    }
+                },
+                'required': ['type', 'id', 'task_id', 'nvr'],
+            },
+            'build': BuildV1.schema(),
+        }
+    }
+
+    topic = "bodhi.update.status.testing.koji-build-group.build.complete"
+    severity = DEBUG
+
+    @property
+    def summary(self) -> str:
+        """
+        Return a short, human-readable representation of this message.
+
+        This should provide a short summary of the message, much like the subject line
+        of an email.
+
+        Returns:
+            A summary for this message.
+        """
+        return (
+            f"{self.update.user.name}'s {self._builds_summary} "
+            f"bodhi update is ready for testing")
+
+    def __str__(self) -> str:
+        """
+        Return a human-readable representation of this message.
+
+        This should provide a detailed representation of the message, much like the body
+        of an email.
+
+        Returns:
+            A human readable representation of this message.
+        """
+        new_line = "\n"
+        return (
+            f"{self.update.user.name}'s Bodhi update {self.update.alias} "
+            f"is ready for testing\n"
+            f"Builds:\n"
+            f"{new_line.join([b.nvr for b in self.update.builds])} ")

bodhi-messages/pyproject.toml

@@ -60,6 +60,7 @@ diff-cover = ">=4.2.1"
 "bodhi.update.complete.testing.v1" = "bodhi.messages.schemas.update:UpdateCompleteTestingV1"
 "bodhi.update.status.testing.v1" = "bodhi.messages.schemas.update:UpdateReadyForTestingV1"
 "bodhi.update.status.testing.v2" = "bodhi.messages.schemas.update:UpdateReadyForTestingV2"
+"bodhi.update.status.testing.v3" = "bodhi.messages.schemas.update:UpdateReadyForTestingV3"
 "bodhi.update.edit.v1" = "bodhi.messages.schemas.update:UpdateEditV1"
 "bodhi.update.edit.v2" = "bodhi.messages.schemas.update:UpdateEditV2"
 "bodhi.update.eject.v1" = "bodhi.messages.schemas.update:UpdateEjectV1"

bodhi-messages/tests/test_update.py

@@ -27,6 +27,7 @@
     UpdateKarmaThresholdV1,
     UpdateReadyForTestingV1,
     UpdateReadyForTestingV2,
+    UpdateReadyForTestingV3,
     UpdateRequestObsoleteV1,
     UpdateRequestRevokeV1,
     UpdateRequestStableV1,
@@ -317,6 +318,60 @@ def test_ready_for_testing_v2(self):
         )
         check_message(msg, expected)
 
+    def test_ready_for_testing_v3(self):
+        expected = {
+            "topic": "bodhi.update.status.testing.koji-build-group.build.complete",
+            "summary": (
+                "plautrba's libselinux-2.8-6.fc29.x86_64 libsepol-2.… bodhi update "
+                "is ready for testing"
+            ),
+            "__str__": (
+                "plautrba's Bodhi update FEDORA-2019-d64d0caab3 is ready for testing\nBuilds:"
+                "\nlibselinux-2.8-6.fc29.x86_64\nlibsepol-2.8-3.fc29.x86_64 "
+            ),
+            "app_icon": "https://apps.fedoraproject.org/img/icons/bodhi.png",
+            "app_name": "bodhi",
+            "url": "https://bodhi.fedoraproject.org/updates/FEDORA-2019-d64d0caab3",
+            "agent_avatar": (
+                "https://seccdn.libravatar.org/avatar/"
+                "20652954adacfd9f6e26536bbcf3b5fbc850dc61f8a2e67c5bfbc6e345032976"
+                "?s=64&d=retro"
+            ),
+            "usernames": ["mohanboddu", "plautrba"],
+            "packages": ["libselinux", "libsepol"],
+        }
+        msg = UpdateReadyForTestingV3(
+            body={
+                "artifact": {
+                    "type": "koji-build-group",
+                    "builds":
+                        [{
+                            "type": "koji-build",
+                            "id": 14546275,
+                            "task_id": 14546276,
+                            "nvr": "libselinux-2.8-6.fc29.x86_64",
+                        }, {
+                            "type": "koji-build",
+                            "id": 14546278,
+                            "task_id": None,
+                            "nvr": "libsepol-2.8-3.fc29.x86_64",
+                        }],
+                },
+                "update": {
+                    "alias": "FEDORA-2019-d64d0caab3",
+                    "builds": [{"nvr": "libselinux-2.8-6.fc29.x86_64"},
+                               {"nvr": "libsepol-2.8-3.fc29.x86_64"}],
+                    "title": "flibselinux-2.8-6.fc29",
+                    'release': {"name": "F29"},
+                    'request': None,
+                    "status": "testing",
+                    "user": {"name": "plautrba"}
+                },
+                "agent": "mohanboddu",
+            }
+        )
+        check_message(msg, expected)
+
     def test_request_testing_v1_multiple(self):
         expected = {
             "topic": "bodhi.update.request.testing",

bodhi-server/bodhi/server/buildsys.py

@@ -208,7 +208,9 @@ def getBuild(self, build='TurboGears-1.0.2.2-2.fc17', other=False, testing=False
                     'package_name': 'gnome-backgrounds',
                     'release': '1.fc17',
                     'tag_name': 'f17-build-side-7777',
-                    'version': '3.0'}
+                    'version': '3.0',
+                    'id': 16061,
+                    'task_id': 15051}
 
         theid = 16058
         if other and not testing:

bodhi-server/bodhi/server/models.py

@@ -2009,13 +2009,14 @@ def __init__(self, *args, **kwargs):
         alias = '%s-%s-%s' % (prefix, year, id)
         self.alias = alias
         self.release_id = kwargs['release'].id
+        # we need this to be set for message publishing to work
+        self.status = kwargs.get('status', UpdateStatus.pending)
 
         super(Update, self).__init__(*args, **kwargs)
 
         log.debug('Set alias for %s to %s' % (self.get_title(), alias))
 
-        if self.status == UpdateStatus.testing:
-            self._ready_for_testing(self, self.status, None, None)
+        self._ready_for_testing(self, None)
 
     @property
     def version_hash(self):
@@ -2716,6 +2717,11 @@ def edit(cls, request, data):
                 'removed_builds': removed_builds
             }
         ))
+        if (new_builds or removed_builds) and up.content_type == ContentType.rpm:
+            message = update_schemas.UpdateReadyForTestingV3.from_dict(
+                message=up._build_group_test_message(agent=request.identity.name)
+            )
+            notifications.publish(message)
 
         # If editing a Pending update, all of whose builds are signed, for a release
         # which isn't composed by Bodhi (i.e. Rawhide), move it directly to Testing.
@@ -4215,10 +4221,10 @@ def _build_group_test_message(self, agent="bodhi", retrigger=False):
         """
         Build the dictionary sent when an update is ready to be tested.
 
-        This is used in bodhi.server.models.Update._ready_for_testing and in
-        bodhi.server.services.updates.trigger_tests which are the two places
-        where we send notifications about an update being ready to be tested
-        by any CI system.
+        This is used when we send notifications about an update being
+        ready to be tested by any CI system - on update creation, any
+        time the update is edited and its builds change, and if someone
+        sends a Re-Trigger Tests request via the web UI or API.
 
         Args:
             agent (str): For the case where the message is sent as a test
@@ -4229,67 +4235,48 @@ def _build_group_test_message(self, agent="bodhi", retrigger=False):
         Returns:
             dict: A dictionary corresponding to the message sent
         """
-        contact = {
-            "name": "Bodhi",
-            "email": "admin@fp.o",
-            "team": "Fedora CI",
-            "docs": "https://docs.fedoraproject.org/en-US/ci/",
-        }
         builds = []
         for build in self.builds:
             builds.append({
                 "type": "koji-build",
                 "id": build.get_build_id(),
                 "task_id": build.get_task_id(),
-                "issuer": build.get_owner_name(),
-                "component": build.nvr_name,
                 "nvr": build.nvr,
-                "scratch": False,
             })
 
         artifact = {
             "type": "koji-build-group",
-            "id": f"{self.alias}-{self.version_hash}",
-            "repository": self.abs_url(),
             "builds": builds,
-            "release": self.release.dist_tag,
         }
         return {
-            "contact": contact,
             "artifact": artifact,
             "update": self,
-            "generated_at": datetime.utcnow().isoformat() + 'Z',
-            "version": "0.2.2",
             'agent': agent,
             're-trigger': retrigger,
         }
 
     @staticmethod
-    def _ready_for_testing(target, value, old, initiator):
+    def _ready_for_testing(target, old):
         """
-        Signal that the update has been moved to testing.
+        Signal that the update is ready for testing.
 
-        This happens in the following cases:
-        - for stable releases: the update lands in the testing repository
-        - for rawhide: all packages in an update have been built by koji
+        This happens when the update is created. The same message is
+        also published when the update is edited and its builds change
+        and when a "re-trigger tests" request is sent, but not by this
+        method.
 
         Args:
-            target (Update): The update that has had a change to its status attribute.
-            value (EnumSymbol): The new value of Update.status.
-            old (EnumSymbol): The old value of the Update.status
-            initiator (sqlalchemy.orm.attributes.Event): The event object that is initiating this
-                transition.
+            target (Update): The update that has been created.
+            old (EnumSymbol): The old value of the Update.status.
         """
-        if value != UpdateStatus.testing or value == old:
-            return
         if old == NEVER_SET:
             # This is the object initialization phase. This instance is not ready, don't create
             # the message now. This method will be called again at the end of __init__
             return
         if target.content_type != ContentType.rpm:
             return
 
-        message = update_schemas.UpdateReadyForTestingV2.from_dict(
+        message = update_schemas.UpdateReadyForTestingV3.from_dict(
             message=target._build_group_test_message()
         )
         notifications.publish(message)
@@ -4304,14 +4291,6 @@ def _ready_for_testing(target, value, old, initiator):
 )
 
 
-event.listen(
-    Update.status,
-    'set',
-    Update._ready_for_testing,
-    active_history=True,
-)
-
-
 class Compose(Base):
     """
     Express the status of an in-progress compose job.

bodhi-server/bodhi/server/services/updates.py

@@ -757,7 +757,7 @@ def trigger_tests(request):
         request.errors.add('body', 'request', 'Update is not in testing status')
     else:
         if update.content_type == ContentType.rpm:
-            message = update_schemas.UpdateReadyForTestingV2.from_dict(
+            message = update_schemas.UpdateReadyForTestingV3.from_dict(
                 message=update._build_group_test_message(agent=request.identity.name,
                                                          retrigger=True)
             )

bodhi-server/tests/base.py

@@ -141,11 +141,12 @@ def create_update(session, build_nvrs, release_name='F17'):
                                             expiration_date=expiration_date)
         session.add(override)
 
-    update = models.Update(
-        builds=builds, user=user, request=models.UpdateRequest.testing,
-        notes='Useful details!', type=models.UpdateType.bugfix,
-        date_submitted=datetime(1984, 11, 2),
-        requirements='rpmlint', stable_karma=3, unstable_karma=-3, release=release)
+    with mock.patch('bodhi.server.models.notifications'):
+        update = models.Update(
+            builds=builds, user=user, request=models.UpdateRequest.testing,
+            notes='Useful details!', type=models.UpdateType.bugfix,
+            date_submitted=datetime(1984, 11, 2),
+            requirements='rpmlint', stable_karma=3, unstable_karma=-3, release=release)
     session.add(update)
     return update
 
@@ -185,7 +186,8 @@ def populate(db):
     db.flush()
     # This mock will help us generate a consistent update alias.
     with mock.patch(target='uuid.uuid4', return_value='wat'):
-        update = create_update(db, ['bodhi-2.0-1.fc17'])
+        with mock.patch('bodhi.server.models.notifications'):
+            update = create_update(db, ['bodhi-2.0-1.fc17'])
     update.type = models.UpdateType.bugfix
     update.severity = models.UpdateSeverity.medium
     bug = models.Bug(bug_id=12345)
@@ -247,11 +249,12 @@ def _setup_method(self):
         self.db = Session()
         self.db.begin_nested()
 
+        buildsys.setup_buildsystem({'buildsystem': 'dev'})
+
         if self._populate_db:
             populate(self.db)
 
         bugs.set_bugtracker()
-        buildsys.setup_buildsystem({'buildsystem': 'dev'})
 
         self._request_sesh = mock.patch('bodhi.server.webapp._complete_database_session',
                                         webapp._rollback_or_commit)