diff --git a/bodhi/client/__init__.py b/bodhi/client/__init__.py index e1b61f6d01..3dae656e90 100644 --- a/bodhi/client/__init__.py +++ b/bodhi/client/__init__.py @@ -117,6 +117,9 @@ def _set_logging_debug(ctx, param, value): click.option('--close-bugs', is_flag=True, help='Automatically close bugs'), click.option('--request', help='Requested repository', type=click.Choice(['testing', 'stable', 'unpush'])), + click.option('--autotime', is_flag=True, help='Enable stable push base on time in testing'), + click.option('--stable-days', type=click.INT, + help='Days in testing required to push to stable'), click.option('--autokarma', is_flag=True, help='Enable karma automatism'), click.option('--stable-karma', type=click.INT, help='Stable karma threshold'), click.option('--unstable-karma', type=click.INT, help='Unstable karma threshold'), diff --git a/bodhi/client/bindings.py b/bodhi/client/bindings.py index f0560a786e..51b8839a05 100644 --- a/bodhi/client/bindings.py +++ b/bodhi/client/bindings.py @@ -245,6 +245,11 @@ def save(self, **kwargs) -> 'munch.Munch': close_bugs (bool): Close bugs when update is stable. suggest (str): Suggest that the user reboot or logout after update. (``reboot``, ``logout``). + autotime (bool): Allow bodhi to automatically change the state of this + update based on the time spent in testing by this update. It + will push your update to ``stable`` once it reaches the ``stable_days``. + stable_days (int): The minimun amount of time an update has to spend in + ``testing`` before being automatically pushed to ``stable``. autokarma (bool): Allow bodhi to automatically change the state of this update based on the ``karma`` from user feedback. It will push your update to ``stable`` once it reaches the ``stable_karma`` diff --git a/bodhi/server/config.py b/bodhi/server/config.py index eeca5a521e..9ce3d17420 100644 --- a/bodhi/server/config.py +++ b/bodhi/server/config.py @@ -514,14 +514,8 @@ class BodhiConfig(dict): 'test_case_base_url': { 'value': 'https://fedoraproject.org/wiki/', 'validator': str}, - 'testing_approval_msg_based_on_karma': { - 'value': ('This update has reached the stable karma threshold and can be pushed to ' - 'stable now if the maintainer wishes.'), - 'validator': str - }, 'testing_approval_msg': { - 'value': ('This update has reached %d days in testing and can be pushed to stable now ' - 'if the maintainer wishes'), + 'value': ('This update can be pushed to stable now if the maintainer wishes'), 'validator': str}, 'testing_bug_epel_msg': { 'value': ( diff --git a/bodhi/server/migrations/versions/b1fd856efcf6_add_autopush_boolean_and_stable_days_to_update.py b/bodhi/server/migrations/versions/b1fd856efcf6_add_autopush_boolean_and_stable_days_to_update.py new file mode 100644 index 0000000000..ea91fbdaa2 --- /dev/null +++ b/bodhi/server/migrations/versions/b1fd856efcf6_add_autopush_boolean_and_stable_days_to_update.py @@ -0,0 +1,50 @@ +# Copyright (c) 2019 Red Hat, Inc. +# +# This file is part of Bodhi. +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +""" +Add autotime boolean and stable_days to Update. + +Revision ID: b1fd856efcf6 +Revises: 5703ddfe855d +Create Date: 2019-03-22 09:51:53.941289 +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'b1fd856efcf6' +down_revision = '5703ddfe855d' + + +def upgrade(): + """Add the autotime boolean and stable_days integer to the updates table.""" + # autotime + op.add_column('updates', sa.Column('autotime', sa.Boolean())) + op.execute('UPDATE updates SET autotime=FALSE') + op.alter_column('updates', 'autotime', existing_type=sa.Boolean(), nullable=False) + + # stable_days + op.add_column('updates', sa.Column('stable_days', sa.Integer())) + op.execute('UPDATE updates SET stable_days=0') + op.alter_column('updates', 'stable_days', existing_type=sa.Integer(), nullable=False) + + +def downgrade(): + """Drop the autotime boolean and the stable_days integer from the updates table.""" + op.drop_column('updates', 'autotime') + op.drop_column('updates', 'stable_days') diff --git a/bodhi/server/models.py b/bodhi/server/models.py index fdf64285e0..3cef7ef04f 100644 --- a/bodhi/server/models.py +++ b/bodhi/server/models.py @@ -1555,8 +1555,12 @@ class Update(Base): Attributes: autokarma (bool): A boolean that indicates whether or not the update will be automatically pushed when the stable_karma threshold is reached. + autotime (bool): A boolean that indicates whether or not the update will + be automatically pushed when the time threshold is reached. stable_karma (int): A positive integer that indicates the amount of "good" karma the update must receive before being automatically marked as stable. + stable_days (int): A positive integer that indicates the number of days an update + needs to spend in testing before being automatically marked as stable. unstable_karma (int): A positive integer that indicates the amount of "bad" karma the update must receive before being automatically marked as unstable. requirements (str): A list of taskotron tests that must pass for this @@ -1625,7 +1629,9 @@ class Update(Base): __get_by__ = ('alias',) autokarma = Column(Boolean, default=True, nullable=False) + autotime = Column(Boolean, default=True, nullable=False) stable_karma = Column(Integer, nullable=False) + stable_days = Column(Integer, nullable=False, default=0) unstable_karma = Column(Integer, nullable=False) requirements = Column(UnicodeText) require_bugs = Column(Boolean, default=False) @@ -2034,6 +2040,16 @@ def new(cls, request, data): release = data.pop('release', None) up = Update(**data, release=release) + # We want to make sure that the value of stable_days + # will not be lower than the mandatory_days_in_testing. + if up.mandatory_days_in_testing > up.stable_days: + up.stable_days = up.mandatory_days_in_testing + caveats.append({ + 'name': 'stable days', + 'description': "The number of stable days required was set to the mandatory " + f"release value of {up.mandatory_days_in_testing} days" + }) + log.debug("Setting request for new update.") up.set_request(db, req, request.user.name) @@ -2073,6 +2089,16 @@ def edit(cls, request, data): caveats = [] edited_builds = [build.nvr for build in up.builds] + # stable_days can be set by the user. We want to make sure that the value + # will not be lower than the mandatory_days_in_testing. + if up.mandatory_days_in_testing > data.get('stable_days', up.stable_days): + data['stable_days'] = up.mandatory_days_in_testing + caveats.append({ + 'name': 'stable days', + 'description': "The number of stable days required was raised to the mandatory " + f"release value of {up.mandatory_days_in_testing} days" + }) + # Determine which builds have been added new_builds = [] for build in data['builds']: @@ -3261,7 +3287,7 @@ def check_karma_thresholds(self, db, agent): notifications.publish(update_schemas.UpdateKarmaThresholdV1.from_dict( dict(update=self, status='stable'))) else: - # Add the 'testing_approval_msg_based_on_karma' message now + # Add the stable approval message now log.info(( "%s update has reached the stable karma threshold and can be pushed to " "stable now if the maintainer wishes"), self.alias) @@ -3391,9 +3417,8 @@ def has_stable_comment(self): """ for comment in self.comments_since_karma_reset: if comment.user.name == 'bodhi' and \ - comment.text.startswith('This update has reached') and \ - 'and can be pushed to stable now if the ' \ - 'maintainer wishes' in comment.text: + comment.text.startswith('This update ') and \ + 'can be pushed to stable now if the maintainer wishes' in comment.text: return True return False diff --git a/bodhi/server/schemas.py b/bodhi/server/schemas.py index 837a00a268..98ae106ea8 100644 --- a/bodhi/server/schemas.py +++ b/bodhi/server/schemas.py @@ -235,6 +235,15 @@ class SaveUpdateSchema(CSRFProtectedSchema, colander.MappingSchema): colander.Boolean(), missing=True, ) + autotime = colander.SchemaNode( + colander.Boolean(), + missing=True, + ) + stable_days = colander.SchemaNode( + colander.Integer(), + validator=colander.Range(min=0), + missing=0, + ) class Cosmetics(colander.MappingSchema): diff --git a/bodhi/server/scripts/approve_testing.py b/bodhi/server/scripts/approve_testing.py index 456d511cca..00d03b7b9f 100644 --- a/bodhi/server/scripts/approve_testing.py +++ b/bodhi/server/scripts/approve_testing.py @@ -30,7 +30,7 @@ from bodhi.messages.schemas import update as update_schemas from bodhi.server import Session, initialize_db, notifications -from ..models import Update, UpdateStatus +from ..models import Update, UpdateStatus, UpdateRequest from ..config import config @@ -77,37 +77,32 @@ def main(argv=sys.argv): testing = db.query(Update).filter_by(status=UpdateStatus.testing, request=None) for update in testing: - # If this release does not have any testing requirements, skip it - if not update.release.mandatory_days_in_testing: - print('%s doesn\'t have mandatory days in testing' % update.release.name) + if not update.release.mandatory_days_in_testing and not update.autotime: + # If this release does not have any testing requirements and is not autotime, + # skip it + print(f"{update.release.name} doesn't have mandatory days in testing") continue # If this update was already commented, skip it if update.has_stable_comment: continue - # Approval message when testing based on karma threshold - if update.karma >= update.stable_karma \ - and not update.autokarma and update.meets_testing_requirements: - print(f'{update.alias} now reaches stable karma threshold') - text = config.get('testing_approval_msg_based_on_karma') - update.comment(db, text, author='bodhi') - db.commit() - continue - - # If autokarma updates have reached the testing threshold, say something! Keep in mind + # If updates have reached the testing threshold, say something! Keep in mind # that we don't care about karma here, because autokarma updates get their request set # to stable by the Update.comment() workflow when they hit the required threshold. Thus, # this function only needs to consider the time requirements because these updates have # not reached the karma threshold. if update.meets_testing_requirements: print(f'{update.alias} now meets testing requirements') - text = str( - config.get('testing_approval_msg') % update.mandatory_days_in_testing) - update.comment(db, text, author='bodhi') + update.comment(db, str(config.get('testing_approval_msg')), author='bodhi') notifications.publish(update_schemas.UpdateRequirementsMetStableV1.from_dict( dict(update=update))) + + if update.autotime and update.days_in_testing >= update.stable_days: + print(f"Automatically marking {update.alias} as stable") + update.set_request(db=db, action=UpdateRequest.stable, username="bodhi") + db.commit() except Exception as e: diff --git a/bodhi/server/templates/new_update.html b/bodhi/server/templates/new_update.html index 6ea8fe8246..7824acf4be 100644 --- a/bodhi/server/templates/new_update.html +++ b/bodhi/server/templates/new_update.html @@ -234,20 +234,29 @@

Edit checked % endif > +
+ Auto-request stable based on karma ?
+
+
+
+ Auto-request stable based on time ?
+
+
-
-
- Auto-request stable?
-
+
Stable karma
Edit % endif >
+
+ Stable days
+
+
Require bugs
diff --git a/bodhi/server/templates/update.html b/bodhi/server/templates/update.html index 74b97a74e4..0e77fe0aee 100644 --- a/bodhi/server/templates/update.html +++ b/bodhi/server/templates/update.html @@ -927,7 +927,7 @@

Add Comment & Feedback

- Autopush + Autopush (karma)
% if update.autokarma is True: @@ -939,8 +939,31 @@

Add Comment & Feedback

% elif update.autokarma is False: - + + Disabled + + + % endif +
+
+ +
+
+ Autopush (time) +
+
+ % if update.autotime is True: + + + Enabled + + + % elif update.autotime is False: + + Disabled diff --git a/bodhi/tests/client/test___init__.py b/bodhi/tests/client/test___init__.py index ba75a043ce..27ef9c4b51 100644 --- a/bodhi/tests/client/test___init__.py +++ b/bodhi/tests/client/test___init__.py @@ -482,8 +482,9 @@ def test_severity_flag(self, send_request): result = runner.invoke( client.new, - ['--user', 'bowlofeggs', '--password', 's3kr3t', '--autokarma', 'bodhi-2.2.4-1.el7', - '--severity', 'urgent', '--notes', 'No description.']) + ['--user', 'bowlofeggs', '--password', 's3kr3t', '--autokarma', '--autotime', + 'bodhi-2.2.4-1.el7', '--severity', 'urgent', '--notes', 'No description.', + '--stable-days', 7]) self.assertEqual(result.exit_code, 0) expected_output = client_test_data.EXPECTED_UPDATE_OUTPUT.replace('unspecified', 'urgent') @@ -497,7 +498,8 @@ def test_severity_flag(self, send_request): 'staging': False, 'builds': 'bodhi-2.2.4-1.el7', 'autokarma': True, 'suggest': None, 'notes': 'No description.', 'request': None, 'bugs': '', 'requirements': None, 'unstable_karma': None, 'file': None, 'notes_file': None, - 'type': 'bugfix', 'severity': 'urgent', 'display_name': None + 'type': 'bugfix', 'severity': 'urgent', 'display_name': None, 'autotime': True, + 'stable_days': 7 } ), mock.call( @@ -538,7 +540,8 @@ def test_debug_flag(self, send_request): 'suggest': None, 'notes': 'No description.', 'request': None, 'bugs': '', 'requirements': None, 'unstable_karma': None, 'file': None, 'notes_file': None, 'type': 'bugfix', 'severity': 'urgent', - 'display_name': None + 'display_name': None, 'autotime': False, + 'stable_days': None } ), mock.call( @@ -577,7 +580,8 @@ def test_url_flag(self, send_request): 'staging': False, 'builds': 'bodhi-2.2.4-1.el7', 'autokarma': True, 'suggest': None, 'notes': 'No description.', 'request': None, 'bugs': '', 'requirements': None, 'unstable_karma': None, 'file': None, - 'notes_file': None, 'type': 'bugfix', 'severity': None, 'display_name': None + 'notes_file': None, 'type': 'bugfix', 'severity': None, 'display_name': None, + 'autotime': False, 'stable_days': None } ), mock.call( @@ -698,7 +702,8 @@ def test_close_bugs_flag(self, send_request): 'staging': False, 'builds': 'bodhi-2.2.4-1.el7', 'autokarma': True, 'suggest': None, 'notes': 'No description.', 'request': None, 'bugs': '1234567', 'requirements': None, 'unstable_karma': None, 'file': None, - 'notes_file': None, 'type': 'bugfix', 'severity': None, 'display_name': None + 'notes_file': None, 'type': 'bugfix', 'severity': None, 'display_name': None, + 'autotime': False, 'stable_days': None } ), mock.call( @@ -740,7 +745,7 @@ def test_display_name_flag(self, send_request): 'suggest': None, 'notes': 'No description.', 'request': None, 'bugs': '1234567', 'requirements': None, 'unstable_karma': None, 'file': None, 'notes_file': None, 'type': 'bugfix', 'severity': None, - 'display_name': 'fake display name' + 'display_name': 'fake display name', 'autotime': False, 'stable_days': None } ), mock.call( @@ -1628,7 +1633,8 @@ def test_bugs_flag(self, send_request, query): 'suggest': 'unspecified', 'notes': 'New package.', 'notes_file': None, 'request': None, 'unstable_karma': -3, 'bugs': '1234,5678', 'requirements': '', 'type': 'newpackage', - 'severity': 'low', 'display_name': None}), + 'severity': 'low', 'display_name': None, 'autotime': False, + 'stable_days': None}), mock.call( bindings_client, 'updates/FEDORA-EPEL-2016-3081a94111/get-test-results', @@ -1665,7 +1671,7 @@ def test_severity_flag(self, send_request, query): 'suggest': 'unspecified', 'notes': 'Updated package.', 'notes_file': None, 'request': None, 'unstable_karma': -3, 'bugs': '1420605', 'requirements': '', 'type': 'newpackage', - 'severity': 'low', 'display_name': None + 'severity': 'low', 'display_name': None, 'autotime': False, 'stable_days': None } ), mock.call( @@ -1708,7 +1714,8 @@ def test_url_flag(self, send_request, query): 'suggest': 'unspecified', 'notes': 'this is an edited note', 'notes_file': None, 'request': None, 'severity': 'low', 'bugs': '1420605', 'requirements': '', 'unstable_karma': -3, - 'type': 'newpackage', 'display_name': None + 'type': 'newpackage', 'display_name': None, 'autotime': False, + 'stable_days': None, } ), mock.call( @@ -1756,7 +1763,8 @@ def test_notes_file(self, send_request, query): 'suggest': 'unspecified', 'notes': 'This is a --notes-file note!', 'notes_file': 'notefile.txt', 'request': None, 'severity': 'low', 'bugs': '1420605', 'requirements': '', 'unstable_karma': -3, - 'type': 'newpackage', 'display_name': None + 'type': 'newpackage', 'display_name': None, 'autotime': False, + 'stable_days': None } ), mock.call( @@ -1804,7 +1812,7 @@ def test_addbuilds_removebuilds(self, send_request, query): 'suggest': u'unspecified', 'notes': u'add and remove builds', 'notes_file': None, 'request': None, 'severity': u'low', 'bugs': '1420605', 'requirements': u'', 'unstable_karma': -3, - 'type': 'newpackage' + 'type': 'newpackage', 'autotime': False, 'stable_days': None } ), mock.call( @@ -1891,7 +1899,8 @@ def test_required_tasks(self, send_request, query): 'suggest': 'unspecified', 'notes': 'testing required tasks', 'notes_file': None, 'request': None, 'severity': 'low', 'bugs': '1420605', 'unstable_karma': -3, 'display_name': None, - 'requirements': 'dist.depcheck dist.rpmdeplint', 'type': 'newpackage' + 'requirements': 'dist.depcheck dist.rpmdeplint', 'type': 'newpackage', + 'autotime': False, 'stable_days': None } ), mock.call( @@ -1950,7 +1959,8 @@ def test_edit_bugless_update_without_bugs_param(self, send_request, query): 'autokarma': False, 'edited': 'FEDORA-2017-c95b33872d', 'suggest': 'unspecified', 'notes': 'New package.', 'display_name': None, 'notes_file': None, 'request': None, 'severity': 'low', - 'bugs': '', 'requirements': '', 'unstable_karma': -3, 'type': 'newpackage' + 'bugs': '', 'requirements': '', 'unstable_karma': -3, 'type': 'newpackage', + 'autotime': False, 'stable_days': None } ), mock.call( diff --git a/bodhi/tests/server/scripts/test_approve_testing.py b/bodhi/tests/server/scripts/test_approve_testing.py index de467051a0..7a03b7c1c9 100644 --- a/bodhi/tests/server/scripts/test_approve_testing.py +++ b/bodhi/tests/server/scripts/test_approve_testing.py @@ -46,6 +46,7 @@ def test_autokarma_update_meeting_time_requirements_gets_one_comment(self): """ update = self.db.query(models.Update).all()[0] update.autokarma = True + update.autotime = False update.request = None update.stable_karma = 10 update.status = models.UpdateStatus.testing @@ -71,9 +72,7 @@ def test_autokarma_update_meeting_time_requirements_gets_one_comment(self): bodhi = self.db.query(models.User).filter_by(name='bodhi').one() comment_q = self.db.query(models.Comment).filter_by(update_id=update.id, user_id=bodhi.id) self.assertEqual(comment_q.count(), 1) - self.assertEqual( - comment_q[0].text, - config.get('testing_approval_msg') % update.release.mandatory_days_in_testing) + self.assertEqual(comment_q[0].text, config.get('testing_approval_msg')) # Set the release's mandatory days in testing to 0 to set up the condition for this test. @patch.dict(config, [('fedora.mandatory_days_in_testing', 0)]) @@ -84,6 +83,7 @@ def test_autokarma_update_without_mandatory_days_in_testing(self): """ update = self.db.query(models.Update).all()[0] update.autokarma = True + update.autotime = False update.request = None update.status = models.UpdateStatus.testing update.date_testing = datetime.utcnow() - timedelta(days=7) @@ -152,8 +152,7 @@ def test_exception_handler(self, stdout, exit, remove, comment): exit.assert_called_once_with(1) comment.assert_called_once_with( self.db, - ('This update has reached 7 days in testing and can be pushed to stable now if the ' - 'maintainer wishes'), + ('This update can be pushed to stable now if the maintainer wishes'), author='bodhi') self.assertEqual(stdout.getvalue(), f'{update.alias} now meets testing requirements\nThe DB died lol\n') @@ -169,11 +168,13 @@ def test_exception_handler_on_the_second_update(self, stdout, exit, remove, comm the Exception handler prints the Exception, rolls back and closes the db, and exits. """ update = self.db.query(models.Update).all()[0] + update.autotime = False update.date_testing = datetime.utcnow() - timedelta(days=15) update.request = None update.status = models.UpdateStatus.testing update2 = self.create_update(['bodhi2-2.0-1.fc17']) + update2.autotime = False update2.date_testing = datetime.utcnow() - timedelta(days=15) update2.request = None update2.status = models.UpdateStatus.testing @@ -191,10 +192,8 @@ def test_exception_handler_on_the_second_update(self, stdout, exit, remove, comm exit.assert_called_once_with(1) comment_expected_call = call( self.db, - ('This update has reached 7 days in testing and can be pushed to stable now if the ' - 'maintainer wishes'), - author='bodhi', - ) + ('This update can be pushed to stable now if the maintainer wishes'), + author='bodhi',) self.assertEqual(comment.call_args_list, [comment_expected_call, comment_expected_call]) self.assertEqual(stdout.getvalue(), (f'{update2.alias} now meets testing requirements\n' @@ -214,6 +213,7 @@ def test_non_autokarma_critpath_update_meeting_karma_requirements_gets_one_comme """ update = self.db.query(models.Update).all()[0] update.autokarma = False + update.autotime = False # Make this update a critpath update to force meets_testing_requirements into a different # code path. update.critpath = True @@ -237,7 +237,7 @@ def test_non_autokarma_critpath_update_meeting_karma_requirements_gets_one_comme bodhi = self.db.query(models.User).filter_by(name='bodhi').one() comment_q = self.db.query(models.Comment).filter_by(update_id=update.id, user_id=bodhi.id) self.assertEqual(comment_q.count(), 1) - self.assertEqual(comment_q[0].text, config.get('testing_approval_msg_based_on_karma')) + self.assertEqual(comment_q[0].text, config.get('testing_approval_msg')) def test_non_autokarma_critpath_update_not_meeting_time_requirements_gets_no_comment(self): """ @@ -289,6 +289,7 @@ def test_non_autokarma_update_meeting_karma_requirements_gets_one_comment(self): """ update = self.db.query(models.Update).all()[0] update.autokarma = False + update.autotime = False update.request = None update.stable_karma = 1 update.status = models.UpdateStatus.testing @@ -307,7 +308,7 @@ def test_non_autokarma_update_meeting_karma_requirements_gets_one_comment(self): bodhi = self.db.query(models.User).filter_by(name='bodhi').one() comment_q = self.db.query(models.Comment).filter_by(update_id=update.id, user_id=bodhi.id) self.assertEqual(comment_q.count(), 1) - self.assertEqual(comment_q[0].text, config.get('testing_approval_msg_based_on_karma')) + self.assertEqual(comment_q[0].text, config.get('testing_approval_msg')) def test_non_autokarma_critpath_update_meeting_time_requirements_gets_one_comment(self): """ @@ -320,6 +321,7 @@ def test_non_autokarma_critpath_update_meeting_time_requirements_gets_one_commen """ update = self.db.query(models.Update).all()[0] update.autokarma = False + update.autotime = False update.request = None update.stable_karma = 10 update.critpath = True @@ -345,9 +347,7 @@ def test_non_autokarma_critpath_update_meeting_time_requirements_gets_one_commen bodhi = self.db.query(models.User).filter_by(name='bodhi').one() comment_q = self.db.query(models.Comment).filter_by(update_id=update.id, user_id=bodhi.id) self.assertEqual(comment_q.count(), 1) - self.assertEqual( - comment_q[0].text, - config.get('testing_approval_msg') % update.mandatory_days_in_testing) + self.assertEqual(comment_q[0].text, config.get('testing_approval_msg')) self.assertEqual(update.release.mandatory_days_in_testing, 7) self.assertEqual(update.mandatory_days_in_testing, 14) @@ -387,6 +387,7 @@ def test_non_autokarma_update_with_unmet_karma_requirement_after_time_met(self): """ update = self.db.query(models.Update).all()[0] update.autokarma = False + update.autotime = False update.request = None update.stable_karma = 10 update.status = models.UpdateStatus.testing @@ -403,9 +404,7 @@ def test_non_autokarma_update_with_unmet_karma_requirement_after_time_met(self): bodhi = self.db.query(models.User).filter_by(name='bodhi').one() comment_q = self.db.query(models.Comment).filter_by(update_id=update.id, user_id=bodhi.id) self.assertEqual(comment_q.count(), 1) - self.assertEqual( - comment_q[0].text, - config.get('testing_approval_msg') % update.release.mandatory_days_in_testing) + self.assertEqual(comment_q[0].text, config.get('testing_approval_msg')) # Set the release's mandatory days in testing to 0 to set up the condition for this test. @patch.dict(config, [('fedora.mandatory_days_in_testing', 0)]) @@ -416,6 +415,7 @@ def test_non_autokarma_update_without_mandatory_days_in_testing(self): """ update = self.db.query(models.Update).all()[0] update.autokarma = False + update.autotime = False update.request = None update.stable_karma = 1 update.status = models.UpdateStatus.testing @@ -449,6 +449,7 @@ def test_subsequent_comments_after_initial_push_comment(self): update.request = None update.status = models.UpdateStatus.testing update.date_testing = datetime.utcnow() - timedelta(days=14) + update.autotime = False self.db.flush() with patch('bodhi.server.scripts.approve_testing.initialize_db'): @@ -463,15 +464,9 @@ def test_subsequent_comments_after_initial_push_comment(self): cmnts = self.db.query(models.Comment).filter_by(update_id=update.id, user_id=bodhi.id) # There are 3 comments: testing_approval_msg, build change, testing_approval_msg self.assertEqual(cmnts.count(), 3) - self.assertEqual( - cmnts[0].text, - config.get('testing_approval_msg') % - update.release.mandatory_days_in_testing) + self.assertEqual(cmnts[0].text, config.get('testing_approval_msg')) self.assertEqual(cmnts[1].text, 'Removed build') - self.assertEqual( - cmnts[2].text, - config.get('testing_approval_msg') % - update.release.mandatory_days_in_testing) + self.assertEqual(cmnts[2].text, config.get('testing_approval_msg')) @patch('sys.exit') @patch('sys.stdout', new_callable=StringIO) @@ -488,6 +483,280 @@ def test_usage(self, stdout, exit): 'usage: nosetests \n(example: "nosetests development.ini")\n') exit.assert_called_once_with(1) + def test_autotime_update_meeting_test_requirements_gets_pushed(self): + """ + Ensure that an autotime update that meets the test requirements gets pushed to stable. + """ + update = self.db.query(models.Update).all()[0] + update.autokarma = False + update.autotime = True + update.request = None + update.stable_karma = 10 + update.stable_days = 7 + update.date_testing = datetime.utcnow() - timedelta(days=7) + update.status = models.UpdateStatus.testing + self.db.commit() + + with patch('bodhi.server.scripts.approve_testing.initialize_db'): + with patch('bodhi.server.scripts.approve_testing.get_appsettings', return_value=''): + approve_testing.main(['nosetests', 'some_config.ini']) + + self.assertEqual(update.request, models.UpdateRequest.stable) + + def test_autotime_update_does_not_meet_stable_days_doesnt_get_pushed(self): + """ + Ensure that an autotime update that meets the test requirements but has a longer + stable days define does not get push. + """ + update = self.db.query(models.Update).all()[0] + update.autokarma = False + update.autotime = True + update.request = None + update.stable_karma = 10 + update.stable_days = 10 + update.date_testing = datetime.utcnow() - timedelta(days=7) + update.status = models.UpdateStatus.testing + self.db.commit() + + with patch('bodhi.server.scripts.approve_testing.initialize_db'): + with patch('bodhi.server.scripts.approve_testing.get_appsettings', return_value=''): + approve_testing.main(['nosetests', 'some_config.ini']) + + self.assertEqual(update.request, None) + + def test_autotime_update_meeting_stable_days_get_pushed(self): + """ + Ensure that an autotime update that meets the stable days gets pushed. + """ + update = self.db.query(models.Update).all()[0] + update.autokarma = False + update.autotime = True + update.request = None + update.stable_karma = 10 + update.stable_days = 10 + update.date_testing = datetime.utcnow() - timedelta(days=10) + update.status = models.UpdateStatus.testing + self.db.commit() + + with patch('bodhi.server.scripts.approve_testing.initialize_db'): + with patch('bodhi.server.scripts.approve_testing.get_appsettings', return_value=''): + approve_testing.main(['nosetests', 'some_config.ini']) + + self.assertEqual(update.request, models.UpdateRequest.stable) + + def test_no_autotime_update_meeting_stable_days_and_test_requirement(self): + """ + Ensure that a normal update that meets the stable days and test requirements + doe not get pushed. + """ + update = self.db.query(models.Update).all()[0] + update.autokarma = False + update.autotime = False + update.request = None + update.stable_karma = 10 + update.stable_days = 10 + update.date_testing = datetime.utcnow() - timedelta(days=10) + update.status = models.UpdateStatus.testing + self.db.commit() + + with patch('bodhi.server.scripts.approve_testing.initialize_db'): + with patch('bodhi.server.scripts.approve_testing.get_appsettings', return_value=''): + approve_testing.main(['nosetests', 'some_config.ini']) + + self.assertEqual(update.request, None) + + @patch.dict(config, [('fedora.mandatory_days_in_testing', 2)]) + def test_autotime_update_does_not_meet_test_requirements(self): + """ + Ensure that an autotime update that does not meet the test requirements + does not pushed to stable. + """ + update = self.db.query(models.Update).all()[0] + update.autokarma = False + update.autotime = True + update.request = None + update.stable_days = update.mandatory_days_in_testing + update.stable_karma = 10 + update.date_testing = datetime.utcnow() - timedelta(days=1) + update.status = models.UpdateStatus.testing + self.db.commit() + + with patch('bodhi.server.scripts.approve_testing.initialize_db'): + with patch('bodhi.server.scripts.approve_testing.get_appsettings', return_value=''): + approve_testing.main(['nosetests', 'some_config.ini']) + + self.assertEqual(update.request, None) + + @patch.dict(config, [('fedora.mandatory_days_in_testing', 0)]) + def test_autotime_update_does_no_mandatory_days_in_testing(self): + """ + Ensure that an autotime update that does not have mandatory days in testing + does get pushed to stable. + """ + update = self.db.query(models.Update).all()[0] + update.autokarma = False + update.autotime = True + update.request = None + update.stable_karma = 10 + update.date_testing = datetime.utcnow() + update.status = models.UpdateStatus.testing + self.db.commit() + + with patch('bodhi.server.scripts.approve_testing.initialize_db'): + with patch('bodhi.server.scripts.approve_testing.get_appsettings', return_value=''): + approve_testing.main(['nosetests', 'some_config.ini']) + + self.assertEqual(update.request, models.UpdateRequest.stable) + + @patch.dict(config, [('fedora.mandatory_days_in_testing', 0)]) + def test_autotime_update_zero_day_in_testing_meeting_test_requirements_gets_pushed(self): + """ + Ensure that an autotime update with 0 mandatory_days_in_testing that meets + the test requirements gets pushed to stable. + """ + update = self.db.query(models.Update).all()[0] + update.autokarma = False + update.autotime = True + update.request = None + update.stable_karma = 10 + update.stable_days = 0 + update.date_testing = datetime.utcnow() - timedelta(days=0) + update.status = models.UpdateStatus.testing + self.db.commit() + + with patch('bodhi.server.scripts.approve_testing.initialize_db'): + with patch('bodhi.server.scripts.approve_testing.get_appsettings', return_value=''): + approve_testing.main(['nosetests', 'some_config.ini']) + + self.assertEqual(update.request, models.UpdateRequest.stable) + + @patch.dict(config, [('fedora.mandatory_days_in_testing', 0)]) + @patch.dict(config, [('test_gating.required', True)]) + def test_autotime_update_zero_day_in_testing_fail_gating_is_not_pushed(self): + """ + Ensure that an autotime update with 0 mandatory days in testing that failed gating + does not get pushed to stable. + """ + update = self.db.query(models.Update).all()[0] + update.autokarma = False + update.autotime = True + update.request = None + update.stable_karma = 10 + update.stable_days = 0 + update.test_gating_status = models.TestGatingStatus.failed + update.date_testing = datetime.utcnow() - timedelta(days=0) + update.status = models.UpdateStatus.testing + self.db.commit() + + with patch('bodhi.server.scripts.approve_testing.initialize_db'): + with patch('bodhi.server.scripts.approve_testing.get_appsettings', return_value=''): + approve_testing.main(['nosetests', 'some_config.ini']) + + self.assertEqual(update.request, None) + + def test_autotime_update_negative_karma_does_not_get_pushed(self): + """ + Ensure that an autotime update with negative karma does not get pushed. + """ + update = self.db.query(models.Update).all()[0] + update.autokarma = False + update.autotime = True + update.request = None + update.stable_karma = 10 + update.stable_days = 0 + update.date_testing = datetime.utcnow() - timedelta(days=0) + update.status = models.UpdateStatus.testing + update.comment(self.db, u'Failed to work', author=u'luke', karma=-1) + self.db.commit() + + with patch('bodhi.server.scripts.approve_testing.initialize_db'): + with patch('bodhi.server.scripts.approve_testing.get_appsettings', return_value=''): + approve_testing.main(['nosetests', 'some_config.ini']) + + self.assertEqual(update.request, None) + + def test_autotime_update_no_autokarma_met_karma_requirements_get_comments(self): + """ + Ensure that an autotime update which met the karma requirements but has autokarma off + get a comment to let the packager know that he can push the update to stable. + """ + update = self.db.query(models.Update).all()[0] + update.autokarma = False + update.autotime = True + update.request = None + update.stable_karma = 1 + update.stable_days = 10 + update.date_testing = datetime.utcnow() - timedelta(days=0) + update.status = models.UpdateStatus.testing + update.comment(self.db, u'Works great', author=u'luke', karma=1) + self.db.commit() + + with patch('bodhi.server.scripts.approve_testing.initialize_db'): + with patch('bodhi.server.scripts.approve_testing.get_appsettings', return_value=''): + approve_testing.main(['nosetests', 'some_config.ini']) + + self.assertEqual(update.request, None) + + bodhi = self.db.query(models.User).filter_by(name='bodhi').one() + cmnts = self.db.query(models.Comment).filter_by(update_id=update.id, user_id=bodhi.id) + self.assertEqual(cmnts.count(), 1) + self.assertEqual(cmnts[0].text, config.get('testing_approval_msg')) + + def test_autotime_update_no_autokarma_met_karma_and_time_requirements_get_pushed(self): + """ + Ensure that an autotime update which met the karma and time requirements but + has autokarma off gets pushed. + """ + update = self.db.query(models.Update).all()[0] + update.autokarma = False + update.autotime = True + update.request = None + update.stable_karma = 1 + update.stable_days = 0 + update.date_testing = datetime.utcnow() - timedelta(days=0) + update.status = models.UpdateStatus.testing + update.comment(self.db, u'Works great', author=u'luke', karma=1) + self.db.commit() + + with patch('bodhi.server.scripts.approve_testing.initialize_db'): + with patch('bodhi.server.scripts.approve_testing.get_appsettings', return_value=''): + approve_testing.main(['nosetests', 'some_config.ini']) + + self.assertEqual(update.request, models.UpdateRequest.stable) + + bodhi = self.db.query(models.User).filter_by(name='bodhi').one() + cmnts = self.db.query(models.Comment).filter_by(update_id=update.id, user_id=bodhi.id) + self.assertEqual(cmnts.count(), 2) + self.assertEqual(cmnts[0].text, config.get('testing_approval_msg')) + self.assertEqual(cmnts[1].text, 'This update has been submitted for stable by bodhi. ') + + def test_autotime_update_with_autokarma_met_karma_and_time_requirements_get_pushed(self): + """ + Ensure that an autotime update which met the karma and time requirements and has autokarma + and autotime enable gets pushed. + """ + update = self.db.query(models.Update).all()[0] + update.autokarma = True + update.autotime = True + update.request = None + update.stable_karma = 1 + update.stable_days = 0 + update.date_testing = datetime.utcnow() - timedelta(days=0) + update.status = models.UpdateStatus.testing + update.comment(self.db, u'Works great', author=u'luke', karma=1) + self.db.commit() + + with patch('bodhi.server.scripts.approve_testing.initialize_db'): + with patch('bodhi.server.scripts.approve_testing.get_appsettings', return_value=''): + approve_testing.main(['nosetests', 'some_config.ini']) + + self.assertEqual(update.request, models.UpdateRequest.stable) + + bodhi = self.db.query(models.User).filter_by(name='bodhi').one() + cmnts = self.db.query(models.Comment).filter_by(update_id=update.id, user_id=bodhi.id) + self.assertEqual(cmnts.count(), 1) + self.assertEqual(cmnts[0].text, 'This update has been submitted for stable by bodhi. ') + @patch('sys.exit') def test_log_level_is_left_alone(self, exit): """Ensure calling main() here leaves the global log level alone. diff --git a/bodhi/tests/server/services/test_updates.py b/bodhi/tests/server/services/test_updates.py index a03b93b3d5..2ba93d3659 100644 --- a/bodhi/tests/server/services/test_updates.py +++ b/bodhi/tests/server/services/test_updates.py @@ -490,7 +490,49 @@ def test_new_update_with_multiple_bugs(self, *args): self.assertEqual(up['bugs'][1]['bug_id'], 5678) self.assertEqual(up['bugs'][2]['bug_id'], 12345) - @mock.patch.dict('bodhi.server.validators.config', {'acl_system': 'dummy'}) + @mock.patch.dict('bodhi.server.validators.config', {'acl_system': u'dummy'}) + @mock.patch(**mock_valid_requirements) + @mock.patch('bodhi.server.notifications.publish') + def test_new_update_with_high_stable_days(self, publish, *args): + update = self.get_update('bodhi-2.0.0-2.fc17') + update['stable_days'] = 10 + r = self.app.post_json('/updates/', update) + up = r.json_body + + self.assertEqual(up['stable_days'], 10) + + @mock.patch.dict('bodhi.server.validators.config', {'acl_system': u'dummy'}) + @mock.patch(**mock_valid_requirements) + @mock.patch('bodhi.server.notifications.publish') + def test_new_update_with_invalid_stable_days(self, publish, *args): + update = self.get_update('bodhi-2.0.0-2.fc17') + update['stable_days'] = -1 + r = self.app.post_json('/updates/', update, status=400) + up = r.json_body + + self.assertEqual(up['status'], 'error') + self.assertEqual(up['errors'][0]['description'], "-1 is less than minimum value 0") + + @mock.patch(**mock_valid_requirements) + @mock.patch('bodhi.server.notifications.publish') + def test_edit_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) + self.assertEqual(r.json['stable_days'], 50) + + @mock.patch(**mock_valid_requirements) + @mock.patch('bodhi.server.notifications.publish') + def test_edit_update_too_low_stable_days(self, publish, *args): + args = self.get_update(u'bodhi-2.0.0-2.fc17') + args['stable_days'] = '1' + r = self.app.post_json('/updates/', args) + self.assertEqual(r.json['stable_days'], 7) + self.assertEqual(r.json['caveats'][0]['description'], + 'The number of stable days required was set to the mandatory ' + 'release value of 7 days') + + @mock.patch.dict('bodhi.server.validators.config', {'acl_system': u'dummy'}) @mock.patch(**mock_valid_requirements) def test_new_update_with_multiple_bugs_as_str(self, *args): update = self.get_update('bodhi-2.0.0-2.fc17') @@ -613,7 +655,7 @@ def test_obsoletion(self, *args): self.assertEqual(r['request'], 'testing') # Since we're obsoleting something owned by someone else. - self.assertEqual(r['caveats'][0]['description'], + self.assertEqual(r['caveats'][1]['description'], 'This update has obsoleted bodhi-2.0.0-2.fc17, ' 'and has inherited its bugs and notes.') @@ -658,13 +700,13 @@ def test_create_new_nonsecurity_update_when_previous_security_one_exists(self, * r = self.app.post_json('/updates/', args).json_body # Since we're trying to obsolete security update with non security update. - self.assertEqual(r['caveats'][0]['description'], + self.assertEqual(r['caveats'][1]['description'], 'Adjusting type of this update to security,' 'since it obsoletes another security update') self.assertEqual(r['request'], 'testing') # Since we're obsoleting something owned by someone else. - self.assertEqual(r['caveats'][1]['description'], + self.assertEqual(r['caveats'][2]['description'], 'This update has obsoleted bodhi-2.0.0-2.fc17, ' 'and has inherited its bugs and notes.') @@ -714,7 +756,7 @@ def test_obsoletion_security_update(self, *args): self.assertEqual(r['request'], 'testing') # Since we're obsoleting something owned by someone else. - self.assertEqual(r['caveats'][0]['description'], + self.assertEqual(r['caveats'][1]['description'], 'This update has obsoleted bodhi-2.0.0-2.fc17, ' 'and has inherited its bugs and notes.') @@ -773,7 +815,7 @@ def test_obsoletion_with_exception(self, *args): self.assertEqual(r['request'], 'testing') # The exception handler should have put an error message in the caveats. - self.assertEqual(r['caveats'][0]['description'], + self.assertEqual(r['caveats'][1]['description'], "Problem obsoleting older updates: bet you didn't see this coming!") # Check for the comment multiple ways. The comment will be about the update being submitted # for testing instead of being about the obsoletion, since the obsoletion failed. @@ -3060,9 +3102,10 @@ def test_pending_update_on_stable_karma_reached_autopush_disabled(self, *args): self.assertEqual(up.status, UpdateStatus.pending) self.assertEqual(up.request, UpdateRequest.testing) - text = str(config.get('testing_approval_msg_based_on_karma')) + text = str(config.get('testing_approval_msg')) up.comment(self.db, text, author='bodhi') - self.assertIn('pushed to stable now if the maintainer wishes', up.comments[-1]['text']) + self.assertEqual('This update can be pushed to stable now if the maintainer wishes', + up.comments[-1]['text']) @mock.patch(**mock_valid_requirements) def test_obsoletion_locked_with_open_request(self, *args): @@ -3714,8 +3757,11 @@ def test_update_with_older_build_in_testing_from_diff_user(self, r): resp = self.app.post_json('/updates/', args) # Note that this does **not** obsolete the other update - self.assertEqual(len(resp.json_body['caveats']), 1) + self.assertEqual(len(resp.json_body['caveats']), 2) self.assertEqual(resp.json_body['caveats'][0]['description'], + 'The number of stable days required was set to the mandatory ' + 'release value of 7 days') + self.assertEqual(resp.json_body['caveats'][1]['description'], "Please be aware that there is another update in " "flight owned by bob, containing " "bodhi-2.0-2.fc17. Are you coordinating with " @@ -3804,11 +3850,17 @@ def test_submitting_multi_release_updates(self, *args): data = r.json_body self.assertIn('caveats', data) - self.assertEqual(len(data['caveats']), 2) + self.assertEqual(len(data['caveats']), 4) self.assertEqual(data['caveats'][0]['description'], "Your update is being split into 2, one for each release.") + self.assertEqual(data['caveats'][1]['description'], + 'The number of stable days required was set to the mandatory ' + 'release value of 7 days') + self.assertEqual(data['caveats'][2]['description'], + 'The number of stable days required was set to the mandatory ' + 'release value of 7 days') self.assertEqual( - data['caveats'][1]['description'], + data['caveats'][3]['description'], "This update has obsoleted bodhi-2.0-1.fc17, and has inherited its bugs and notes.") self.assertIn('updates', data) @@ -4848,7 +4900,7 @@ def test_manually_push_to_stable_based_on_karma_request_none(self, *args): self.assertEqual(upd.autokarma, False) self.assertEqual(upd.pushed, True) - text = str(config.get('testing_approval_msg_based_on_karma')) + 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'] = [] @@ -4899,7 +4951,7 @@ def test_manually_push_to_stable_based_on_karma_request_testing(self, *args): self.assertEqual(upd.request, None) self.assertEqual(upd.autokarma, False) - text = str(config.get('testing_approval_msg_based_on_karma')) + 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'] = [] diff --git a/bodhi/tests/server/test_models.py b/bodhi/tests/server/test_models.py index 64ebeaf663..4c29eaf611 100644 --- a/bodhi/tests/server/test_models.py +++ b/bodhi/tests/server/test_models.py @@ -3159,7 +3159,7 @@ def test_has_stable_comment_at_7_days_after_bodhi_comment(self): # The update should be eligible to receive the testing_approval_msg now. self.assertEqual(self.obj.meets_testing_requirements, True) # Add the testing_approval_message - text = str(config.get('testing_approval_msg') % self.obj.days_in_testing) + text = str(config.get('testing_approval_msg')) self.obj.comment(self.db, text, author='bodhi') # met_testing_requirement() should return True since Bodhi has commented on the Update to @@ -3238,7 +3238,7 @@ def test_has_stable_comment_with_karma_after_bodhi_comment(self): self.obj.comment(self.db, 'testing', author='hunter2', karma=1) self.obj.comment(self.db, 'testing', author='hunter3', karma=1) # Add the testing_approval_message - text = config.get('testing_approval_msg_based_on_karma') + text = config.get('testing_approval_msg') self.obj.comment(self.db, text, author='bodhi') # met_testing_requirement() should return True since Bodhi has commented on the Update to diff --git a/docs/user/man_pages/bodhi.rst b/docs/user/man_pages/bodhi.rst index e71b15a002..ccfd10317f 100644 --- a/docs/user/man_pages/bodhi.rst +++ b/docs/user/man_pages/bodhi.rst @@ -236,6 +236,11 @@ The ``updates`` command allows users to interact with bodhi updates. Enable autokarma for this update. + ``--autotime`` + + Enable autotime for this update. Automatically push the update to stable based on the + time spent in testing. + ``--stable-karma `` Configure the stable karma threshold for the given value. @@ -244,6 +249,11 @@ The ``updates`` command allows users to interact with bodhi updates. Configure the unstable karma threshold for the given value. + ``--stable-days `` + + Configure the number of days an update has to spend in testing before + being automatically pushed to stable. + ``--suggest [logout | reboot]`` Suggest that the user logout or reboot upon applying the update. diff --git a/production.ini b/production.ini index 26dcc7b13b..eb57b7c866 100644 --- a/production.ini +++ b/production.ini @@ -15,16 +15,12 @@ use = egg:bodhi-server # updates that reach the required time in testing if they are not stable yet. Positional # substitution is used, and the %%d will be replaced with the time in testing required for the # update. -# testing_approval_msg = This update has reached %%d days in testing and can be pushed to stable now if the maintainer wishes +# testing_approval_msg = This update can be pushed to stable now if the maintainer wishes # not_yet_tested_msg = This update has not yet met the minimum testing requirements defined in the Package Update Acceptance Criteria # not_yet_tested_epel_msg = This update has not yet met the minimum testing requirements defined in the EPEL Update Policy -# Bodhi will post this comment on Updates that don't use autokarma when they reach the stable -# threshold. -# testing_approval_msg_based_on_karma = This update has reached the stable karma threshold and can be pushed to stable now if the maintainer wishes. - # The comment that Bodhi will post on updates when a user posts negative karma. # disable_automatic_push_to_stable = Bodhi is disabling automatic push to stable due to negative karma. The maintainer may push manually if they determine that the issue is not severe.