This is an automated email from the ASF dual-hosted git repository. gcruz pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/allura.git
commit 0ca517797c45c4b7159ddd5acf8bcf40112ece29 Author: Dave Brondsema <dbronds...@slashdotmedia.com> AuthorDate: Wed Mar 20 17:16:19 2024 -0400 [#8539] code simplifications --- Allura/allura/command/taskd_cleanup.py | 4 +-- Allura/allura/controllers/project.py | 2 +- Allura/allura/controllers/rest.py | 2 +- Allura/allura/controllers/site_admin.py | 2 +- Allura/allura/lib/utils.py | 2 +- Allura/allura/tests/decorators.py | 8 ++---- .../allura/tests/functional/test_neighborhood.py | 4 +-- .../tests/functional/test_personal_dashboard.py | 29 +++++++++++----------- .../allura/tests/functional/test_user_profile.py | 17 ++++++------- Allura/allura/tests/model/test_project.py | 11 ++++---- Allura/allura/tests/test_commands.py | 4 +-- Allura/allura/tests/test_helpers.py | 5 ++-- Allura/allura/tests/test_tasks.py | 10 ++++---- Allura/allura/tests/test_webhooks.py | 5 ++-- Allura/allura/tests/unit/test_session.py | 28 ++++++++++----------- ForgeActivity/forgeactivity/main.py | 2 +- .../tests/unit/test_root_controller.py | 4 +-- ForgeImporters/forgeimporters/github/__init__.py | 2 +- ForgeTracker/forgetracker/tracker_main.py | 2 +- ruff.toml | 5 ++++ 20 files changed, 71 insertions(+), 77 deletions(-) diff --git a/Allura/allura/command/taskd_cleanup.py b/Allura/allura/command/taskd_cleanup.py index 0f4ec913e..232773f93 100644 --- a/Allura/allura/command/taskd_cleanup.py +++ b/Allura/allura/command/taskd_cleanup.py @@ -154,7 +154,7 @@ class TaskdCleanupCommand(base.Command): def _check_taskd_status(self, pid): for i in range(self.options.num_retry): - retry = False if i == 0 else True + retry = i != 0 status = self._taskd_status(pid, retry) if ('taskd pid %s' % pid) in status: return 'OK' @@ -164,7 +164,7 @@ class TaskdCleanupCommand(base.Command): def _check_task(self, taskd_pid, task): for i in range(self.options.num_retry): - retry = False if i == 0 else True + retry = i != 0 status = self._taskd_status(taskd_pid, retry) line = 'taskd pid {} is currently handling task {}'.format( taskd_pid, task) diff --git a/Allura/allura/controllers/project.py b/Allura/allura/controllers/project.py index f4d8c9865..f04fa5b62 100644 --- a/Allura/allura/controllers/project.py +++ b/Allura/allura/controllers/project.py @@ -564,7 +564,7 @@ class NeighborhoodAdminController: set_nav(self.neighborhood) c.overview_form = W.neighborhood_overview_form allow_undelete = asbool(config.get('allow_project_undelete', True)) - allow_wiki_as_root = True if get_default_wiki_page() else False + allow_wiki_as_root = bool(get_default_wiki_page()) return dict( neighborhood=self.neighborhood, diff --git a/Allura/allura/controllers/rest.py b/Allura/allura/controllers/rest.py index f077b3d40..255348208 100644 --- a/Allura/allura/controllers/rest.py +++ b/Allura/allura/controllers/rest.py @@ -189,7 +189,7 @@ class Oauth1Validator(oauthlib.oauth1.RequestValidator): if request.environ.get('paste.testing'): # test suite is running return False - elif asbool(config.get('debug')) and config['base_url'].startswith('http://'): + elif asbool(config.get('debug')) and config['base_url'].startswith('http://'): # noqa: SIM103 # development w/o https return False else: diff --git a/Allura/allura/controllers/site_admin.py b/Allura/allura/controllers/site_admin.py index 07009028b..252aea353 100644 --- a/Allura/allura/controllers/site_admin.py +++ b/Allura/allura/controllers/site_admin.py @@ -184,7 +184,7 @@ class SiteAdminController: try: end_dt = datetime.strptime(end_dt, '%Y/%m/%d %H:%M:%S') except ValueError: - end_dt = start_dt - timedelta(days=3) if not end_dt else end_dt + end_dt = end_dt if end_dt else start_dt - timedelta(days=3) start = bson.ObjectId.from_datetime(start_dt) end = bson.ObjectId.from_datetime(end_dt) nb = M.Neighborhood.query.get(name='Users') diff --git a/Allura/allura/lib/utils.py b/Allura/allura/lib/utils.py index 0cf6b8c3c..de33f559b 100644 --- a/Allura/allura/lib/utils.py +++ b/Allura/allura/lib/utils.py @@ -337,7 +337,7 @@ class AntiSpam: if params is None: params = request.params new_params = dict(params) - if not request.method == 'GET': + if request.method != 'GET': obj = None try: new_params.pop('timestamp', None) diff --git a/Allura/allura/tests/decorators.py b/Allura/allura/tests/decorators.py index 6b1ea527e..7311f58be 100644 --- a/Allura/allura/tests/decorators.py +++ b/Allura/allura/tests/decorators.py @@ -109,12 +109,8 @@ class raises: def __exit__(self, exc_type, exc_val, exc_t): if exc_type: self.exc = exc_val - if issubclass(exc_type, self.ExcType): - # ok - return True - else: - # root exception will be raised, untouched - return False + # otherwise root exception will be raised, untouched + return issubclass(exc_type, self.ExcType) else: raise AssertionError('Did not raise %s' % self.ExcType) diff --git a/Allura/allura/tests/functional/test_neighborhood.py b/Allura/allura/tests/functional/test_neighborhood.py index ec1633172..e7e675920 100644 --- a/Allura/allura/tests/functional/test_neighborhood.py +++ b/Allura/allura/tests/functional/test_neighborhood.py @@ -342,7 +342,7 @@ class TestNeighborhood(TestController): neighborhood = M.Neighborhood.query.get(name='Adobe') neighborhood.features['css'] = 'picker' r = self.app.get('/adobe/') - while isinstance(r.response, HTTPFound) or isinstance(r.response, HTTPMovedPermanently): + while isinstance(r.response, (HTTPFound, HTTPMovedPermanently)): r = r.follow() assert test_css in r r = self.app.get('/adobe/_admin/overview', @@ -352,7 +352,7 @@ class TestNeighborhood(TestController): neighborhood = M.Neighborhood.query.get(name='Adobe') neighborhood.features['css'] = 'custom' r = self.app.get('/adobe/') - while isinstance(r.response, HTTPFound) or isinstance(r.response, HTTPMovedPermanently): + while isinstance(r.response, (HTTPFound, HTTPMovedPermanently)): r = r.follow() assert test_css in r r = self.app.get('/adobe/_admin/overview', diff --git a/Allura/allura/tests/functional/test_personal_dashboard.py b/Allura/allura/tests/functional/test_personal_dashboard.py index 9a64ba7cc..b6a68eb3d 100644 --- a/Allura/allura/tests/functional/test_personal_dashboard.py +++ b/Allura/allura/tests/functional/test_personal_dashboard.py @@ -50,21 +50,20 @@ class TestPersonalDashboard(TestController): return m eps = list(map(ep, ['a', 'b', 'c', 'd'])) order = {'personal_dashboard_sections.order': 'b, d,c , f '} - with mock.patch('allura.lib.helpers.iter_entry_points') as iep: - with mock.patch.dict(tg.config, order): - iep.return_value = eps - sections = SectionsUtil.load_sections('personal_dashboard') - assert sections == [ - eps[1].load(), - eps[3].load(), - eps[2].load(), - eps[0].load()] - r = self.app.get('/dashboard') - assert 'Section a' in r.text - assert 'Section b' in r.text - assert 'Section c' in r.text - assert 'Section d' in r.text - assert 'Section f' not in r.text + with mock.patch('allura.lib.helpers.iter_entry_points') as iep, mock.patch.dict(tg.config, order): + iep.return_value = eps + sections = SectionsUtil.load_sections('personal_dashboard') + assert sections == [ + eps[1].load(), + eps[3].load(), + eps[2].load(), + eps[0].load()] + r = self.app.get('/dashboard') + assert 'Section a' in r.text + assert 'Section b' in r.text + assert 'Section c' in r.text + assert 'Section d' in r.text + assert 'Section f' not in r.text class TestTicketsSection(TrackerTestController): diff --git a/Allura/allura/tests/functional/test_user_profile.py b/Allura/allura/tests/functional/test_user_profile.py index e6de739d1..230bafb95 100644 --- a/Allura/allura/tests/functional/test_user_profile.py +++ b/Allura/allura/tests/functional/test_user_profile.py @@ -48,15 +48,14 @@ class TestUserProfileSections(TestController): order = {'user_profile_sections.order': 'b, d,c , f '} if hasattr(type(app), '_sections'): delattr(type(app), '_sections') - with mock.patch('allura.lib.helpers.iter_entry_points') as iep: - with mock.patch.dict(tg.config, order): - iep.return_value = eps - sections = app.profile_sections - assert sections == [ - eps[1].load(), - eps[3].load(), - eps[2].load(), - eps[0].load()] + with mock.patch('allura.lib.helpers.iter_entry_points') as iep, mock.patch.dict(tg.config, order): + iep.return_value = eps + sections = app.profile_sections + assert sections == [ + eps[1].load(), + eps[3].load(), + eps[2].load(), + eps[0].load()] r = self.app.get('/u/test-user/profile') assert 'Section a' in r.text assert 'Section b' in r.text diff --git a/Allura/allura/tests/model/test_project.py b/Allura/allura/tests/model/test_project.py index b5850b87c..fc7b2bb02 100644 --- a/Allura/allura/tests/model/test_project.py +++ b/Allura/allura/tests/model/test_project.py @@ -120,12 +120,11 @@ class TestProjectModel: def test_subproject(self): project = M.Project.query.get(shortname='test') - with td.raises(ToolError): - with patch('allura.lib.plugin.ProjectRegistrationProvider') as Provider: - Provider.get().shortname_validator.to_python.side_effect = Invalid( - 'name', 'value', {}) - # name doesn't validate - sp = project.new_subproject('test-proj-nose') + with td.raises(ToolError), patch('allura.lib.plugin.ProjectRegistrationProvider') as Provider: + Provider.get().shortname_validator.to_python.side_effect = Invalid( + 'name', 'value', {}) + # name doesn't validate + sp = project.new_subproject('test-proj-nose') sp = project.new_subproject('test-proj-nose') spp = sp.new_subproject('spp') ThreadLocalODMSession.flush_all() diff --git a/Allura/allura/tests/test_commands.py b/Allura/allura/tests/test_commands.py index 7ad19a1f5..ed304676c 100644 --- a/Allura/allura/tests/test_commands.py +++ b/Allura/allura/tests/test_commands.py @@ -435,14 +435,14 @@ def test_status_log_retries(): cmd._taskd_status.return_value = '' cmd.options = Mock(num_retry=10) cmd._check_taskd_status(123) - expected_calls = [call(123, False if i == 0 else True) for i in range(10)] + expected_calls = [call(123, i != 0) for i in range(10)] assert cmd._taskd_status.mock_calls == expected_calls cmd._taskd_status = Mock() cmd._taskd_status.return_value = '' cmd.options = Mock(num_retry=3) cmd._check_task(123, Mock()) - expected_calls = [call(123, False if i == 0 else True) for i in range(3)] + expected_calls = [call(123, i != 0) for i in range(3)] assert cmd._taskd_status.mock_calls == expected_calls diff --git a/Allura/allura/tests/test_helpers.py b/Allura/allura/tests/test_helpers.py index 99eae9ce4..71318eef0 100644 --- a/Allura/allura/tests/test_helpers.py +++ b/Allura/allura/tests/test_helpers.py @@ -542,9 +542,8 @@ def test_login_overlay(): raise HTTPUnauthorized() with h.login_overlay(exceptions=['foo']): raise HTTPUnauthorized() - with td.raises(HTTPUnauthorized): - with h.login_overlay(exceptions=['foobar']): - raise HTTPUnauthorized() + with td.raises(HTTPUnauthorized), h.login_overlay(exceptions=['foobar']): + raise HTTPUnauthorized() class TestIterEntryPoints(TestCase): diff --git a/Allura/allura/tests/test_tasks.py b/Allura/allura/tests/test_tasks.py index 54e936ec6..81fb4123d 100644 --- a/Allura/allura/tests/test_tasks.py +++ b/Allura/allura/tests/test_tasks.py @@ -584,11 +584,11 @@ class TestNotificationTasks: setup_global_objects() def test_delivers_messages(self): - with mock.patch.object(M.Mailbox, 'deliver') as deliver: - with mock.patch.object(M.Mailbox, 'fire_ready') as fire_ready: - notification_tasks.notify('42', ['52'], 'none') - deliver.assert_called_with('42', ['52'], 'none') - fire_ready.assert_called_with() + with mock.patch.object(M.Mailbox, 'deliver') as deliver, \ + mock.patch.object(M.Mailbox, 'fire_ready') as fire_ready: + notification_tasks.notify('42', ['52'], 'none') + deliver.assert_called_with('42', ['52'], 'none') + fire_ready.assert_called_with() @event_handler('my_event') diff --git a/Allura/allura/tests/test_webhooks.py b/Allura/allura/tests/test_webhooks.py index 63935734d..e8432415d 100644 --- a/Allura/allura/tests/test_webhooks.py +++ b/Allura/allura/tests/test_webhooks.py @@ -564,9 +564,8 @@ class TestRepoPushWebhookSender(TestWebhookBase): def test_get_payload(self): sender = RepoPushWebhookSender() _ci = lambda x: MagicMock(webhook_info={'id': str(x)}, parent_ids=['0']) - with patch.object(self.git.repo, 'commit', new=_ci): - with h.push_config(c, app=self.git): - result = sender.get_payload(commit_ids=['1', '2', '3'], ref='ref') + with patch.object(self.git.repo, 'commit', new=_ci), h.push_config(c, app=self.git): + result = sender.get_payload(commit_ids=['1', '2', '3'], ref='ref') expected_result = { 'size': 3, 'commits': [{'id': '1'}, {'id': '2'}, {'id': '3'}], diff --git a/Allura/allura/tests/unit/test_session.py b/Allura/allura/tests/unit/test_session.py index e2042c039..282b5678e 100644 --- a/Allura/allura/tests/unit/test_session.py +++ b/Allura/allura/tests/unit/test_session.py @@ -46,14 +46,13 @@ def test_extensions_cm(): def test_extensions_cm_raises(): session = mock.Mock(_kwargs=dict(extensions=[])) extension = mock.Mock() - with td.raises(ValueError): - with substitute_extensions(session, [extension]) as sess: - session.flush.side_effect = AttributeError - assert session.flush.call_count == 1 - assert session.close.call_count == 1 - assert sess == session - assert sess._kwargs['extensions'] == [extension] - raise ValueError('test') + with td.raises(ValueError), substitute_extensions(session, [extension]) as sess: + session.flush.side_effect = AttributeError + assert session.flush.call_count == 1 + assert session.close.call_count == 1 + assert sess == session + assert sess._kwargs['extensions'] == [extension] + raise ValueError('test') assert session.flush.call_count == 1 assert session.close.call_count == 1 assert session._kwargs['extensions'] == [] @@ -62,13 +61,12 @@ def test_extensions_cm_raises(): def test_extensions_cm_flush_raises(): session = mock.Mock(_kwargs=dict(extensions=[])) extension = mock.Mock() - with td.raises(AttributeError): - with substitute_extensions(session, [extension]) as sess: - session.flush.side_effect = AttributeError - assert session.flush.call_count == 1 - assert session.close.call_count == 1 - assert sess == session - assert sess._kwargs['extensions'] == [extension] + with td.raises(AttributeError), substitute_extensions(session, [extension]) as sess: + session.flush.side_effect = AttributeError + assert session.flush.call_count == 1 + assert session.close.call_count == 1 + assert sess == session + assert sess._kwargs['extensions'] == [extension] assert session.flush.call_count == 2 assert session.close.call_count == 1 assert session._kwargs['extensions'] == [] diff --git a/ForgeActivity/forgeactivity/main.py b/ForgeActivity/forgeactivity/main.py index 5d85145da..de05a1944 100644 --- a/ForgeActivity/forgeactivity/main.py +++ b/ForgeActivity/forgeactivity/main.py @@ -168,7 +168,7 @@ class ForgeActivityController(BaseController): followee=followee, following=following, timeline=filtered_timeline, - noindex=False if filtered_timeline else True, + noindex=not filtered_timeline, page=page, limit=limit, has_more=has_more, diff --git a/ForgeFeedback/forgefeedback/tests/unit/test_root_controller.py b/ForgeFeedback/forgefeedback/tests/unit/test_root_controller.py index fd04a04d2..0df97874d 100644 --- a/ForgeFeedback/forgefeedback/tests/unit/test_root_controller.py +++ b/ForgeFeedback/forgefeedback/tests/unit/test_root_controller.py @@ -35,10 +35,10 @@ class TestFeedbackApp(FeedbackTestWithModel): def test_index(self): reviews = feedback_main.RootController().index() - assert True if not reviews['user_has_already_reviewed'] else False + assert bool(not reviews['user_has_already_reviewed']) create_feedbacks() reviews = feedback_main.RootController().index() - assert True if reviews['user_has_already_reviewed'] else False + assert bool(reviews['user_has_already_reviewed']) def test_feedback(self): create_feedbacks() diff --git a/ForgeImporters/forgeimporters/github/__init__.py b/ForgeImporters/forgeimporters/github/__init__.py index f14d215f7..57deee4c4 100644 --- a/ForgeImporters/forgeimporters/github/__init__.py +++ b/ForgeImporters/forgeimporters/github/__init__.py @@ -208,7 +208,7 @@ def oauth_app_basic_auth(config): def valid_access_token(access_token, scopes_required=None): tok_details = access_token_details(access_token) - if not tok_details.status_code == 200: + if tok_details.status_code != 200: return False if scopes_required and not all(scope_req in tok_details.json()['scopes'] for scope_req in scopes_required): diff --git a/ForgeTracker/forgetracker/tracker_main.py b/ForgeTracker/forgetracker/tracker_main.py index e70a61d7c..ad6ab614c 100644 --- a/ForgeTracker/forgetracker/tracker_main.py +++ b/ForgeTracker/forgetracker/tracker_main.py @@ -1329,7 +1329,7 @@ class changelog: if key in self.originals: orig_value = self.originals[key] curr_value = self.data[key] - if not orig_value == curr_value: + if orig_value != curr_value: t.append((key, (orig_value, curr_value))) return t diff --git a/ruff.toml b/ruff.toml index 4337590c5..6df46f41d 100644 --- a/ruff.toml +++ b/ruff.toml @@ -36,6 +36,11 @@ lint.select = [ "PLC", "PLE", "PLW", + "SIM101", + "SIM103", + "SIM117", + "SIM2", + "SIM401", "FA", # future annotations (to ensure compatibility with `target-version`) ]