From 293a756884d5ab8e660b23b29813ea02db752982 Mon Sep 17 00:00:00 2001
From: Adam Williamson <awilliam@redhat.com>
Date: Fri, 8 Dec 2023 12:15:32 -0800
Subject: [PATCH] Overhaul update "met requirements" checks

There are several problems in the general area of "checking if
an update 'meets requirements'".

Most obviously, as discussed in the comments on
https://github.com/fedora-infra/bodhi/issues/3938 , there are
two similar-but-different functions for this,
`Update.meets_testing_requirements()` and
`Update.check_requirements()`. `Update.set_request()` also sort
of re-implements the same thing in-line, and there is also
`Update.critpath_approved()` which does something similar.
These paths all implement somewhat different requirements,
which has various weird and confusing results (like the way
you can sometimes push an update stable from the CLI but not
from the web UI, or the way sometimes updates can be submitted
stable successfully but then get ejected from the stable push
for not meeting requirements).

This commit introduces `meets_requirements_why`, which returns
a boolean and a string explanation, and turns the existing
methods into thin wrappers around it.

The block in the API `set_request()` function which checked
requirements is dropped because it's now redundant; the DB
model `set_request()` which it calls does the same requirement
check. There's no need to do it twice.

The code block commented "Disable pushing critical path updates
for pending releases directly to stable" does not really do
that. It just applied the requirements for critical path
updates on push to stable (the block immediately under it
did the same for non-critpath updates). The boutique requirements
to do with a "critpath.num_admin_approvals" setting were for a
very old policy which gave special status to feedback from
"proven testers"; that setting has been 0 in both Bodhi
deployments for years and is not likely to come back, so we can
just get rid of the setting and simplify the code path.

This commit clarifies and simplifies the meanings of the two
types of stable push threshold settings, and renames some of
them. The `stable_karma` and `stable_days` properties of each
update are now *only* relevant to autopush. They are never used
in "can this update be pushed manually?" calculations. The
`mandatory_days_in_testing` and `min_karma` settings act *only*
as policy minimums for manual push eligibility. They can still
be set with prefix modifiers for the release and the 'status',
as before (to allow Fedora to apply different policies to
different releases at different points in the cycle). If an
update reaches either currently-applicable policy minimum, it
is eligible for stable push. If it does not, it is not. This
results in clearer logic and more consistent requirements that
are more in line with the distribution policies.

critpath.stable_after_days_without_negative_karma is renamed
to critpath.mandatory_days_in_testing and now simply sets
a days-in-testing threshold that applies to critical path
updates only. The "without_negative_karma" logic is removed,
because it has no basis in the Fedora or EPEL update policies;
they never specified such a rule.

critpath.min_karma is renamed to min_karma and applied to all
updates, because this is what the current Fedora and EPEL update
policies say. They both apply the same minimum karma thresholds
to *all* updates. We *could* instead add min_karma and keep
critpath.min_karma, but this is somewhat more work and I don't
want to do more work than is actually necessary here. If the
distros ever do decide to split the thresholds again, we can
re-introduce critpath.min_karma.

The functions and properties in the models are renamed in line
with the new names of the settings, again for sanity's sake.

This is of course an API change and a backwards-incompatible
configuration change. As there are only two real deployments
of Bodhi (both run out of the same ansible repo) and I don't
think anything uses the code interfaces, this seems like it
shouldn't be too much of a problem.

Signed-off-by: Adam Williamson <awilliam@redhat.com>
(cherry picked from commit 93598c78e5ddd9f9849aebc4c5c125e3c6f38100)
---
 bodhi-server/bodhi/server/config.py           |  11 +-
 bodhi-server/bodhi/server/models.py           | 249 +++---
 bodhi-server/bodhi/server/services/updates.py |  13 +-
 bodhi-server/bodhi/server/tasks/composer.py   |   2 +-
 bodhi-server/production.ini                   |  12 +-
 bodhi-server/tests/services/test_updates.py   | 760 +++++-------------
 .../tests/tasks/test_approve_testing.py       |   3 +
 bodhi-server/tests/tasks/test_composer.py     | 228 +++---
 bodhi-server/tests/test_models.py             | 245 +++---
 bodhi-server/tests/testing.ini                |   6 +-
 devel/ci/integration/bodhi/production.ini     |  10 +-
 devel/development.ini.example                 |   4 +-
 docs/user/testing.rst                         |  36 +-
 13 files changed, 584 insertions(+), 995 deletions(-)

diff --git a/bodhi-server/bodhi/server/config.py b/bodhi-server/bodhi/server/config.py
index 08a5e98ee8..0589808863 100644
--- a/bodhi-server/bodhi/server/config.py
+++ b/bodhi-server/bodhi/server/config.py
@@ -360,13 +360,7 @@ class BodhiConfig(dict):
         'critpath.jsonpath': {
             'value': '/etc/bodhi/critpath',
             'validator': str},
-        'critpath.min_karma': {
-            'value': 2,
-            'validator': int},
-        'critpath.num_admin_approvals': {
-            'value': 2,
-            'validator': int},
-        'critpath.stable_after_days_without_negative_karma': {
+        'critpath.mandatory_days_in_testing': {
             'value': 14,
             'validator': int},
         'critpath.type': {
@@ -452,6 +446,9 @@ class BodhiConfig(dict):
         'mako.directories': {
             'value': 'bodhi.server:templates',
             'validator': str},
+        'min_karma': {
+            'value': 2,
+            'validator': int},
         'mandatory_packager_groups': {
             'value': ['packager'],
             'validator': _generate_list_validator()},
diff --git a/bodhi-server/bodhi/server/models.py b/bodhi-server/bodhi/server/models.py
index 21dd56cb37..474971add5 100644
--- a/bodhi-server/bodhi/server/models.py
+++ b/bodhi-server/bodhi/server/models.py
@@ -27,6 +27,7 @@
 import time
 import typing
 import uuid
+import warnings
 
 from mediawiki import MediaWiki
 from packaging.version import parse as parse_version
@@ -887,22 +888,22 @@ class Release(Base):
     composes = relationship('Compose', back_populates='release')
 
     @property
-    def critpath_min_karma(self) -> int:
+    def min_karma(self) -> int:
         """
-        Return the min_karma for critpath updates for this release.
+        Return the min_karma for updates for this release.
 
-        If the release doesn't specify a min_karma, the default critpath.min_karma setting is used
+        If the release doesn't specify a min_karma, the default min_karma setting is used
         instead.
 
         Returns:
-            The minimum karma required for critical path updates for this release.
+            The minimum karma required for updates for this release.
         """
         if self.setting_status:
             min_karma = config.get(
-                f'{self.setting_prefix}.{self.setting_status}.critpath.min_karma', None)
+                f'{self.setting_prefix}.{self.setting_status}.min_karma', None)
             if min_karma:
                 return int(min_karma)
-        return config.get('critpath.min_karma')
+        return config.get('min_karma')
 
     @property
     def version_int(self):
@@ -948,11 +949,11 @@ def critpath_mandatory_days_in_testing(self):
         """
         if self.setting_status:
             days = config.get(f'{self.setting_prefix}.{self.setting_status}'
-                              f'.critpath.stable_after_days_without_negative_karma', None)
+                              f'.critpath.mandatory_days_in_testing', None)
             if days is not None:
                 return int(days)
 
-        return config.get('critpath.stable_after_days_without_negative_karma', 0)
+        return config.get('critpath.mandatory_days_in_testing', 0)
 
     @property
     def collection_name(self):
@@ -2530,6 +2531,16 @@ def new(cls, request, data):
                                f"release value of {up.mandatory_days_in_testing} days"
             })
 
+        # We also need to make sure stable_karma is not set below
+        # the policy minimum for this release
+        if release.min_karma > up.stable_karma:
+            up.stable_karma = release.min_karma
+            caveats.append({
+                'name': 'stable karma',
+                'description': "The stable karma required was set to the mandatory "
+                               f"release value of {release.min_karma}"
+            })
+
         log.debug(f"Adding new update to the db {up.alias}.")
         db.add(up)
         log.debug(f"Triggering db commit for new update {up.alias}.")
@@ -2599,6 +2610,16 @@ def edit(cls, request, data):
                                f"release value of {up.mandatory_days_in_testing} days"
             })
 
+        # We also need to make sure stable_karma is not set below
+        # the policy minimum for this release
+        if up.release.min_karma > data.get('stable_karma', up.stable_karma):
+            data['stable_karma'] = up.release.min_karma
+            caveats.append({
+                'name': 'stable karma',
+                'description': "The stable karma required was raised to the mandatory "
+                               f"release value of {up.release.min_karma}"
+            })
+
         # Determine which builds have been added
         new_builds = []
         for build in data['builds']:
@@ -3128,57 +3149,25 @@ def set_request(self, db, action, username):
                 'The release of this update is frozen and the update has not yet been '
                 'pushed to testing. It is currently not possible to push it to stable.')
 
-        # Disable pushing critical path updates for pending releases directly to stable
-        if action == UpdateRequest.stable and self.critpath:
-            if config.get('critpath.num_admin_approvals') is not None:
-                if not self.critpath_approved:
-                    stern_note = (
-                        'This critical path update has not yet been approved for pushing to the '
-                        'stable repository.  It must first reach a karma of %s, consisting of %s '
-                        'positive karma from proventesters, along with %d additional karma from '
-                        'the community. Or, it must spend %s days in testing without any negative '
-                        'feedback')
-                    additional_karma = config.get('critpath.min_karma') \
-                        - config.get('critpath.num_admin_approvals')
-                    stern_note = stern_note % (
-                        config.get('critpath.min_karma'),
-                        config.get('critpath.num_admin_approvals'),
-                        additional_karma,
-                        config.get('critpath.stable_after_days_without_negative_karma'))
-                    if config.get('test_gating.required'):
-                        stern_note += ' Additionally, it must pass automated tests.'
-                    notes.append(stern_note)
-
-                    if self.status is UpdateStatus.testing:
-                        self.request = None
-                        raise BodhiException('. '.join(notes))
-                    else:
-                        log.info('Forcing critical path update into testing')
-                        action = UpdateRequest.testing
-
-        # Ensure this update meets the minimum testing requirements
-        flash_notes = ''
-        if action == UpdateRequest.stable and not self.critpath:
-            # Check if we've met the karma requirements
-            if self.karma >= self.stable_karma or self.critpath_approved:
-                log.debug('%s meets stable karma requirements' % self.alias)
-            else:
-                # If we haven't met the stable karma requirements, check if it
-                # has met the mandatory time-in-testing requirements
-                if self.mandatory_days_in_testing:
-                    if not self.has_stable_comment and \
-                       not self.meets_testing_requirements:
-                        if self.release.id_prefix == "FEDORA-EPEL":
-                            flash_notes = config.get('not_yet_tested_epel_msg')
-                        else:
-                            flash_notes = config.get('not_yet_tested_msg')
-                        if self.status is UpdateStatus.testing:
-                            self.request = None
-                            raise BodhiException(flash_notes)
-                        elif self.request is UpdateRequest.testing:
-                            raise BodhiException(flash_notes)
-                        else:
-                            action = UpdateRequest.testing
+        # Don't allow push to stable unless testing requirements are met
+        if action == UpdateRequest.stable:
+            met, reason = self.meets_requirements_why
+            if not met:
+                if self.release.id_prefix == "FEDORA-EPEL":
+                    msg = config.get('not_yet_tested_epel_msg')
+                else:
+                    msg = config.get('not_yet_tested_msg')
+                msg = f'{msg}: {reason}'
+                notes.append(msg)
+
+                if self.status is UpdateStatus.testing:
+                    self.request = None
+                    raise BodhiException('. '.join(notes))
+                elif self.request is UpdateRequest.testing:
+                    raise BodhiException('. '.join(notes))
+                else:
+                    log.info('Forcing update into testing')
+                    action = UpdateRequest.testing
 
         # Add the appropriate 'pending' koji tag to this update, so tools
         # could compose repositories of them for testing.
@@ -3200,10 +3189,9 @@ def set_request(self, db, action, username):
         self.request = action
 
         notes = notes and '. '.join(notes) + '.' or ''
-        flash_notes = flash_notes and '. %s' % flash_notes
         log.debug(
-            "%s has been submitted for %s. %s%s" % (
-                self.alias, action.description, notes, flash_notes))
+            "%s has been submitted for %s. %s" % (
+                self.alias, action.description, notes))
 
         comment_text = 'This update has been submitted for %s by %s. %s' % (
             action.description, username, notes)
@@ -3597,8 +3585,8 @@ def comment(self, session, text, karma=0, author=None, karma_critpath=0,
                     pass
                 except BodhiException as e:
                     # This gets thrown if the karma is pushed over the
-                    # threshold, but it is a critpath update that is not
-                    # critpath_approved. ... among other cases.
+                    # threshold, but it does not meet testing requirements
+                    # for some reason (failed gating test...)
                     log.exception('Problem checking the karma threshold.')
                     caveats.append({
                         'name': 'karma', 'description': str(e),
@@ -3774,27 +3762,23 @@ def product_version(self):
 
     def check_requirements(self, session, settings):
         """
-        Check that an update meets its self-prescribed policy to be pushed.
+        Check that an update meets all requirements to be pushed stable.
+
+        See https://docs.fedoraproject.org/en-US/fesco/Updates_Policy .
+        Deprecated in favor of meets_requirements_why property.
 
         Args:
             session (sqlalchemy.orm.session.Session): A database session. Unused.
-            settings (bodhi.server.config.BodhiConfig): Bodhi's settings.
+            settings (bodhi.server.config.BodhiConfig): Bodhi's settings. Unused.
         Returns:
             tuple: A tuple containing (result, reason) where result is a bool
                 and reason is a str.
         """
-        # TODO - check require_bugs and require_testcases also?
-        if config.get('test_gating.required'):
-            if self.test_gating_status == TestGatingStatus.passed:
-                return (True, "All required tests passed or were waived.")
-            elif self.test_gating_status in (TestGatingStatus.ignored, None):
-                return (True, "No checks required.")
-            elif self.test_gating_status == TestGatingStatus.waiting:
-                return (False, "Required tests for this update are still running.")
-            else:
-                return (False, "Required tests did not pass on this update.")
-
-        return (True, "No checks required.")
+        warnings.warn(
+            "check_requirements is deprecated, use meets_requirements_why instead",
+            DeprecationWarning,
+        )
+        return self.meets_requirements_why
 
     def check_karma_thresholds(self, db, agent):
         """
@@ -3896,55 +3880,73 @@ def critpath_approved(self):
         Returns:
             bool: True if this update meets critpath testing requirements, False otherwise.
         """
-        # https://fedorahosted.org/bodhi/ticket/642
-        if self.meets_testing_requirements:
-            return True
-        min_karma = self.release.critpath_min_karma
-        if self.release.setting_status:
-            num_admin_approvals = config.get(
-                f'{self.release.setting_prefix}.{self.release.setting_status}'
-                '.critpath.num_admin_approvals')
-            if num_admin_approvals is not None and min_karma:
-                return self.num_admin_approvals >= int(num_admin_approvals) and \
-                    self.karma >= min_karma
-        return self.num_admin_approvals >= config.get('critpath.num_admin_approvals') and \
-            self.karma >= min_karma
+        warnings.warn(
+            "critpath_approved is deprecated, use meets_testing_requirements instead",
+            DeprecationWarning,
+        )
+        return self.meets_testing_requirements
 
     @property
-    def meets_testing_requirements(self):
+    def meets_requirements_why(self):
         """
-        Return whether or not this update meets its release's testing requirements.
+        Return whether or not this update meets its release's testing requirements and why.
+
+        Checks gating requirements and then minimum karma and wait requirements, see
+        https://docs.fedoraproject.org/en-US/fesco/Updates_Policy . Note this is
+        checking the minimum policy requirements for a manual push (as specified in
+        the release configuration), not the per-update thresholds for an automatic push.
+        Before updates-testing enablement the minimum wait will be 0, so unless the gating
+        check fails, this will always return True.
 
-        If this update's release does not have a mandatory testing requirement, then
-        simply return True.
+        FIXME: we should probably wire up the test case and bug feedback requirements
+        that are shown in the UI but not currently enforced.
 
         Returns:
-            bool: True if the update meets testing requirements, False otherwise.
+            tuple: A tuple containing (result, reason) where result is a bool
+                and reason is a str.
         """
-        num_days = self.mandatory_days_in_testing
+        req_days = self.mandatory_days_in_testing
+        req_karma = self.release.min_karma
 
-        if config.get('test_gating.required') and not self.test_gating_passed:
-            return False
+        if config.get('test_gating.required'):
+            tgs = self.test_gating_status
+            if tgs in (TestGatingStatus.waiting, TestGatingStatus.queued, TestGatingStatus.running):
+                return (False, "Required tests for this update are queued or running.")
+            elif tgs == TestGatingStatus.greenwave_failed:
+                return (False, "Greenwave failed to respond.")
+            elif tgs == TestGatingStatus.passed:
+                basemsg = "All required tests passed or were waived"
+            elif tgs == TestGatingStatus.ignored:
+                basemsg = "No checks required"
+            else:
+                return (False, "Required tests did not pass on this update.")
+        else:
+            basemsg = "Test gating is disabled,"
 
-        if self.karma >= self.release.critpath_min_karma:
-            return True
+        if self.karma >= req_karma:
+            return (True, basemsg + f" and the update has at least {req_karma} karma.")
 
-        if self.critpath:
-            # Ensure there is no negative karma. We're looking at the sum of
-            # each users karma for this update, which takes into account
-            # changed votes.
-            if self._composite_karma[1] < 0:
-                return False
-            return self.days_in_testing >= num_days
+        if not req_days:
+            return (True, basemsg + " and there is no minimum wait set.")
 
-        if not num_days:
-            return True
+        # Any update that reaches req_days has met the testing requirements.
+        if self.days_in_testing >= req_days:
+            return (True, basemsg + " and update meets the wait time requirement.")
+        return (False,
+                basemsg + f" but update has less than {req_karma} karma and has been in testing "
+                f"less than {req_days} days.")
 
-        if self.karma >= self.stable_karma:
-            return True
+    @property
+    def meets_testing_requirements(self):
+        """
+        Return whether or not this update meets its release's testing requirements.
 
-        # Any update that reaches num_days has met the testing requirements.
-        return self.days_in_testing >= num_days
+        Same as meets_requirements_why, but without the reason.
+
+        Returns:
+            bool: True if the update meets testing requirements, False otherwise.
+        """
+        return self.meets_requirements_why[0]
 
     @property
     def has_stable_comment(self):
@@ -3997,25 +3999,6 @@ def days_in_testing(self):
         else:
             return 0
 
-    @property
-    def num_admin_approvals(self):
-        """
-        Return the number of Releng/QA approvals of this update.
-
-        Returns:
-            int: The number of admin approvals found in the comments of this update.
-        """
-        approvals = 0
-        for comment in self.comments_since_karma_reset:
-            if comment.karma != 1:
-                continue
-            admin_groups = config.get('admin_groups')
-            for group in comment.user.groups:
-                if group.name in admin_groups:
-                    approvals += 1
-                    break
-        return approvals
-
     @property
     def test_cases(self):
         """
diff --git a/bodhi-server/bodhi/server/services/updates.py b/bodhi-server/bodhi/server/services/updates.py
index a9b29c9d42..e7d40ec6c7 100644
--- a/bodhi-server/bodhi/server/services/updates.py
+++ b/bodhi-server/bodhi/server/services/updates.py
@@ -211,21 +211,10 @@ def set_request(request):
                            "Pushing back to testing a stable update is not allowed")
         return
 
-    if action == UpdateRequest.stable:
-        settings = request.registry.settings
-        result, reason = update.check_requirements(request.db, settings)
-        if not result:
-            log.info(
-                f'Unable to set request for {update.alias} to stable due to failed requirements: '
-                f'{reason}')
-            request.errors.add('body', 'request',
-                               'Requirement not met %s' % reason)
-            return
-
     try:
         update.set_request(request.db, action, request.identity.name)
     except BodhiException as e:
-        log.info("Failed to set the request: %s", e)
+        log.info(f"Failed to set the request: {e}")
         request.errors.add('body', 'request', str(e))
     except Exception as e:
         log.exception("Unhandled exception in set_request")
diff --git a/bodhi-server/bodhi/server/tasks/composer.py b/bodhi-server/bodhi/server/tasks/composer.py
index 2b874ba11d..ae5d8ae5db 100644
--- a/bodhi-server/bodhi/server/tasks/composer.py
+++ b/bodhi-server/bodhi/server/tasks/composer.py
@@ -469,7 +469,7 @@ def perform_gating(self):
         """Eject Updates that don't meet testing requirements from the compose."""
         log.debug('Performing gating.')
         for update in self.compose.updates:
-            result, reason = update.check_requirements(self.db, config)
+            result, reason = update.meets_requirements_why
             if not result:
                 log.warning("%s failed gating: %s" % (update.alias, reason))
                 self.eject_from_compose(update, reason)
diff --git a/bodhi-server/production.ini b/bodhi-server/production.ini
index d1ba5e5244..47acfb32a9 100644
--- a/bodhi-server/production.ini
+++ b/bodhi-server/production.ini
@@ -408,10 +408,10 @@ use = egg:bodhi-server
 # critpath.num_admin_approvals = 2
 
 # The net karma required to submit a critical path update to a pending release.
-# critpath.min_karma = 2
+# min_karma = 2
 
 # Allow critpath to submit for stable after 2 weeks with no negative karma
-# critpath.stable_after_days_without_negative_karma = 14
+# critpath.mandatory_days_in_testing = 14
 
 # The minimum amount of time an update must spend in testing before
 # it can reach the stable repository
@@ -431,12 +431,12 @@ fedora_epel.mandatory_days_in_testing = 14
 #f28.status = pre_beta
 #f28.pre_beta.mandatory_days_in_testing = 3
 #f28.pre_beta.critpath.num_admin_approvals = 0
-#f28.pre_beta.critpath.min_karma = 1
-#f28.pre_beta.critpath.stable_after_days_without_negative_karma = 3
+#f28.pre_beta.min_karma = 1
+#f28.pre_beta.critpath.mandatory_days_in_testing = 3
 #f28.post_beta.mandatory_days_in_testing = 7
 #f28.post_beta.critpath.num_admin_approvals = 0
-#f28.post_beta.critpath.min_karma = 2
-#f28.post_beta.critpath.stable_after_days_without_negative_karma = 5
+#f28.post_beta.min_karma = 2
+#f28.post_beta.critpath.mandatory_days_in_testing = 5
 
 
 ##
diff --git a/bodhi-server/tests/services/test_updates.py b/bodhi-server/tests/services/test_updates.py
index ab270c3441..05fe49cdb9 100644
--- a/bodhi-server/tests/services/test_updates.py
+++ b/bodhi-server/tests/services/test_updates.py
@@ -705,22 +705,34 @@ def test_new_update_with_invalid_stable_days(self, publish, *args):
         assert up['errors'][0]['description'] == "-1 is less than minimum value 0"
 
     @mock.patch('bodhi.server.notifications.publish')
-    def test_edit_update_stable_days(self, publish, *args):
+    def test_new_update_stable_days(self, publish, *args):
         args = self.get_update(u'bodhi-2.0.0-2.fc17')
         args['stable_days'] = '50'
         r = self.app.post_json('/updates/', args)
         assert r.json['stable_days'] == 50
 
     @mock.patch('bodhi.server.notifications.publish')
-    def test_edit_update_too_low_stable_days(self, publish, *args):
+    def test_new_update_too_low_stable_days(self, publish, *args):
         args = self.get_update(u'bodhi-2.0.0-2.fc17')
         args['stable_days'] = '1'
+        args['stable_karma'] = '50'
         r = self.app.post_json('/updates/', args)
         assert r.json['stable_days'] == 7
         assert r.json['caveats'][0]['description'] == (
             'The number of stable days required was set to the mandatory release value of 7 days'
         )
 
+    @mock.patch('bodhi.server.notifications.publish')
+    def test_new_update_too_low_stable_karma(self, publish, *args):
+        args = self.get_update(u'bodhi-2.0.0-2.fc17')
+        args['stable_days'] = '50'
+        args['stable_karma'] = '1'
+        r = self.app.post_json('/updates/', args)
+        assert r.json['stable_karma'] == 2
+        assert r.json['caveats'][0]['description'] == (
+            'The stable karma required was set to the mandatory release value of 2'
+        )
+
     @mock.patch.dict('bodhi.server.validators.config', {'acl_system': u'dummy'})
     def test_new_update_with_multiple_bugs_as_str(self, *args):
         update = self.get_update('bodhi-2.0.0-2.fc17')
@@ -1134,6 +1146,7 @@ def test_test_gating_status_failed(self, info):
         up.locked = False
         up.test_gating_status = TestGatingStatus.failed
         up.date_testing = datetime.utcnow() - timedelta(days=8)
+        up.status = UpdateStatus.testing
         up.request = None
         post_data = dict(update=nvr, request='stable', csrf_token=self.get_csrf_token())
 
@@ -1143,11 +1156,11 @@ def test_test_gating_status_failed(self, info):
         assert up.request is None
         assert res.json_body['status'] == 'error'
         assert res.json_body['errors'][0]['description'] == (
-            "Requirement not met Required tests did not pass on this update."
+            f"{config.get('not_yet_tested_msg')}: Required tests did not pass on this update."
         )
         info_logs = '\n'.join([c[1][0] for c in info.mock_calls])
-        assert (f'Unable to set request for {up.alias} to stable due to failed requirements: '
-                'Required tests did not pass on this update.') in info_logs
+        assert (f"Failed to set the request: {config.get('not_yet_tested_msg')}: "
+                "Required tests did not pass on this update.") in info_logs
 
     @mock.patch.dict(config, {'test_gating.required': True})
     def test_test_gating_status_passed(self):
@@ -1167,11 +1180,11 @@ def test_test_gating_status_passed(self):
         assert res.json['update']['request'] == 'stable'
 
     @mock.patch('bodhi.server.services.updates.Update.set_request',
-                side_effect=BodhiException('BodhiException. oops!'))
-    @mock.patch('bodhi.server.services.updates.Update.check_requirements',
-                return_value=(True, "a fake reason"))
+                side_effect=BodhiException('oops!'))
+    @mock.patch('bodhi.server.services.updates.Update.meets_requirements_why',
+                (True, "a fake reason"))
     @mock.patch('bodhi.server.services.updates.log.info')
-    def test_BodhiException_exception(self, log_info, check_requirements, send_request, *args):
+    def test_BodhiException_exception(self, log_info, send_request, *args):
         """Ensure that an BodhiException Exception is handled by set_request()."""
         nvr = 'bodhi-2.0-1.fc17'
 
@@ -1186,16 +1199,16 @@ def test_BodhiException_exception(self, log_info, check_requirements, send_reque
                                  status=400)
 
         assert res.json_body['status'] == 'error'
-        assert res.json_body['errors'][0]['description'] == 'BodhiException. oops!'
+        assert res.json_body['errors'][0]['description'] == 'oops!'
         assert log_info.call_count == 1
-        assert log_info.call_args_list[0][0][0] == "Failed to set the request: %s"
+        assert log_info.call_args_list[0][0][0] == "Failed to set the request: oops!"
 
     @mock.patch('bodhi.server.services.updates.Update.set_request',
                 side_effect=IOError('IOError. oops!'))
-    @mock.patch('bodhi.server.services.updates.Update.check_requirements',
-                return_value=(True, "a fake reason"))
+    @mock.patch('bodhi.server.services.updates.Update.meets_requirements_why',
+                (True, "a fake reason"))
     @mock.patch('bodhi.server.services.updates.log.exception')
-    def test_unexpected_exception(self, log_exception, check_requirements, send_request, *args):
+    def test_unexpected_exception(self, log_exception, send_request, *args):
         """Ensure that an unexpected Exception is handled by set_request()."""
         nvr = 'bodhi-2.0-1.fc17'
 
@@ -1597,7 +1610,11 @@ def test_provenpackager_request_privs(self, *args):
 
         # Ensure we can't push it until it meets the requirements
         assert res.json_body['status'] == 'error'
-        assert res.json_body['errors'][0]['description'] == config.get('not_yet_tested_msg')
+        exp = (
+            f"{config.get('not_yet_tested_msg')}: Test gating is disabled, but update has "
+            "less than 2 karma and has been in testing less than 7 days."
+        )
+        assert res.json['errors'][0]['description'] == exp
 
         update = Build.query.filter_by(nvr=nvr).one().update
         assert update.stable_karma == 3
@@ -1699,178 +1716,21 @@ def test_provenpackager_request_privs(self, *args):
         res = app.get(f'/updates/{update.alias}', status=200)
         assert res.json_body['can_edit'] is False
 
-    def test_provenpackager_request_update_queued_in_test_gating(self, *args):
-        """Ensure provenpackagers cannot request changes for any update which
-        test gating status is `queued`"""
-        nvr = 'bodhi-2.1-1.fc17'
-        user = User(name='bob')
-        self.db.add(user)
-        group = self.db.query(Group).filter_by(name='provenpackager').one()
-        user.groups.append(group)
-        self.db.commit()
-
-        up_data = self.get_update(nvr)
-        up_data['csrf_token'] = self.app.get('/csrf').json_body['csrf_token']
-
-        with fml_testing.mock_sends(update_schemas.UpdateReadyForTestingV3,
-                                    update_schemas.UpdateRequestTestingV1):
-            res = self.app.post_json('/updates/', up_data)
-
-        build = self.db.query(RpmBuild).filter_by(nvr=nvr).one()
-        build.update.test_gating_status = TestGatingStatus.queued
-        assert build.update.request == UpdateRequest.testing
-
-        # Try and submit the update to stable as a provenpackager
-        post_data = dict(update=nvr, request='stable',
-                         csrf_token=self.app.get('/csrf').json_body['csrf_token'])
-        with mock.patch.dict(config, {'test_gating.required': True}):
-            res = self.app.post_json(f'/updates/{build.update.alias}/request',
-                                     post_data, status=400)
-
-        # Ensure we can't push it until it passed test gating
-        assert res.json_body['status'] == 'error'
-        assert res.json_body['errors'][0]['description'] == (
-            'Requirement not met Required tests did not pass on this update.'
-        )
-
-    def test_provenpackager_request_update_running_in_test_gating(self, *args):
-        """Ensure provenpackagers cannot request changes for any update which
-        test gating status is `running`"""
-        nvr = 'bodhi-2.1-1.fc17'
-        user = User(name='bob')
-        self.db.add(user)
-        group = self.db.query(Group).filter_by(name='provenpackager').one()
-        user.groups.append(group)
-        self.db.commit()
-
-        up_data = self.get_update(nvr)
-        up_data['csrf_token'] = self.app.get('/csrf').json_body['csrf_token']
-
-        with fml_testing.mock_sends(update_schemas.UpdateReadyForTestingV3,
-                                    update_schemas.UpdateRequestTestingV1):
-            res = self.app.post_json('/updates/', up_data)
-
-        build = self.db.query(RpmBuild).filter_by(nvr=nvr).one()
-        build.update.test_gating_status = TestGatingStatus.running
-        assert build.update.request == UpdateRequest.testing
-
-        # Try and submit the update to stable as a provenpackager
-        post_data = dict(update=nvr, request='stable',
-                         csrf_token=self.app.get('/csrf').json_body['csrf_token'])
-        with mock.patch.dict(config, {'test_gating.required': True}):
-            res = self.app.post_json(f'/updates/{build.update.alias}/request',
-                                     post_data, status=400)
-
-        # Ensure we can't push it until it passed test gating
-        assert res.json_body['status'] == 'error'
-        assert res.json_body['errors'][0]['description'] == (
-            'Requirement not met Required tests did not pass on this update.'
-        )
-
-    def test_provenpackager_request_update_failed_test_gating(self, *args):
-        """Ensure provenpackagers cannot request changes for any update which
-        test gating status is `failed`"""
-        nvr = 'bodhi-2.1-1.fc17'
-        user = User(name='bob')
-        self.db.add(user)
-        group = self.db.query(Group).filter_by(name='provenpackager').one()
-        user.groups.append(group)
-        self.db.commit()
-
-        up_data = self.get_update(nvr)
-        up_data['csrf_token'] = self.app.get('/csrf').json_body['csrf_token']
-
-        with fml_testing.mock_sends(update_schemas.UpdateReadyForTestingV3,
-                                    update_schemas.UpdateRequestTestingV1):
-            res = self.app.post_json('/updates/', up_data)
-
-        build = self.db.query(RpmBuild).filter_by(nvr=nvr).one()
-        build.update.test_gating_status = TestGatingStatus.failed
-        assert build.update.request == UpdateRequest.testing
-
-        # Try and submit the update to stable as a provenpackager
-        post_data = dict(update=nvr, request='stable',
-                         csrf_token=self.app.get('/csrf').json_body['csrf_token'])
-        with mock.patch.dict(config, {'test_gating.required': True}):
-            res = self.app.post_json(f'/updates/{build.update.alias}/request',
-                                     post_data, status=400)
-
-        # Ensure we can't push it until it passed test gating
-        assert res.json_body['status'] == 'error'
-        assert res.json_body['errors'][0]['description'] == (
-            'Requirement not met Required tests did not pass on this update.'
-        )
-
-    def test_provenpackager_request_update_ignored_by_test_gating(self, *args):
-        """Ensure provenpackagers can request changes for any update which
-        test gating status is `ignored`"""
-        nvr = 'bodhi-2.1-1.fc17'
-        user = User(name='bob')
-        self.db.add(user)
-        group = self.db.query(Group).filter_by(name='provenpackager').one()
-        user.groups.append(group)
-        self.db.commit()
-
-        up_data = self.get_update(nvr)
-        up_data['csrf_token'] = self.app.get('/csrf').json_body['csrf_token']
-
-        with fml_testing.mock_sends(update_schemas.UpdateReadyForTestingV3,
-                                    update_schemas.UpdateRequestTestingV1):
-            res = self.app.post_json('/updates/', up_data)
-
-        assert 'does not have commit access to bodhi' not in res
-        build = self.db.query(RpmBuild).filter_by(nvr=nvr).one()
-        build.update.test_gating_status = TestGatingStatus.ignored
-        assert build.update.request == UpdateRequest.testing
-
-        # Try and submit the update to stable as a provenpackager
-        post_data = dict(update=nvr, request='stable',
-                         csrf_token=self.app.get('/csrf').json_body['csrf_token'])
-        with mock.patch.dict(config, {'test_gating.required': True}):
-            res = self.app.post_json(f'/updates/{build.update.alias}/request',
-                                     post_data, status=400)
-
-        # Ensure the reason we cannot push isn't test gating this time
-        assert res.json_body['status'] == 'error'
-        assert res.json_body['errors'][0]['description'] == config.get('not_yet_tested_msg')
-
-    def test_provenpackager_request_update_waiting_on_test_gating(self, *args):
-        """Ensure provenpackagers cannot request changes for any update which
-        test gating status is `waiting`"""
-        nvr = 'bodhi-2.1-1.fc17'
-        user = User(name='bob')
-        self.db.add(user)
-        group = self.db.query(Group).filter_by(name='provenpackager').one()
-        user.groups.append(group)
-        self.db.commit()
-
-        up_data = self.get_update(nvr)
-        up_data['csrf_token'] = self.app.get('/csrf').json_body['csrf_token']
-
-        with fml_testing.mock_sends(update_schemas.UpdateReadyForTestingV3,
-                                    update_schemas.UpdateRequestTestingV1):
-            res = self.app.post_json('/updates/', up_data)
-
-        build = self.db.query(RpmBuild).filter_by(nvr=nvr).one()
-        build.update.test_gating_status = TestGatingStatus.waiting
-        assert build.update.request == UpdateRequest.testing
-
-        # Try and submit the update to stable as a provenpackager
-        post_data = dict(update=nvr, request='stable',
-                         csrf_token=self.app.get('/csrf').json_body['csrf_token'])
-        with mock.patch.dict(config, {'test_gating.required': True}):
-            res = self.app.post_json(f'/updates/{build.update.alias}/request',
-                                     post_data, status=400)
-
-        # Ensure we can't push it until it passed test gating
-        assert res.json_body['status'] == 'error'
-        assert res.json_body['errors'][0]['description'] == (
-            'Requirement not met Required tests for this update are still running.'
+    @pytest.mark.parametrize(
+        "status,canchange",
+        (
+            (TestGatingStatus.queued, False),
+            (TestGatingStatus.running, False),
+            (TestGatingStatus.failed, False),
+            (TestGatingStatus.failed, False),
+            (TestGatingStatus.waiting, False),
+            (None, False),
+            (TestGatingStatus.passed, True),
         )
-
-    def test_provenpackager_request_update_with_none_test_gating(self, *args):
-        """Ensure provenpackagers cannot request changes for any update which
-        test gating status is `None`"""
+    )
+    def test_provenpackager_request_update_test_gating_status(self, status, canchange):
+        """Ensure provenpackagers can request changes when test gating
+        is passed or ignored, but otherwise cannot."""
         nvr = 'bodhi-2.1-1.fc17'
         user = User(name='bob')
         self.db.add(user)
@@ -1885,20 +1745,33 @@ def test_provenpackager_request_update_with_none_test_gating(self, *args):
                                     update_schemas.UpdateRequestTestingV1):
             res = self.app.post_json('/updates/', up_data)
 
-        build = self.db.query(RpmBuild).filter_by(nvr=nvr).one()
-        build.update.test_gating_status = None
-        assert build.update.request == UpdateRequest.testing
+        update = Build.query.filter_by(nvr=nvr).one().update
+        update.test_gating_status = status
+        # have the update meet karma requirements
+        update.comment(self.db, 'LGTM', author='ralph', karma=1)
+        update.comment(self.db, 'LGTM', author='test', karma=1)
+        # Clear pending messages
+        self.db.info['messages'] = []
+        assert update.request == UpdateRequest.testing
 
         # Try and submit the update to stable as a provenpackager
         post_data = dict(update=nvr, request='stable',
                          csrf_token=self.app.get('/csrf').json_body['csrf_token'])
         with mock.patch.dict(config, {'test_gating.required': True}):
-            res = self.app.post_json(f'/updates/{build.update.alias}/request',
-                                     post_data, status=400)
-
-        # Ensure the reason we can't push is not test gating
-        assert res.json_body['status'] == 'error'
-        assert res.json_body['errors'][0]['description'] == config.get('not_yet_tested_msg')
+            if canchange is False:
+                res = self.app.post_json(f'/updates/{update.alias}/request', post_data, status=400)
+            else:
+                with fml_testing.mock_sends(update_schemas.UpdateRequestStableV1):
+                    res = self.app.post_json(f'/updates/{update.alias}/request', post_data)
+
+        if canchange is False:
+            # Ensure we can't push it until it passed test gating
+            assert res.json_body['status'] == 'error'
+            assert res.json_body['errors'][0]['description'] in (
+                f"{config.get('not_yet_tested_msg')}: Required tests did not pass on this update.",
+                f"{config.get('not_yet_tested_msg')}: "
+                "Required tests for this update are queued or running.",
+            )
 
     def test_404(self):
         self.app.get('/a', status=404)
@@ -3710,6 +3583,64 @@ def test_edit_locked_update(self, *args):
         build = self.db.query(RpmBuild).filter_by(nvr=nvr).one()
         assert up.builds == [build]
 
+    def test_edit_update_too_low_stable_days(self, *args):
+        """Check stable days below the minimum is increased on edit"""
+        nvr = 'bodhi-2.0.0-2.fc17'
+        args = self.get_update(nvr)
+        args['stable_days'] = 50
+        args['stable_karma'] = 50
+
+        with fml_testing.mock_sends(update_schemas.UpdateReadyForTestingV3,
+                                    update_schemas.UpdateRequestTestingV1):
+            self.app.post_json('/updates/', args, status=200)
+
+        up = self.db.query(Build).filter_by(nvr=nvr).one().update
+        up.locked = True
+        up.status = UpdateStatus.testing
+        up.request = None
+        # Clear pending messages
+        self.db.info['messages'] = []
+
+        args['edited'] = up.alias
+        args['stable_days'] = 1
+
+        with fml_testing.mock_sends(update_schemas.UpdateEditV2):
+            up = self.app.post_json('/updates/', args, status=200).json_body
+
+        assert up['stable_days'] == 7
+        assert up['caveats'][0]['description'] == (
+            'The number of stable days required was raised to the mandatory release value of 7 days'
+        )
+
+    def test_edit_update_too_low_stable_karma(self, *args):
+        """Check stable karma below the minimum is increased on edit"""
+        nvr = 'bodhi-2.0.0-2.fc17'
+        args = self.get_update(nvr)
+        args['stable_days'] = 50
+        args['stable_karma'] = 50
+
+        with fml_testing.mock_sends(update_schemas.UpdateReadyForTestingV3,
+                                    update_schemas.UpdateRequestTestingV1):
+            self.app.post_json('/updates/', args, status=200)
+
+        up = self.db.query(Build).filter_by(nvr=nvr).one().update
+        up.locked = True
+        up.status = UpdateStatus.testing
+        up.request = None
+        # Clear pending messages
+        self.db.info['messages'] = []
+
+        args['edited'] = up.alias
+        args['stable_karma'] = 1
+
+        with fml_testing.mock_sends(update_schemas.UpdateEditV2):
+            up = self.app.post_json('/updates/', args, status=200).json_body
+
+        assert up['stable_karma'] == 2
+        assert up['caveats'][0]['description'] == (
+            'The stable karma required was raised to the mandatory release value of 2'
+        )
+
     def test_obsoletion_locked_with_open_request(self, *args):
         nvr = 'bodhi-2.0.0-2.fc17'
         args = self.get_update(nvr)
@@ -3997,12 +3928,16 @@ def test_invalid_stable_request(self, *args):
             {'request': 'stable', 'csrf_token': self.get_csrf_token()},
             status=400)
         assert resp.json['status'] == 'error'
-        assert resp.json['errors'][0]['description'] == config.get('not_yet_tested_msg')
+        exp = (
+            f"{config.get('not_yet_tested_msg')}: Test gating is disabled, but update has "
+            "less than 2 karma and has been in testing less than 7 days."
+        )
+        assert resp.json['errors'][0]['description'] == exp
 
-    def test_request_to_stable_based_on_stable_karma(self, *args):
+    def test_request_to_stable_based_on_minimum_karma(self, *args):
         """
-        Test request to stable before an update reaches stable karma
-        and after it reaches stable karma when autokarma is disabled
+        Test request to stable before and after an update reaches minimum
+        karma threshold when autokarma is disabled
         """
         user = User(name='bob')
         self.db.add(user)
@@ -4011,7 +3946,6 @@ def test_request_to_stable_based_on_stable_karma(self, *args):
         nvr = 'bodhi-2.0.0-2.fc17'
         args = self.get_update(nvr)
         args['autokarma'] = False
-        args['stable_karma'] = 1
         with fml_testing.mock_sends(update_schemas.UpdateReadyForTestingV3,
                                     update_schemas.UpdateRequestTestingV1):
             response = self.app.post_json('/updates/', args)
@@ -4020,13 +3954,15 @@ def test_request_to_stable_based_on_stable_karma(self, *args):
         up.status = UpdateStatus.testing
         up.request = None
         assert len(up.builds) == 1
+        # just to set our base expectation
+        assert up.release.min_karma == 2
         up.test_gating_status = TestGatingStatus.passed
         # Clear pending messages
         self.db.info['messages'] = []
         self.db.commit()
 
-        # Checks failure for requesting to stable push before the update reaches stable karma
-        up.comment(self.db, 'Not working', author='ralph', karma=0)
+        # Checks failure for requesting to stable push before the update reaches min karma
+        up.comment(self.db, 'LGTM', author='ralph', karma=1)
 
         with fml_testing.mock_sends(api.Message):
             self.app.post_json(
@@ -4038,8 +3974,8 @@ def test_request_to_stable_based_on_stable_karma(self, *args):
         assert up.request is None
         assert up.status == UpdateStatus.testing
 
-        # Checks Success for requesting to stable push after the update reaches stable karma
-        up.comment(self.db, 'LGTM', author='ralph', karma=1)
+        # Checks Success for requesting to stable push after the update reaches min karma
+        up.comment(self.db, 'LGTM', author='foo', karma=2)
 
         with fml_testing.mock_sends(api.Message, api.Message):
             self.app.post_json(
@@ -4854,7 +4790,7 @@ def _reach_autopush_threshold(self, up, publish):
         status = up.status
         request = up.request
         # just for clarity that stable_karma is not lower and both are reached
-        assert up.release.critpath_min_karma == 2
+        assert up.release.min_karma == 2
         up.comment(self.db, "foo", 1, 'biz')
         # nothing should have changed yet, on any path. we don't need to test the
         # messages published so far here, either
@@ -4878,7 +4814,7 @@ def _reach_autopush_threshold(self, up, publish):
     def test_autopush_reached_main(self, autokarma, cbb, critpath, status):
         """The update should be autopushed (request set to stable) if update reaches
         stable_karma, autokarma is on and release is composed by Bodhi, whether or not the update
-        is critical path (so long as stable_karma is not lower than critpath_min_karma), and
+        is critical path (so long as stable_karma is not lower than min_karma), and
         whether its status is testing or pending. This test covers the main cases for autopush.
         Subsequent tests cover some other cases where updates that otherwise would be autopushed
         are not; these aren't covered in this test for reasons explained in the docstring of
@@ -4959,40 +4895,41 @@ def test_autopush_reached_gating_failed(self):
     @pytest.mark.parametrize('critpath', (True, False))
     def test_autopush_reached_critpath_not(self, critpath, status):
         """If the stable_karma threshold is reached but it is lower than the release's
-        critpath_min_karma threshold and that is not reached, autopush should happen for
-        a non-critpath update but not for a critpath update. For a critpath update, if
-        status is testing, the request should not be changed; if it's pending, the
-        request should be changed to testing.
+        min_karma threshold and that is not reached, autopush should not happen
+        (whether or not the update is critical path). It should be very uncommon to
+        encounter this situation these days because we disallow setting the stable_karma
+        threshold lower than min_karma when creating or editing an update, but it
+        could theoretically happen if min_karma for a release was increased while an
+        update which had stable_karma set to the previous minimum value was in-flight.
         """
         up = self._prepare_autopush_update()
         up.critpath = critpath
         up.status = status
         up.stable_karma = 1
-        assert up.release.critpath_min_karma == 2
+        assert up.release.min_karma == 2
         # we don't use _reach_autopush_threshold here because this is a bit different
         with mock.patch('bodhi.server.models.notifications.publish', autospec=True) as publish:
             _, caveats = up.comment(self.db, "foo", 1, 'biz')
             msgs = [type(call[0][0]) for call in publish.call_args_list]
         assert up.karma == 1
-        if critpath and status is UpdateStatus.testing:
+        if status is UpdateStatus.testing:
             assert up.request is None
             # we should not set this, really, but currently we do
             assert up.date_approved is not None
             assert msgs == [update_schemas.UpdateCommentV1]
+            days = 7
+            if critpath:
+                days = 14
             assert caveats == (
                 [{'name': 'karma',
-                  'description': ('This critical path update has not yet been approved for pushing '
-                                  'to the stable repository.  It must first reach a karma of 2, '
-                                  'consisting of 0 positive karma from proventesters, along with 2 '
-                                  'additional karma from the community. Or, it must spend 14 days '
-                                  'in testing without any negative feedback')}])
+                  'description': ('This update has not yet met the minimum testing requirements '
+                                  'defined in the <a href="https://fedoraproject.org/wiki/'
+                                  'Package_update_acceptance_criteria">Package Update Acceptance '
+                                  'Criteria</a>: Test gating is disabled, but update has less than '
+                                  f'2 karma and has been in testing less than {days} days.')}])
         else:
-            if critpath:
-                assert up.request is UpdateRequest.testing
-                reqmsg = update_schemas.UpdateRequestTestingV1
-            else:
-                assert up.request is UpdateRequest.stable
-                reqmsg = update_schemas.UpdateRequestStableV1
+            assert up.request is UpdateRequest.testing
+            reqmsg = update_schemas.UpdateRequestTestingV1
             assert up.date_approved is not None
             assert msgs == [
                 reqmsg,
@@ -5076,43 +5013,29 @@ def test_edit_button_not_present_when_stable(self, *args):
         assert 'Push to Stable' not in resp
         assert '<span class="fa fa-fw fa-pencil-square-o"></span> Edit' not in resp
 
-    @mock.patch.dict('bodhi.server.models.config', {'test_gating.required': True})
-    @mock.patch('bodhi.server.models.Update.update_test_gating_status')
-    def test_push_to_stable_button_not_present_when_test_gating_status_failed(self, update_tg):
-        """The push to stable button should not appear if the test_gating_status is failed."""
-        nvr = 'bodhi-2.0.0-2.fc17'
-        args = self.get_update(nvr)
-        update_tg.return_value = TestGatingStatus.failed
-
-        with fml_testing.mock_sends(update_schemas.UpdateReadyForTestingV3,
-                                    update_schemas.UpdateRequestTestingV1):
-            resp = self.app.post_json('/updates/', args, headers={'Accept': 'application/json'})
-
-        update = Update.get(resp.json['alias'])
-        update.status = UpdateStatus.testing
-        update.request = None
-        update.pushed = True
-        update.autokarma = False
-        update.stable_karma = 1
-        update.test_gating_status = TestGatingStatus.failed
-        update.comment(self.db, 'works', 1, 'bowlofeggs')
-        # Let's clear any messages that might get sent
-        self.db.info['messages'] = []
-        self.registry.settings['test_gating.required'] = True
-
-        resp = self.app.get(f'/updates/{update.alias}', headers={'Accept': 'text/html'})
-
-        assert 'Push to Stable' not in resp
-        assert 'Edit' in resp
-
-    @mock.patch.dict('bodhi.server.models.config', {'test_gating.required': True})
-    @mock.patch('bodhi.server.models.Update.update_test_gating_status')
-    def test_push_to_stable_button_present_when_test_gating_status_passed(self, update_tg):
-        """The push to stable button should appear if the test_gating_status is passed."""
+    @pytest.mark.parametrize(
+        'tgon,tgmet,karmamet,timemet,urgent,critpath,frozen,present',
+        (
+            (True, True, True, False, False, False, False, True),
+            (True, True, True, False, True, False, False, True),
+            (True, True, True, False, False, True, False, True),
+            (True, True, True, False, True, False, True, False),
+            (True, False, True, True, False, False, False, False),
+            (False, False, True, False, False, False, False, True),
+            (False, False, False, True, False, False, False, True),
+            (False, False, False, False, False, False, False, False),
+            (False, False, False, False, True, False, False, False),
+            (False, False, False, False, False, True, False, False),
+        )
+    )
+    def test_push_to_stable_button_presence(
+        self, tgon, tgmet, karmamet, timemet, urgent, critpath, frozen, present
+    ):
+        """The push to stable button should be present when the update meets
+        requirements, but not otherwise. Whether the update is urgent or
+        critical path should not matter."""
         nvr = 'bodhi-2.0.0-2.fc17'
         args = self.get_update(nvr)
-        update_tg.return_value = TestGatingStatus.passed
-
         with fml_testing.mock_sends(update_schemas.UpdateReadyForTestingV3,
                                     update_schemas.UpdateRequestTestingV1):
             resp = self.app.post_json('/updates/', args, headers={'Accept': 'application/json'})
@@ -5122,197 +5045,38 @@ def test_push_to_stable_button_present_when_test_gating_status_passed(self, upda
         update.request = None
         update.pushed = True
         update.autokarma = False
-        update.stable_karma = 1
-        update.test_gating_status = TestGatingStatus.passed
-        update.comment(self.db, 'works', 1, 'bowlofeggs')
-        # Let's clear any messages that might get sent
-        self.db.info['messages'] = []
-        self.registry.settings['test_gating.required'] = True
-
-        resp = self.app.get(f'/updates/{update.alias}', headers={'Accept': 'text/html'})
-
-        assert 'Push to Stable' in resp
-        assert 'Edit' in resp
-
-    def test_push_to_stable_button_present_when_karma_reached(self, *args):
-        """
-        Assert that the "Push to Stable" button appears when the required karma is
-        reached.
-        """
-        nvr = 'bodhi-2.0.0-2.fc17'
-        args = self.get_update(nvr)
-
-        with fml_testing.mock_sends(update_schemas.UpdateReadyForTestingV3,
-                                    update_schemas.UpdateRequestTestingV1):
-            resp = self.app.post_json('/updates/', args)
-
-        update = Update.get(resp.json['alias'])
-        update.status = UpdateStatus.testing
-        update.request = None
-        update.pushed = True
-        update.autokarma = False
-        update.stable_karma = 1
-        update.comment(self.db, 'works', 1, 'bowlofeggs')
-        # Let's clear any messages that might get sent
-        self.db.info['messages'] = []
-
-        resp = self.app.get(f'/updates/{update.alias}', headers={'Accept': 'text/html'})
-
-        # Checks Push to Stable text in the html page for this update
-        assert 'text/html' in resp.headers['Content-Type']
-        assert nvr in resp
-        assert 'Push to Stable' in resp
-        assert 'Edit' in resp
-
-    def test_push_to_stable_button_present_when_karma_reached_urgent(self, *args):
-        """
-        Assert that the "Push to Stable" button appears when the required karma is
-        reached for an urgent update.
-        """
-        nvr = 'bodhi-2.0.0-2.fc17'
-        args = self.get_update(nvr)
-
-        with fml_testing.mock_sends(update_schemas.UpdateReadyForTestingV3,
-                                    update_schemas.UpdateRequestTestingV1):
-            resp = self.app.post_json('/updates/', args)
-
-        update = Update.get(resp.json['alias'])
-        update.severity = UpdateSeverity.urgent
-        update.status = UpdateStatus.testing
-        update.request = None
-        update.pushed = True
-        update.autokarma = False
-        update.stable_karma = 1
-        update.comment(self.db, 'works', 1, 'bowlofeggs')
-        # Let's clear any messages that might get sent
-        self.db.info['messages'] = []
-
-        resp = self.app.get(f'/updates/{update.alias}', headers={'Accept': 'text/html'})
-
-        # Checks Push to Stable text in the html page for this update
-        assert 'text/html' in resp.headers['Content-Type']
-        assert nvr in resp
-        assert 'Push to Stable' in resp
-        assert 'Edit' in resp
-
-    def test_push_to_stable_button_present_when_time_reached(self, *args):
-        """
-        Assert that the "Push to Stable" button appears when the required time in testing is
-        reached.
-        """
-        nvr = 'bodhi-2.0.0-2.fc17'
-        args = self.get_update(nvr)
-
-        with fml_testing.mock_sends(update_schemas.UpdateReadyForTestingV3,
-                                    update_schemas.UpdateRequestTestingV1):
-            resp = self.app.post_json('/updates/', args)
-
-        update = Update.get(resp.json['alias'])
-        update.status = UpdateStatus.testing
-        update.request = None
-        update.pushed = True
-        # This update has been in testing a while, so a "Push to Stable" button should appear.
-        update.date_testing = datetime.now() - timedelta(days=30)
-        # Let's clear any messages that might get sent
-        self.db.info['messages'] = []
-
-        resp = self.app.get(f'/updates/{update.alias}', headers={'Accept': 'text/html'})
-
-        # Checks Push to Stable text in the html page for this update
-        assert 'text/html' in resp.headers['Content-Type']
-        assert nvr in resp
-        assert 'Push to Stable' in resp
-        assert 'Edit' in resp
-
-    def test_push_to_stable_button_present_when_time_reached_and_urgent(self, *args):
-        """
-        Assert that the "Push to Stable" button appears when the required time in testing is
-        reached.
-        """
-        nvr = 'bodhi-2.0.0-2.fc17'
-        args = self.get_update(nvr)
-
-        with fml_testing.mock_sends(update_schemas.UpdateReadyForTestingV3,
-                                    update_schemas.UpdateRequestTestingV1):
-            resp = self.app.post_json('/updates/', args)
-
-        update = Update.get(resp.json['alias'])
-        update.severity = UpdateSeverity.urgent
-        update.status = UpdateStatus.testing
-        update.request = None
-        update.pushed = True
-        # This urgent update has been in testing a while, so a "Push to Stable" button should
-        # appear.
-        update.date_testing = datetime.now() - timedelta(days=30)
-        # Let's clear any messages that might get sent
-        self.db.info['messages'] = []
-
-        resp = self.app.get(f'/updates/{update.alias}', headers={'Accept': 'text/html'})
-
-        # Checks Push to Stable text in the html page for this update
-        assert 'text/html' in resp.headers['Content-Type']
-        assert nvr in resp
-        assert 'Push to Stable' in resp
-        assert 'Edit' in resp
-
-    def test_push_to_stable_button_present_when_time_reached_critpath(self, *args):
-        """
-        Assert that the "Push to Stable" button appears when it should for a critpath update.
-        """
-        nvr = 'bodhi-2.0.0-2.fc17'
-        args = self.get_update(nvr)
-
-        with fml_testing.mock_sends(update_schemas.UpdateReadyForTestingV3,
-                                    update_schemas.UpdateRequestTestingV1):
-            resp = self.app.post_json('/updates/', args)
-
-        update = Update.get(resp.json['alias'])
-        update.status = UpdateStatus.testing
-        update.request = None
-        update.pushed = True
-        update.critpath = True
-        # This update has been in testing a while, so a "Push to Stable" button should appear.
-        update.date_testing = datetime.now() - timedelta(days=30)
-        # Let's clear any messages that might get sent
-        self.db.info['messages'] = []
-
-        resp = self.app.get(f'/updates/{update.alias}', headers={'Accept': 'text/html'})
-
-        # Checks Push to Stable text in the html page for this update
-        assert 'text/html' in resp.headers['Content-Type']
-        assert nvr in resp
-        assert 'Push to Stable' in resp
-        assert 'Edit' in resp
-
-    def test_push_to_stable_button_not_present_when_karma_reached_and_frozen_release(self, *args):
-        """
-        Assert that the "Push to Stable" button is not displayed when the required karma is
-        reached, but the release is frozen and the update is still pending.
-        """
-        nvr = 'bodhi-2.0.0-2.fc17'
-        args = self.get_update(nvr)
-
-        with fml_testing.mock_sends(update_schemas.UpdateReadyForTestingV3,
-                                    update_schemas.UpdateRequestTestingV1):
-            resp = self.app.post_json('/updates/', args)
-
-        update = Update.get(resp.json['alias'])
-        update.status = UpdateStatus.pending
-        update.request = UpdateRequest.testing
-        update.pushed = False
-        update.autokarma = False
-        update.stable_karma = 1
-        update.release.state = ReleaseState.frozen
-        update.comment(self.db, 'works', 1, 'bowlofeggs')
+        if urgent:
+            update.severity = UpdateSeverity.urgent
+        if critpath:
+            update.critpath = True
+        if frozen:
+            # specific set of conditions we want to make sure the
+            # button does not appear in
+            update.release.state = ReleaseState.frozen
+            update.status = UpdateStatus.pending
+            update.pushed = False
+        if tgmet:
+            update.test_gating_status = TestGatingStatus.passed
+        else:
+            update.test_gating_status = TestGatingStatus.failed
+        if karmamet:
+            update.comment(self.db, 'works', 1, 'bowlofeggs')
+            update.comment(self.db, 'works', 1, 'ralph')
+        if timemet:
+            # This update has been in testing a while, so a "Push to Stable" button should appear.
+            update.date_testing = datetime.now() - timedelta(days=30)
         # Let's clear any messages that might get sent
         self.db.info['messages'] = []
 
-        resp = self.app.get(f'/updates/{update.alias}', headers={'Accept': 'text/html'})
+        with mock.patch.dict(config, [('test_gating.required', tgon)]):
+            resp = self.app.get(f'/updates/{update.alias}', headers={'Accept': 'text/html'})
 
-        # Checks Push to Stable text in the html page for this update
         assert 'text/html' in resp.headers['Content-Type']
         assert nvr in resp
-        assert 'Push to Stable' not in resp
+        if present:
+            assert 'Push to Stable' in resp
+        else:
+            assert 'Push to Stable' not in resp
         assert 'Edit' in resp
 
     def assert_severity_html(self, severity, text=()):
@@ -5406,115 +5170,6 @@ def test_update_severity_label_absent_when_severity_is_None(self, *args):
         assert nvr in resp
         assert '<strong>Update Severity</strong>' not in resp
 
-    def test_manually_push_to_stable_based_on_karma_request_none(self, *args):
-        """
-        Test manually push to stable when autokarma is disabled
-        and karma threshold is reached
-
-        This test starts the update with request None.
-        """
-        user = User(name='bob')
-        self.db.add(user)
-        self.db.commit()
-
-        # Makes autokarma disabled
-        # Sets stable karma to 1
-        nvr = 'bodhi-2.0.0-2.fc17'
-        args = self.get_update(nvr)
-        args['autokarma'] = False
-        args['stable_karma'] = 1
-
-        with fml_testing.mock_sends(update_schemas.UpdateReadyForTestingV3,
-                                    update_schemas.UpdateRequestTestingV1):
-            resp = self.app.post_json('/updates/', args)
-
-        # Marks it as no request
-        upd = Update.get(resp.json['alias'])
-        upd.status = UpdateStatus.testing
-        upd.request = None
-        upd.pushed = True
-        upd.date_testing = datetime.now() - timedelta(days=1)
-        # Clear pending messages
-        self.db.info['messages'] = []
-        self.db.commit()
-
-        # Checks karma threshold is reached
-        # Makes sure stable karma is not None
-        # Ensures Request doesn't get set to stable automatically since autokarma is disabled
-        upd.comment(self.db, 'LGTM', author='ralph', karma=1)
-        upd = Update.get(upd.alias)
-        assert upd.karma == 1
-        assert upd.stable_karma == 1
-        assert upd.status == UpdateStatus.testing
-        assert upd.request is None
-        assert upd.autokarma is False
-        assert upd.pushed is True
-
-        text = str(config.get('testing_approval_msg'))
-        upd.comment(self.db, text, author='bodhi')
-        # Let's clear any messages that might get sent
-        self.db.info['messages'] = []
-
-        # Checks Push to Stable text in the html page for this update
-        resp = self.app.get(f'/updates/{upd.alias}',
-                            headers={'Accept': 'text/html'})
-        assert 'text/html' in resp.headers['Content-Type']
-        assert upd.builds[0].nvr in resp
-        assert 'Push to Stable' in resp
-
-    def test_manually_push_to_stable_based_on_karma_request_testing(self, *args):
-        """
-        Test manually push to stable when autokarma is disabled
-        and karma threshold is reached. Ensure that the option/button to push to
-        stable is not present prior to entering the stable request state.
-
-        This test starts the update with request testing.
-        """
-
-        # Disabled
-        # Sets stable karma to 1
-        nvr = 'bodhi-2.0.0-2.fc17'
-        args = self.get_update(nvr)
-        args['autokarma'] = False
-        args['stable_karma'] = 1
-
-        with fml_testing.mock_sends(update_schemas.UpdateReadyForTestingV3,
-                                    update_schemas.UpdateRequestTestingV1):
-            resp = self.app.post_json('/updates/', args)
-
-        # Marks it as testing
-        upd = Update.get(resp.json['alias'])
-        upd.status = UpdateStatus.testing
-        upd.pushed = True
-        upd.request = None
-        upd.date_testing = datetime.now() - timedelta(days=1)
-        # Clear pending messages
-        self.db.info['messages'] = []
-        self.db.commit()
-
-        # Checks karma threshold is reached
-        # Makes sure stable karma is not None
-        # Ensures Request doesn't get set to stable automatically since autokarma is disabled
-        upd.comment(self.db, 'LGTM', author='ralph', karma=1)
-        upd = Update.get(upd.alias)
-        assert upd.karma == 1
-        assert upd.stable_karma == 1
-        assert upd.status == UpdateStatus.testing
-        assert upd.request is None
-        assert upd.autokarma is False
-
-        text = str(config.get('testing_approval_msg'))
-        upd.comment(self.db, text, author='bodhi')
-        # Let's clear any messages that might get sent
-        self.db.info['messages'] = []
-
-        # Checks Push to Stable text in the html page for this update
-        resp = self.app.get(f'/updates/{upd.alias}',
-                            headers={'Accept': 'text/html'})
-        assert 'text/html' in resp.headers['Content-Type']
-        assert upd.builds[0].nvr in resp
-        assert 'Push to Stable' in resp
-
     @mock.patch('bodhi.server.services.updates.handle_side_and_related_tags_task', mock.Mock())
     @mock.patch('bodhi.server.models.tag_update_builds_task', mock.Mock())
     def test_edit_update_with_expired_override(self, *args):
@@ -5758,7 +5413,6 @@ def test_meets_testing_requirements_since_karma_reset_critpath(self, *args):
         update.request = None
         update.critpath = True
         update.autokarma = True
-        update.date_testing = datetime.utcnow() + timedelta(days=-20)
         update.comment(self.db, 'lgtm', author='friend', karma=1)
         update.comment(self.db, 'lgtm', author='friend2', karma=1)
         update.comment(self.db, 'bad', author='enemy', karma=-1)
diff --git a/bodhi-server/tests/tasks/test_approve_testing.py b/bodhi-server/tests/tasks/test_approve_testing.py
index dd4fb2be6a..494cad2706 100644
--- a/bodhi-server/tests/tasks/test_approve_testing.py
+++ b/bodhi-server/tests/tasks/test_approve_testing.py
@@ -175,6 +175,9 @@ def test_update_approved_and_autotime(self, delete_tag, remove_tag, add_tag,
         self.update.autotime = True
         # a sprinkling of possible cases just to make sure our logic isn't broken
         # and we're not somehow relying on defaults
+        if stable_days < 7:
+            # our test config sets it to 7, so need to override it
+            config["fedora.mandatory_days_in_testing"] = stable_days
         self.update.stable_days = stable_days
         if stable_days > 0:
             # in this case we need to make sure we look like we've been in testing
diff --git a/bodhi-server/tests/tasks/test_composer.py b/bodhi-server/tests/tasks/test_composer.py
index 2b470cf313..96b4dec889 100644
--- a/bodhi-server/tests/tasks/test_composer.py
+++ b/bodhi-server/tests/tasks/test_composer.py
@@ -483,8 +483,14 @@ def test_tags(self, *args):
             comp = session.query(Compose).first()
             if comp is not None:
                 session.delete(comp)
-            # Set the update request to stable and the release to pending
             up = session.query(Update).one()
+            # Have the update reach the stable karma threshold
+            assert up.karma == 1
+            up.comment(session, "foo", 1, 'foo')
+            # Clear pending messages
+            self.db.info['messages'] = []
+            assert up.karma == 2
+            # Set the update request to stable and the release to pending
             up.release.state = ReleaseState.pending
             up.request = UpdateRequest.stable
 
@@ -948,6 +954,10 @@ def test_security_update_priority(self, *args):
 
         with self.db_factory() as db:
             up = db.query(Update).one()
+            # Have the update reach the stable karma threshold
+            assert up.karma == 1
+            up.comment(db, "foo", 1, 'foo')
+            assert up.karma == 2
             user = db.query(User).first()
 
             # Create a security update for a different release
@@ -980,6 +990,10 @@ def test_security_update_priority(self, *args):
                 type=UpdateType.security)
 
             db.add(update)
+            # Have the update reach the stable karma threshold
+            update.comment(db, "foo", 1, 'foo')
+            update.comment(db, "test", 1, 'test')
+            assert update.karma == 2
 
             update.test_gating_status = TestGatingStatus.passed
 
@@ -1030,6 +1044,10 @@ def test_security_update_priority_testing(self, *args):
             up = db.query(Update).one()
             up.type = UpdateType.security
             up.request = UpdateRequest.testing
+            # Have the update reach the stable karma threshold
+            assert up.karma == 1
+            up.comment(db, "foo", 1, 'foo')
+            assert up.karma == 2
             user = db.query(User).first()
 
             # Create a security update for a different release
@@ -1062,6 +1080,10 @@ def test_security_update_priority_testing(self, *args):
                 type=UpdateType.enhancement)
 
             db.add(update)
+            # Have the update reach the stable karma threshold
+            update.comment(db, "foo", 1, 'foo')
+            update.comment(db, "test", 1, 'test')
+            assert update.karma == 2
 
             update.test_gating_status = TestGatingStatus.passed
 
@@ -1100,6 +1122,10 @@ def test_security_updates_parallel(self, *args):
 
         with self.db_factory() as db:
             up = db.query(Update).one()
+            # Have the update reach the stable karma threshold
+            assert up.karma == 1
+            up.comment(db, "foo", 1, 'foo')
+            assert up.karma == 2
             up.type = UpdateType.security
             up.status = UpdateStatus.testing
             up.request = UpdateRequest.stable
@@ -1135,6 +1161,10 @@ def test_security_updates_parallel(self, *args):
                 type=UpdateType.security)
 
             db.add(update)
+            # Have the update reach the stable karma threshold
+            update.comment(db, "foo", 1, 'foo')
+            update.comment(db, "test", 1, 'test')
+            assert update.karma == 2
 
             update.test_gating_status = TestGatingStatus.passed
 
@@ -1237,10 +1267,11 @@ def test_compose_late_exit(self, *args):
     @mock.patch('bodhi.server.tasks.composer.PungiComposerThread._wait_for_repo_signature')
     @mock.patch('bodhi.server.tasks.composer.PungiComposerThread._wait_for_sync')
     @mock.patch('bodhi.server.tasks.composer.time.sleep')
-    def test_clean_old_composes_false(self, *args):
-        """Test work() with clean_old_composes set to False."""
+    @pytest.mark.parametrize('clean_old', (False, True))
+    def test_clean_old_composes_param(self, sleep, wfs, wfrs, stage, scr, clean_old):
+        """Test work() with clean_old_composes set to False or True."""
         self.expected_sems = 1
-        config["clean_old_composes"] = False
+        config["clean_old_composes"] = clean_old
 
         # Set the request to stable right out the gate so we can test gating
         self.set_stable_request('bodhi-2.0-1.fc17')
@@ -1275,6 +1306,7 @@ def test_clean_old_composes_false(self, *args):
                               'ralph', self.db_factory, compose_dir)
         t.keep_old_composes = 2
         expected_messages = (
+            update_schemas.UpdateCommentV1,
             compose_schemas.ComposeComposingV1,
             override_schemas.BuildrootOverrideUntagV1,
             update_schemas.UpdateCompleteStableV1,
@@ -1283,119 +1315,39 @@ def test_clean_old_composes_false(self, *args):
                 success=True, repo='f17-updates', ctype='rpm', agent='ralph')))
 
         with self.db_factory() as session:
-            with mock.patch('bodhi.server.tasks.composer.subprocess.Popen') as Popen:
-                release = session.query(Release).filter_by(name='F17').one()
-                Popen.side_effect = self._generate_fake_pungi(t, 'stable_tag', release)
-                with mock_sends(*expected_messages):
-                    t.run()
-
-        actual_dirs = set([
-            d for d in os.listdir(compose_dir)
-            if os.path.isdir(os.path.join(compose_dir, d))
-            and not d.startswith("Fedora-17-updates")])
-
-        # No dirs should have been removed since we had clean_old_composes set False.
-        assert actual_dirs == dirs
-        # The cool file should still be here
-        actual_files = [f for f in os.listdir(compose_dir)
-                        if os.path.isfile(os.path.join(compose_dir, f))]
-        assert actual_files == ['COOL_FILE.txt']
-
-        assert Popen.mock_calls == \
-            [mock.call(
-                [config['pungi.cmd'], '--config', '{}/pungi.conf'.format(t._pungi_conf_dir),
-                 '--quiet', '--print-output-dir', '--target-dir', t.compose_dir, '--old-composes',
-                 t.compose_dir, '--no-latest-link', '--label', t._label],
-                cwd=t.compose_dir, shell=False, stderr=-1,
-                stdin=mock.ANY,
-                stdout=mock.ANY)]
-        d = datetime.datetime.utcnow()
-        assert t._checkpoints == \
-            {'completed_repo': os.path.join(
-                compose_dir, 'Fedora-17-updates-{}{:02}{:02}.0'.format(d.year, d.month, d.day)),
-             'compose_done': True,
-             'determine_and_perform_tag_actions': True,
-             'modify_bugs': True,
-             'send_stable_announcements': True,
-             'send_testing_digest': True,
-             'status_comments': True}
-        assert os.path.exists(compose_dir)
-
-    @mock.patch('bodhi.server.tasks.composer.PungiComposerThread._sanity_check_repo')
-    @mock.patch('bodhi.server.tasks.composer.PungiComposerThread._stage_repo')
-    @mock.patch('bodhi.server.tasks.composer.PungiComposerThread._wait_for_repo_signature')
-    @mock.patch('bodhi.server.tasks.composer.PungiComposerThread._wait_for_sync')
-    @mock.patch('bodhi.server.tasks.composer.time.sleep')
-    def test_clean_old_composes_true(self, *args):
-        """Test work() with clean_old_composes set to True."""
-        config["clean_old_composes"] = True
-        self.expected_sems = 1
-
-        # Set the request to stable right out the gate so we can test gating
-        self.set_stable_request('bodhi-2.0-1.fc17')
-        task = self._make_task()
-        compose_dir = os.path.join(self.tempdir, 'cool_dir')
-        config["compose_dir"] = compose_dir
-
-        # Set up some directories that look similar to what might be found in production, with
-        # some directories that don't match the pattern of ending in -<timestamp>.
-        dirs = [
-            'dist-5E-epel-161003.0724', 'dist-5E-epel-161011.0458', 'dist-5E-epel-161012.1854',
-            'dist-5E-epel-161013.1711', 'dist-5E-epel-testing-161001.0424',
-            'dist-5E-epel-testing-161003.0856', 'dist-5E-epel-testing-161006.0053',
-            'dist-6E-epel-161002.2331', 'dist-6E-epel-161003.2046',
-            'dist-6E-epel-testing-161001.0528', 'epel7-161003.0724', 'epel7-161003.2046',
-            'epel7-161004.1423', 'epel7-161005.1122', 'epel7-testing-161001.0424',
-            'epel7-testing-161003.0621', 'epel7-testing-161003.2217', 'f23-updates-161002.2331',
-            'f23-updates-161003.1302', 'f23-updates-161004.1423', 'f23-updates-161005.0259',
-            'f23-updates-testing-161001.0424', 'f23-updates-testing-161003.0621',
-            'f23-updates-testing-161003.2217', 'f24-updates-161002.2331',
-            'f24-updates-161003.1302', 'f24-updates-testing-161001.0424',
-            'this_should_get_left_alone', 'f23-updates-should_be_untouched',
-            'f23-updates.repocache', 'f23-updates-testing-blank']
-        [os.makedirs(os.path.join(compose_dir, d)) for d in dirs]
-        # Now let's make a few files here and there.
-        with open(os.path.join(compose_dir, 'dist-5E-epel-161003.0724', 'oops.txt'), 'w') as oops:
-            oops.write('This compose failed to get cleaned and left this file around, oops!')
-        with open(os.path.join(compose_dir, 'COOL_FILE.txt'), 'w') as cool_file:
-            cool_file.write('This file should be allowed to hang out here because it\'s cool.')
-
-        t = RPMComposerThread(self.semmock, task['composes'][0],
-                              'ralph', self.db_factory, compose_dir)
-        t.keep_old_composes = 2
-        expected_messages = (
-            compose_schemas.ComposeComposingV1,
-            override_schemas.BuildrootOverrideUntagV1,
-            update_schemas.UpdateCompleteStableV1,
-            errata_schemas.ErrataPublishV1,
-            compose_schemas.ComposeCompleteV1.from_dict(dict(
-                success=True, repo='f17-updates', ctype='rpm', agent='ralph')))
+            # have the update reach karma threshold
+            up = session.query(Update).one()
+            assert up.karma == 1
+            up.comment(session, "foo", 1, 'foo')
+            assert up.karma == 2
 
-        with self.db_factory() as session:
             with mock.patch('bodhi.server.tasks.composer.subprocess.Popen') as Popen:
                 release = session.query(Release).filter_by(name='F17').one()
                 Popen.side_effect = self._generate_fake_pungi(t, 'stable_tag', release)
                 with mock_sends(*expected_messages):
                     t.run()
 
-        # We expect these and only these directories to remain.
-        expected_dirs = {
-            'dist-5E-epel-161012.1854', 'dist-5E-epel-161013.1711',
-            'dist-5E-epel-testing-161003.0856', 'dist-5E-epel-testing-161006.0053',
-            'dist-6E-epel-161002.2331', 'dist-6E-epel-161003.2046',
-            'dist-6E-epel-testing-161001.0528', 'epel7-161004.1423', 'epel7-161005.1122',
-            'epel7-testing-161003.0621', 'epel7-testing-161003.2217', 'f23-updates-161004.1423',
-            'f23-updates-161005.0259', 'f23-updates-testing-161003.0621',
-            'f23-updates-testing-161003.2217', 'f24-updates-161002.2331',
-            'f24-updates-161003.1302', 'f24-updates-testing-161001.0424',
-            'this_should_get_left_alone', 'f23-updates-should_be_untouched',
-            'f23-updates.repocache', 'f23-updates-testing-blank'}
         actual_dirs = set([
             d for d in os.listdir(compose_dir)
             if os.path.isdir(os.path.join(compose_dir, d))
             and not d.startswith("Fedora-17-updates")])
 
-        # Assert that remove_old_composes removes the correct items and leaves the rest in place.
+        if clean_old:
+            # We expect these and only these directories to remain.
+            expected_dirs = {
+                'dist-5E-epel-161012.1854', 'dist-5E-epel-161013.1711',
+                'dist-5E-epel-testing-161003.0856', 'dist-5E-epel-testing-161006.0053',
+                'dist-6E-epel-161002.2331', 'dist-6E-epel-161003.2046',
+                'dist-6E-epel-testing-161001.0528', 'epel7-161004.1423', 'epel7-161005.1122',
+                'epel7-testing-161003.0621', 'epel7-testing-161003.2217', 'f23-updates-161004.1423',
+                'f23-updates-161005.0259', 'f23-updates-testing-161003.0621',
+                'f23-updates-testing-161003.2217', 'f24-updates-161002.2331',
+                'f24-updates-161003.1302', 'f24-updates-testing-161001.0424',
+                'this_should_get_left_alone', 'f23-updates-should_be_untouched',
+                'f23-updates.repocache', 'f23-updates-testing-blank'}
+        else:
+            # No dirs should have been removed since we had clean_old_composes set False.
+            expected_dirs = dirs
         assert actual_dirs == expected_dirs
         # The cool file should still be here
         actual_files = [f for f in os.listdir(compose_dir)
@@ -1440,6 +1392,7 @@ def test_compose(self, *args):
                               'ralph', self.db_factory, compose_dir)
         t.keep_old_composes = 2
         expected_messages = (
+            update_schemas.UpdateCommentV1,
             compose_schemas.ComposeComposingV1,
             override_schemas.BuildrootOverrideUntagV1,
             update_schemas.UpdateCompleteStableV1,
@@ -1448,6 +1401,12 @@ def test_compose(self, *args):
                 success=True, repo='f17-updates', ctype='rpm', agent='ralph')))
 
         with self.db_factory() as session:
+            # have the update reach karma threshold
+            up = session.query(Update).one()
+            assert up.karma == 1
+            up.comment(session, "foo", 1, 'foo')
+            assert up.karma == 2
+
             with mock.patch('bodhi.server.tasks.composer.subprocess.Popen') as Popen:
                 release = session.query(Release).filter_by(name='F17').one()
                 Popen.side_effect = self._generate_fake_pungi(t, 'stable_tag', release)
@@ -1509,6 +1468,10 @@ def test_compose_module(self, *args):
                 type=UpdateType.security)
 
             db.add(update)
+            # Have the update reach the stable karma threshold
+            update.comment(db, "foo", 1, 'foo')
+            update.comment(db, "test", 1, 'test')
+            assert update.karma == 2
 
             update.test_gating_status = TestGatingStatus.passed
 
@@ -1622,6 +1585,7 @@ def test_test_gating_status_failed(self, *args):
             self.semmock, task['composes'][0], 'ralph', self.db_factory,
             self.tempdir)
         expected_messages = (
+            update_schemas.UpdateCommentV1,
             compose_schemas.ComposeComposingV1.from_dict({
                 'repo': u'f17-updates',
                 'ctype': 'rpm',
@@ -1632,6 +1596,12 @@ def test_test_gating_status_failed(self, *args):
                 success=True, repo='f17-updates', ctype='rpm', agent='ralph')))
 
         with self.db_factory() as session:
+            # have the update reach karma threshold
+            up = session.query(Update).one()
+            assert up.karma == 1
+            up.comment(session, "foo", 1, 'foo')
+            assert up.karma == 2
+
             with mock.patch('bodhi.server.tasks.composer.subprocess.Popen') as Popen:
                 release = session.query(Release).filter_by(name='F17').one()
                 Popen.side_effect = self._generate_fake_pungi(t, 'stable_tag', release)
@@ -1665,6 +1635,7 @@ def test_test_gating_status_passed(self, *args):
             self.semmock, task['composes'][0], 'ralph', self.db_factory,
             self.tempdir)
         expected_messages = (
+            update_schemas.UpdateCommentV1,
             compose_schemas.ComposeComposingV1.from_dict(
                 {'repo': 'f17-updates', 'updates': [u.builds[0].nvr], 'agent': 'ralph',
                  'ctype': 'rpm'}),
@@ -1676,6 +1647,12 @@ def test_test_gating_status_passed(self, *args):
                 success=True, ctype='rpm', repo='f17-updates', agent='ralph')))
 
         with self.db_factory() as session:
+            # have the update reach karma threshold
+            up = session.query(Update).one()
+            assert up.karma == 1
+            up.comment(session, "foo", 1, 'foo')
+            assert up.karma == 2
+
             with mock.patch('bodhi.server.tasks.composer.subprocess.Popen') as Popen:
                 release = session.query(Release).filter_by(name='F17').one()
                 Popen.side_effect = self._generate_fake_pungi(t, 'stable_tag', release)
@@ -1684,7 +1661,7 @@ def test_test_gating_status_passed(self, *args):
                     # t.run() modified some of the objects we used to construct the expected
                     # messages above, so we need to inject the altered data into them so the
                     # assertions are correct.
-                    expected_messages[1].body['override'] = u.builds[0].override.__json__()
+                    expected_messages[2].body['override'] = u.builds[0].override.__json__()
 
         u = Build.query.filter_by(nvr='bodhi-2.0-1.fc17').one().update
         assert u.comments[-1].text == 'This update has been pushed to stable.'
@@ -1735,6 +1712,15 @@ def test_modify_stable_bugs(self, close, comment, *args):
         self.set_stable_request('bodhi-2.0-1.fc17')
         task = self._make_task()
 
+        with self.db_factory() as session:
+            # have the update reach karma threshold
+            up = session.query(Update).one()
+            assert up.karma == 1
+            up.comment(session, "foo", 1, 'foo')
+            # Clear pending messages
+            self.db.info['messages'] = []
+            assert up.karma == 2
+
         t = RPMComposerThread(self.semmock, task['composes'][0],
                               'ralph', self.db_factory, self.tempdir)
 
@@ -1785,7 +1771,13 @@ def test_status_comment_stable(self, *args):
         with self.db_factory() as session:
             up = session.query(Update).one()
             up.request = UpdateRequest.stable
-            assert len(up.comments) == 2
+            # Have the update reach the stable karma threshold
+            assert up.karma == 1
+            up.comment(session, "foo", 1, 'foo')
+            # Clear pending messages
+            self.db.info['messages'] = []
+            assert up.karma == 2
+            assert len(up.comments) == 3
 
         with mock_sends(*[base_schemas.BodhiMessage] * 6):
             task = self._make_task()
@@ -1794,7 +1786,7 @@ def test_status_comment_stable(self, *args):
 
         with self.db_factory() as session:
             up = session.query(Update).one()
-            assert len(up.comments) == 3
+            assert len(up.comments) == 4
             assert up.comments[-1]['text'] == 'This update has been pushed to stable.'
 
     @mock.patch('bodhi.server.tasks.composer.PungiComposerThread._sanity_check_repo')
@@ -1835,7 +1827,13 @@ def test_unlock_updates(self, *args):
         with self.db_factory() as session:
             up = session.query(Update).one()
             up.request = UpdateRequest.stable
-            assert len(up.comments) == 2
+            # Have the update reach the stable karma threshold
+            assert up.karma == 1
+            up.comment(session, "foo", 1, 'foo')
+            # Clear pending messages
+            self.db.info['messages'] = []
+            assert up.karma == 2
+            assert len(up.comments) == 3
 
         with mock_sends(*[base_schemas.BodhiMessage] * 6):
             task = self._make_task()
@@ -2047,6 +2045,12 @@ def test_push_timestamps(self, *args):
             assert up.date_testing is not None
             assert up.date_stable is None
             up.request = UpdateRequest.stable
+            # Have the update reach the stable karma threshold
+            assert up.karma == 1
+            up.comment(session, "foo", 1, 'foo')
+            # Clear pending messages
+            self.db.info['messages'] = []
+            assert up.karma == 2
 
         self.koji.clear()
         expected_messages = (
@@ -2134,6 +2138,12 @@ def test_expire_buildroot_overrides_exception(self, expire, exception_log, *args
             up = db.query(Update).one()
             up.release.state = ReleaseState.pending
             up.request = UpdateRequest.stable
+            # Have the update reach the stable karma threshold
+            assert up.karma == 1
+            up.comment(db, "foo", 1, 'foo')
+            # Clear pending messages
+            self.db.info['messages'] = []
+            assert up.karma == 2
         task = self._make_task()
         api_version = task.pop("api_version")
 
diff --git a/bodhi-server/tests/test_models.py b/bodhi-server/tests/test_models.py
index 77b2c7a941..7992b0a8de 100644
--- a/bodhi-server/tests/test_models.py
+++ b/bodhi-server/tests/test_models.py
@@ -313,7 +313,7 @@ def test_epel_without_testing_bug_epel_msg(self, warning):
         config.update({
             'testing_bug_msg': 'cool fedora stuff {update_url}',
             'base_address': 'b',
-            'critpath.min_karma': 1,
+            'min_karma': 1,
             'fedora_epel.mandatory_days_in_testing': 0
         })
         del config["testing_bug_epel_msg"]
@@ -769,8 +769,8 @@ def test_critpath_mandatory_days_in_testing_no_status(self):
         release has no status.
         """
         config.update({
-            'critpath.stable_after_days_without_negative_karma': 11,
-            'f11.current.critpath.stable_after_days_without_negative_karma': 42
+            'critpath.mandatory_days_in_testing': 11,
+            'f11.current.critpath.mandatory_days_in_testing': 42
         })
         assert self.obj.critpath_mandatory_days_in_testing == 11
 
@@ -780,7 +780,7 @@ def test_critpath_mandatory_days_in_testing_status_default(self):
         release has status, but no override set.
         """
         config.update({
-            'critpath.stable_after_days_without_negative_karma': 11,
+            'critpath.mandatory_days_in_testing': 11,
             'f11.status': 'current'
         })
         assert self.obj.critpath_mandatory_days_in_testing == 11
@@ -791,9 +791,9 @@ def test_critpath_mandatory_days_in_testing_status_override(self):
         release has status and override set.
         """
         config.update({
-            'critpath.stable_after_days_without_negative_karma': 11,
+            'critpath.mandatory_days_in_testing': 11,
             'f11.status': 'current',
-            'f11.current.critpath.stable_after_days_without_negative_karma': 42
+            'f11.current.critpath.mandatory_days_in_testing': 42
         })
         assert self.obj.critpath_mandatory_days_in_testing == 42
 
@@ -879,31 +879,31 @@ def test_inherit_override_tags_wrong_release_name(self):
 
 
 class TestReleaseCritpathMinKarma(BasePyTestCase):
-    """Tests for the Release.critpath_min_karma property."""
+    """Tests for the Release.min_karma property."""
 
     @mock.patch.dict(
-        config, {'critpath.min_karma': 2, 'f17.beta.critpath.min_karma': 42, 'f17.status': "beta"})
+        config, {'min_karma': 2, 'f17.beta.min_karma': 42, 'f17.status': "beta"})
     def test_setting_status_min(self):
         """If a min is defined for the release, it should be returned."""
         release = model.Release.query.first()
 
-        assert release.critpath_min_karma == 42
+        assert release.min_karma == 42
 
     @mock.patch.dict(
-        config, {'critpath.min_karma': 25, 'f17.status': "beta"})
+        config, {'min_karma': 25, 'f17.status': "beta"})
     def test_setting_status_no_min(self):
         """If no min is defined for the release, the general min karma config should be returned."""
         release = model.Release.query.first()
 
-        assert release.critpath_min_karma == 25
+        assert release.min_karma == 25
 
     @mock.patch.dict(
-        config, {'critpath.min_karma': 72})
+        config, {'min_karma': 72})
     def test_setting_status_no_setting_status(self):
         """If no status is defined for the release, the general min karma should be returned."""
         release = model.Release.query.first()
 
-        assert release.critpath_min_karma == 72
+        assert release.min_karma == 72
 
 
 class TestReleaseModular(ModelTest):
@@ -2654,7 +2654,7 @@ def test_backref_different_build_types(self):
 @mock.patch('bodhi.server.models.fetch_test_cases_task', mock.Mock())
 class TestUpdateMeetsTestingRequirements(BasePyTestCase):
     """
-    Test the Update.meets_testing_requirements() method.
+    Test the Update.meets_testing_requirements() method and friends.
 
     It is significant and not obvious that the update used by these tests
     starts out with karma of +1/-0. These tests also rely on manual and automatic
@@ -2664,11 +2664,14 @@ class TestUpdateMeetsTestingRequirements(BasePyTestCase):
 
     @pytest.mark.parametrize('critpath', (True, False))
     @pytest.mark.parametrize("autokarma", (True, False))
-    def test_update_reaching_stable_karma_not_critpath_min_karma(self, autokarma, critpath):
+    def test_update_reaching_stable_karma_not_min_karma(self, autokarma, critpath):
         """
-        Assert that meets_testing_requirements() returns the correct result for updates
-        that haven't reached the days in testing or critpath_min_karma, but have reached
-        stable_karma.
+        Assert that meets_testing_requirements() returns False for updates that
+        haven't reached the days in testing or min_karma, but have reached stable_karma.
+        It should be very uncommon to encounter this situation these days because we disallow
+        setting the stable_karma threshold lower than min_karma when creating or editing an
+        update, but it could theoretically happen if min_karma for a release was increased
+        while an update which had stable_karma set to the previous minimum value was in-flight.
         """
         update = model.Update.query.first()
         update.autokarma = autokarma
@@ -2677,18 +2680,17 @@ def test_update_reaching_stable_karma_not_critpath_min_karma(self, autokarma, cr
         update.stable_karma = 1
 
         # check, and make it clear, what we're assuming the karma value is now
-        # and the critpath_min_karma value we're testing against
+        # and the min_karma value we're testing against
         assert update.karma == 1
-        assert update.release.critpath_min_karma == 2
-        # this means 'False for critpath, True for non-critpath'. critpath
-        # should be required to meet critpath_min_karma, but non-critpath
-        # should not
-        assert update.meets_testing_requirements is not critpath
+        assert update.release.min_karma == 2
+        # this should always be False. in the past we allowed non-critpath
+        # updates to be pushed stable in this case but we no longer do
+        assert not update.meets_testing_requirements
 
     @pytest.mark.parametrize('critpath', (True, False))
     @pytest.mark.parametrize('autokarma', (True, False))
     def test_update_below_stable_karma(self, autokarma, critpath):
-        """It should return False for all updates below stable karma, critpath_min_karma
+        """It should return False for all updates below stable karma, min_karma
         and time."""
         update = model.Update.query.first()
         update.autokarma = autokarma
@@ -2713,7 +2715,7 @@ def test_update_reaching_time_in_testing(self, critpath):
         update.critpath = critpath
         update.request = None
         if critpath:
-            # the default critpath.stable_after_days_without_negative_karma
+            # the default critpath.mandatory_days_in_testing
             # value is 14
             update.date_testing = datetime.utcnow() - timedelta(days=15)
         else:
@@ -2732,7 +2734,7 @@ def test_update_below_time_in_testing(self, critpath):
         update.critpath = critpath
         update.request = None
         if critpath:
-            # the default critpath.stable_after_days_without_negative_karma
+            # the default critpath.mandatory_days_in_testing
             # value is 14
             update.date_testing = datetime.utcnow() - timedelta(days=13)
         else:
@@ -2745,15 +2747,16 @@ def test_update_below_time_in_testing(self, critpath):
 
     @pytest.mark.parametrize('critpath', (True, False))
     def test_update_reaching_time_in_testing_negative_karma(self, critpath):
-        """If an update meets the mandatory_days_in_testing and stable_karma thresholds
-        but not the critpath_min_karma threshold, it should return False for critical path
-        updates but True for non-critical-path updates."""
+        """If an update meets mandatory_days_in_testing but not min_karma, it
+        should still return True. In the past we did not allow critical path updates
+        through for days_in_testing if they had any negative karma, but this is not
+        part of the Fedora or EPEL update policies."""
         update = model.Update.query.first()
         update.critpath = critpath
         update.status = model.UpdateStatus.testing
         update.request = None
         if critpath:
-            # the default critpath.stable_after_days_without_negative_karma
+            # the default critpath.mandatory_days_in_testing
             # value is 14
             update.date_testing = datetime.utcnow() - timedelta(days=15)
         else:
@@ -2762,33 +2765,30 @@ def test_update_reaching_time_in_testing_negative_karma(self, critpath):
         update.stable_karma = 1
         update.comment(self.db, 'testing', author='enemy', karma=-1)
         # This gets the update back to positive karma, but not to the required
-        # +2 karma needed to clear the critpath_min_karma requirement
+        # +2 karma needed to clear the min_karma requirement
         update.comment(self.db, 'testing', author='bro', karma=1)
 
         assert update.karma == 1
-        assert update.release.critpath_min_karma == 2
-        # this means 'False for critpath, True for non-critpath'
-        assert update.meets_testing_requirements is not critpath
+        assert update.release.min_karma == 2
+        assert update.meets_testing_requirements is True
 
     @pytest.mark.parametrize('critpath', (True, False))
-    def test_update_reaching_critpath_min_karma(self, critpath):
+    def test_update_reaching_min_karma(self, critpath):
         """Both critpath and non-critpath packages should be allowed to go stable when
         meeting the policy minimum karma requirement for critpath packages, even if there
         is negative karma."""
         update = model.Update.query.first()
         update.critpath = critpath
+        # make sure no autopush gets in the way
         update.stable_karma = 3
         update.comment(self.db, 'testing', author='enemy', karma=-1)
         update.comment(self.db, 'testing', author='bro', karma=1)
-        # Despite meeting the stable_karma, the function should still not
-        # mark this as meeting testing requirements because critpath packages
-        # have a higher requirement for minimum karma - at this point we
-        # are in the same state as test_critpath_14_days_negative_karma.
-        # So add a third +1 so total is +2, which meets critpath_min_karma
+        # at this point we're at +1 as the update starts at +1. Add
+        # another +1 so total is +2, which meets min_karma
         update.comment(self.db, 'testing', author='ham', karma=1)
 
         assert update.karma == 2
-        assert update.release.critpath_min_karma == 2
+        assert update.release.min_karma == 2
         assert update.meets_testing_requirements
 
     @pytest.mark.parametrize('critpath', (True, False))
@@ -2800,7 +2800,7 @@ def test_update_reaching_critpath_min_karma(self, critpath):
             (TestGatingStatus.running, False),
             (TestGatingStatus.passed, True),
             (TestGatingStatus.failed, False),
-            (None, True),
+            (None, False),
             (TestGatingStatus.greenwave_failed, False)
         )
     )
@@ -2819,7 +2819,7 @@ def test_test_gating_status(self, status, expected, critpath):
         update.comment(self.db, 'testing', author='bro', karma=1)
         # Assert that our preconditions from the docblock are correct.
         assert update.karma == 2
-        assert update.release.critpath_min_karma == 2
+        assert update.release.min_karma == 2
         assert update.meets_testing_requirements is expected
 
     def test_test_gating_not_required(self):
@@ -2839,6 +2839,33 @@ def test_test_gating_not_required(self):
         assert update.karma == 2
         assert update.meets_testing_requirements
 
+    def test_other_interfaces(self):
+        """
+        Check the other related interfaces: meets_requirements_why
+        (around which meets_testing_requirements is now a wrapper),
+        critpath_approved (which is now a deprecated synonym for
+        meets_testing_requirements), and check_requirements (which is
+        now a deprecated synonym for meets_requirements_why).
+        """
+        config["test_gating.required"] = False
+        update = model.Update.query.first()
+        update.autokarma = False
+        update.stable_karma = 1
+        update.test_gating_status = TestGatingStatus.running
+        update.comment(self.db, 'I found $100 after applying this update.', karma=1,
+                       author='bowlofeggs')
+        # Assert that our preconditions from the docblock are correct.
+        assert update.karma == 2
+        expwhy = (
+            True,
+            "Test gating is disabled, and the update has at least 2 karma."
+        )
+        assert update.meets_requirements_why == expwhy
+        with pytest.deprecated_call():
+            assert update.check_requirements(self.db, config) == expwhy
+        with pytest.deprecated_call():
+            assert update.critpath_approved
+
 
 @mock.patch('bodhi.server.models.work_on_bugs_task', mock.Mock())
 @mock.patch('bodhi.server.models.fetch_test_cases_task', mock.Mock())
@@ -3087,7 +3114,7 @@ def test_mandatory_days_in_testing_critpath(self):
         update.critpath = True
 
         # Configured value.
-        expected = int(config.get('critpath.stable_after_days_without_negative_karma'))
+        expected = int(config.get('critpath.mandatory_days_in_testing'))
 
         assert update.mandatory_days_in_testing == expected
 
@@ -3121,7 +3148,7 @@ def test_days_to_stable_critpath(self):
         update.date_testing = datetime.utcnow() + timedelta(days=-4)
 
         critpath_days_to_stable = int(
-            config.get('critpath.stable_after_days_without_negative_karma'))
+            config.get('critpath.mandatory_days_in_testing'))
 
         assert update.days_to_stable == critpath_days_to_stable - 4
 
@@ -3132,9 +3159,10 @@ def test_days_to_stable_meets_testing_requirements(self):
         """
         update = self.obj
         update.autokarma = False
-        update.stable_karma = 1
         update.comment(self.db, 'I found $100 after applying this update.', karma=1,
                        author='bowlofeggs')
+        update.comment(self.db, 'This update saved my life!', karma=1,
+                       author='ralph')
         # Assert that our preconditions from the docblock are correct.
         assert update.meets_testing_requirements
 
@@ -3598,32 +3626,6 @@ def test__composite_karma_one_negative_two_positive(self):
 
         assert self.obj._composite_karma == (2, -1)
 
-    def test_critpath_approved_no_release_requirements(self):
-        """critpath_approved() should use the broad requirements if the release doesn't have any."""
-        self.obj.critpath = True
-        self.obj.comment(self.db, "foo", 1, 'biz')
-        release_name = self.obj.release.name.lower().replace('-', '')
-
-        with mock.patch.dict(
-                config,
-                {'{}.status'.format(release_name): 'stable', 'critpath.num_admin_approvals': 0,
-                 'critpath.min_karma': 1}):
-            assert self.obj.critpath_approved
-
-    def test_critpath_approved_release_requirements(self):
-        """critpath_approved() should use the release requirements if they are defined."""
-        self.obj.critpath = True
-        self.obj.comment(self.db, "foo", 1, 'biz')
-        release_name = self.obj.release.name.lower().replace('-', '')
-
-        with mock.patch.dict(
-                config,
-                {'{}.status'.format(release_name): 'stable', 'critpath.num_admin_approvals': 0,
-                 'critpath.min_karma': 1,
-                 '{}.{}.critpath.num_admin_approvals'.format(release_name, 'stable'): 0,
-                 '{}.{}.critpath.min_karma'.format(release_name, 'stable'): 2}):
-            assert not self.obj.critpath_approved
-
     def test_last_modified_no_dates(self):
         """last_modified() should raise ValueError if there are no available dates."""
         self.obj.date_submitted = None
@@ -3771,14 +3773,15 @@ def test_set_request_pending_stable(self):
         req.errors = cornice.Errors()
         req.koji = buildsys.get_session()
         assert self.obj.status == UpdateStatus.pending
-        self.obj.stable_karma = 1
         # disable autokarma, so sending the comment doesn't do the request
         self.obj.autokarma = False
         self.obj.comment(self.db, 'works', karma=1, author='bowlofeggs')
+        self.obj.comment(self.db, 'sure does', karma=1, author='ralph')
         # make sure we are actually doing something here
         assert self.obj.request is not UpdateRequest.stable
 
         expected_messages = (
+            update_schemas.UpdateCommentV1,
             update_schemas.UpdateCommentV1,
             update_schemas.UpdateRequestStableV1,
         )
@@ -3801,8 +3804,13 @@ def test_set_request_pending_stable_frozen_release(self):
         req.errors = cornice.Errors()
         req.koji = buildsys.get_session()
         assert self.obj.status == UpdateStatus.pending
-        self.obj.stable_karma = 1
         self.obj.release.state = ReleaseState.frozen
+        # disable autokarma, so sending the comment doesn't do the request
+        self.obj.autokarma = False
+        # make sure we meet the karma requirements, so we *could* push
+        # stable if we weren't frozen
+        self.obj.comment(self.db, 'works', karma=1, author='bowlofeggs')
+        self.obj.comment(self.db, 'sure does', karma=1, author='ralph')
 
         with pytest.raises(BodhiException) as exc:
             self.obj.set_request(self.db, UpdateRequest.stable, req.user.name)
@@ -3903,7 +3911,10 @@ def test_set_request_untested_stable(self):
         assert self.obj.status == UpdateStatus.pending
         with pytest.raises(BodhiException) as exc:
             self.obj.set_request(self.db, UpdateRequest.stable, req.user.name)
-        assert str(exc.value) == config.get('not_yet_tested_msg')
+        assert str(exc.value) == (
+            f"{config.get('not_yet_tested_msg')}: Test gating is disabled, but "
+            "update has less than 2 karma and has been in testing less than 7 days."
+        )
         assert self.obj.request == UpdateRequest.testing
         assert self.obj.status == UpdateStatus.pending
 
@@ -3991,7 +4002,10 @@ def test_set_request_stable_epel_requirements_not_met(self):
         with pytest.raises(BodhiException) as exc:
             with mock_sends():
                 self.obj.set_request(self.db, UpdateRequest.stable, req.user.name)
-        assert str(exc.value) == config['not_yet_tested_epel_msg']
+        assert str(exc.value) == (
+            f"{config['not_yet_tested_epel_msg']}: Test gating is disabled, "
+            "but update has less than 2 karma and has been in testing less than 14 days."
+        )
 
         assert self.obj.request is None
 
@@ -4037,17 +4051,10 @@ def test_set_request_stable_for_critpath_update_when_test_gating_enabled(self):
             self.obj.set_request(self.db, UpdateRequest.stable, req.user.name)
 
         expected_msg = (
-            'This critical path update has not yet been approved for pushing to the '
-            'stable repository.  It must first reach a karma of %s, consisting of %s '
-            'positive karma from proventesters, along with %d additional karma from '
-            'the community. Or, it must spend %s days in testing without any negative '
-            'feedback')
-        expected_msg = expected_msg % (
-            config.get('critpath.min_karma'),
-            config.get('critpath.num_admin_approvals'),
-            (config.get('critpath.min_karma') - config.get('critpath.num_admin_approvals')),
-            config.get('critpath.stable_after_days_without_negative_karma'))
-        expected_msg += ' Additionally, it must pass automated tests.'
+            'This update has not yet met the minimum testing requirements defined in the '
+            '<a href="https://fedoraproject.org/wiki/Package_update_acceptance_criteria">'
+            'Package Update Acceptance Criteria</a>: Required tests did not pass on this update.'
+        )
         assert str(exc.value) == expected_msg
 
     def test_set_request_string_action(self):
@@ -4058,12 +4065,13 @@ def test_set_request_string_action(self):
         assert self.obj.status == UpdateStatus.pending
         # disable autokarma, so sending the comment doesn't do the request
         self.obj.autokarma = False
-        self.obj.stable_karma = 1
         self.obj.comment(self.db, 'works', karma=1, author='bowlofeggs')
+        self.obj.comment(self.db, 'sure does', karma=1, author='ralph')
         # make sure we are actually doing something here
         assert self.obj.request is not UpdateRequest.stable
 
         expected_messages = (
+            update_schemas.UpdateCommentV1,
             update_schemas.UpdateCommentV1,
             update_schemas.UpdateRequestStableV1,
         )
@@ -4341,65 +4349,6 @@ def test_send_update_notice_status_testing(self, SMTP):
             [config['{}_test_announce_list'.format(release_name)]],
             msg.encode('utf-8'))
 
-    def test_check_requirements_test_gating_disabled(self):
-        '''Should pass regardless if gating is disabled.'''
-        self.obj.test_gating_status = model.TestGatingStatus.failed
-        with mock.patch.dict(config, {'test_gating.required': False}):
-            assert self.obj.check_requirements(self.db, config) == (True, 'No checks required.')
-
-    def test_check_requirements_test_gating_status_failed(self):
-        """check_requirements() should return False when test_gating_status is failed."""
-        self.obj.test_gating_status = model.TestGatingStatus.failed
-
-        with mock.patch.dict(config, {'test_gating.required': True}):
-            assert self.obj.check_requirements(self.db, config) == (
-                False, 'Required tests did not pass on this update.')
-
-    def test_check_requirements_test_gating_status_passed(self):
-        """check_requirements() should return True when test_gating_status is passed."""
-        self.obj.test_gating_status = model.TestGatingStatus.passed
-
-        with mock.patch.dict(config, {'test_gating.required': True}):
-            assert self.obj.check_requirements(self.db, config) == (
-                True, 'All required tests passed or were waived.')
-
-    def test_check_requirements_test_gating_status_ignored(self):
-        """check_requirements() should return True when test_gating_status is ignored."""
-        self.obj.test_gating_status = model.TestGatingStatus.ignored
-
-        with mock.patch.dict(config, {'test_gating.required': True}):
-            assert self.obj.check_requirements(self.db, config) == (
-                True, 'No checks required.')
-
-    def test_check_requirements_test_gating_status_waiting(self):
-        """check_requirements() should return False when test_gating_status is waiting."""
-        self.obj.test_gating_status = model.TestGatingStatus.waiting
-
-        with mock.patch.dict(config, {'test_gating.required': True}):
-            assert self.obj.check_requirements(self.db, config) == (
-                False, 'Required tests for this update are still running.')
-
-    def test_num_admin_approvals_after_karma_reset(self):
-        """Make sure number of admin approvals is counted correctly for the build."""
-        update = model.Update.query.first()
-
-        # Approval from admin 'bodhiadmin' {config.admin_groups}
-        user_group = [model.Group(name='bodhiadmin')]
-        user = model.User(name='bodhiadmin', groups=user_group)
-        comment = model.Comment(text='Test comment', karma=1, user=user)
-        self.db.add(comment)
-        update.comments.append(comment)
-
-        assert update.num_admin_approvals == 1
-
-        # This is a "karma reset event", so the above comments should not be counted in the karma.
-        user = model.User(name='bodhi')
-        comment = model.Comment(text="New build", karma=0, user=user)
-        self.db.add(comment)
-        update.comments.append(comment)
-
-        assert update.num_admin_approvals == 0
-
     def test_validate_release_failure(self):
         """Test the validate_release() method for the failure case."""
         user = self.db.query(model.User).first()
diff --git a/bodhi-server/tests/testing.ini b/bodhi-server/tests/testing.ini
index 6c3411dc13..4ed23bc647 100644
--- a/bodhi-server/tests/testing.ini
+++ b/bodhi-server/tests/testing.ini
@@ -14,9 +14,8 @@ fedora.mandatory_days_in_testing = 7
 fedora_epel.mandatory_days_in_testing = 14
 f7.status = post_beta
 f7.post_beta.mandatory_days_in_testing = 7
-f7.post_beta.critpath.num_admin_approvals = 0
-f7.post_beta.critpath.min_karma = 2
-f7.post_beta.critpath.stable_after_days_without_negative_karma = 3
+f7.post_beta.min_karma = 2
+f7.post_beta.critpath.mandatory_days_in_testing = 3
 cors_origins_ro = *
 cors_origins_rw = http://0.0.0.0:6543 http://bodhi-dev.example.com/ https://bodhi-dev.example.com/
 cors_connect_src = http://0.0.0.0:6543 http://bodhi-dev.example.com/ https://bodhi-dev.example.com/ http://localhost:6543 https://*.fedoraproject.org/ wss://hub.fedoraproject.org:9939/
@@ -65,7 +64,6 @@ pungi.cmd = /usr/bin/true
 pungi.conf.rpm = pungi.rpm.conf.j2
 pungi.conf.module = pungi.module.conf.j2
 critpath_pkgs = kernel
-critpath.num_admin_approvals = 0
 bugtracker = dummy
 stats_blacklist = bodhi autoqa
 test_case_base_url = https://fedoraproject.org/wiki/
diff --git a/devel/ci/integration/bodhi/production.ini b/devel/ci/integration/bodhi/production.ini
index 774a4e4111..ad1bc767ed 100644
--- a/devel/ci/integration/bodhi/production.ini
+++ b/devel/ci/integration/bodhi/production.ini
@@ -399,10 +399,12 @@ critpath_pkgs = kernel
 critpath.num_admin_approvals = 0
 
 # The net karma required to submit a critical path update to a pending release.
-# critpath.min_karma = 2
+# we set this to 0 otherwise we can't be sure test_updates_request will
+# pass, it might pick an update with karma lower than this value
+min_karma = 0
 
 # Allow critpath to submit for stable after 2 weeks with no negative karma
-# critpath.stable_after_days_without_negative_karma = 14
+# critpath.mandatory_days_in_testing = 14
 
 # The minimum amount of time an update must spend in testing before
 # it can reach the stable repository
@@ -423,10 +425,10 @@ fedora_epel.mandatory_days_in_testing = 14
 #f28.status = pre_beta
 #f28.pre_beta.mandatory_days_in_testing = 3
 #f28.pre_beta.critpath.num_admin_approvals = 0
-#f28.pre_beta.critpath.min_karma = 1
+#f28.pre_beta.min_karma = 1
 #f28.post_beta.mandatory_days_in_testing = 7
 #f28.post_beta.critpath.num_admin_approvals = 0
-#f28.post_beta.critpath.min_karma = 2
+#f28.post_beta.min_karma = 2
 
 
 ##
diff --git a/devel/development.ini.example b/devel/development.ini.example
index 4d97cdede6..28a0d95cca 100644
--- a/devel/development.ini.example
+++ b/devel/development.ini.example
@@ -13,8 +13,8 @@ fedora_epel.mandatory_days_in_testing = 14
 f7.status = post_beta
 f7.post_beta.mandatory_days_in_testing = 7
 f7.post_beta.critpath.num_admin_approvals = 0
-f7.post_beta.critpath.min_karma = 2
-f7.post_beta.critpath.stable_after_days_without_negative_karma = 3
+f7.post_beta.min_karma = 2
+f7.post_beta.critpath.mandatory_days_in_testing = 3
 cors_origins_ro = *
 cors_origins_rw = http://0.0.0.0:6543 http://bodhi-dev.example.com/ https://bodhi-dev.example.com/
 cors_connect_src = http://0.0.0.0:6543 http://bodhi-dev.example.com/ https://bodhi-dev.example.com/ http://localhost:6543 https://*.fedoraproject.org/ wss://hub.fedoraproject.org:9939/
diff --git a/docs/user/testing.rst b/docs/user/testing.rst
index 74751750ba..ad264682a4 100644
--- a/docs/user/testing.rst
+++ b/docs/user/testing.rst
@@ -38,24 +38,28 @@ Stable push
 An update can be pushed to stable repository either manually or automatically, through Bodhi's
 `autotime` or `autokarma` features.
 
-For becoming pushable to stable an update must fullfill some criteria set by the associated
-release. Tipically, each release has a `mandatory_days_in_testing` and a `critpath_min_karma`
-values that define the threshold after that the associated updates can be pushed to stable.
-The `mandatory_days_in_testing` define the minimum amount of days each update must spend in
-testing repository. The `critpath_min_karma` is the minimum karma that each update must reach.
-An update can be manually pushed to stable by its submitter whatever threshold is reached first.
-For example, if a release has a `critpath_min_karma` of +2, an update which reaches a +2 karma
-**can be pushed to stable even if it hasn't reached the `mandatory_days_in_testing`**.
+For becoming pushable to stable an update must fulfill some criteria set by the associated
+release. Typically, each release has `critpath.mandatory_days_in_testing`,
+`mandatory_days_in_testing` and `min_karma` values that define thresholds for associated updates
+to be pushed to stable. The `mandatory_days_in_testing` settings define an amount of days of
+waiting in the updates repository for critical path and non-critical-path updates respectively.
+The `min_karma` setting specifies a karma total threshold. An update can be manually pushed to
+stable by its submitter or anyone else with edit privileges on the update when **either**
+threshold is reached. For example, if a release has a `min_karma` of 2, an update which reaches
++2 karma **can be pushed to stable even if it hasn't reached the `mandatory_days_in_testing`**.
 
 When submitting an update, the user can enable the `autotime` and the `autokarma` features and
 set the related values `stable_days` and `stable_karma` for that specific update.
-The `stable_days` value define the minimum amount of days each update must spend in
-testing repository before the autopush is performed. It must be equal or greater than the release
-`mandatory_days_in_testing`. The `stable_karma` is the minimum karma that each update must reach
-before the autopush is performed. It must be equal or greater than the release `critpath_min_karma`.
-For example, if a release has a `mandatory_days_in_testing` of 7, an update which has `autotime`
-enabled and `stable_days` set to 10 will be pushable after 7 days **by manual action**, but the
-autopush will be performed **after 3 more days**.
-
+The `stable_days` value defines an amount of days the update must spend in testing repository
+before the autopush is done. It must be equal to or greater than the `mandatory_days_in_testing`
+value that applies to the update. The `stable_karma` value defines a karma total the update must
+reach before the autopush is done. It must be equal to or greater than the release `min_karma`.
+Autopush will happen as soon as **either** threshold is reached. For example, if a release has a
+`mandatory_days_in_testing` of 7, an update which has `autotime` enabled and `stable_days` set to
+10 will be pushable after 7 days **by manual action**, but the autopush will be performed
+**after 3 more days**. If an update has both `autotime` and `autokarma` enabled, with thresholds
+of 7 days and 3 karma respectively, it will be pushed stable as soon as it reaches +3 karma, even
+if 7 days have not yet passed; or it will be pushed stable after 7 days have passed, even if it
+has not yet reached +3 karma.
 
 .. _Greenwave: https://pagure.io/greenwave