From b8d0f89a419b30d133446cd42c565a8edd920ce4 Mon Sep 17 00:00:00 2001 From: eviljeff Date: Fri, 18 Sep 2026 13:15:27 +0100 Subject: [PATCH 1/2] fix more ruff rule exclusions; ignore BLE001 intentionally --- pyproject.toml | 6 +- .../commands/fake_cinder_webhook.py | 3 +- src/olympia/abuse/tests/test_cinder.py | 2 +- src/olympia/access/acl.py | 7 +- src/olympia/accounts/views.py | 9 +- src/olympia/activity/models.py | 4 +- src/olympia/addons/indexers.py | 5 +- src/olympia/addons/serializers.py | 13 +- src/olympia/addons/tasks.py | 27 ++-- src/olympia/addons/tests/test_decorators.py | 6 +- src/olympia/addons/tests/test_tasks.py | 2 +- src/olympia/addons/tests/test_views.py | 30 +++-- src/olympia/amo/models.py | 2 +- src/olympia/amo/reverse.py | 2 +- src/olympia/amo/tests/test_amo_utils.py | 6 +- src/olympia/amo/tests/test_commands.py | 18 +-- src/olympia/amo/tests/test_helpers.py | 28 ++-- src/olympia/amo/tests/test_monitor.py | 6 +- src/olympia/amo/tests/test_settings.py | 5 +- src/olympia/amo/tests/test_url_prefix.py | 6 +- src/olympia/amo/tests/test_utils.py | 6 +- src/olympia/amo/urlresolvers.py | 2 +- src/olympia/amo/utils.py | 18 ++- src/olympia/api/fields.py | 2 +- src/olympia/api/tests/test_exceptions.py | 20 +-- src/olympia/blocklist/tests/test_mlbf.py | 4 +- src/olympia/core/db/mysql/base.py | 10 +- .../devhub/file_validation_annotations.py | 21 ++- src/olympia/devhub/forms.py | 27 ++-- src/olympia/devhub/tasks.py | 13 +- src/olympia/devhub/tests/test_forms.py | 28 ++-- src/olympia/devhub/tests/test_models.py | 2 +- src/olympia/devhub/tests/test_tasks.py | 35 +++-- src/olympia/devhub/tests/test_views.py | 11 +- src/olympia/devhub/tests/test_views_edit.py | 126 +++++++++--------- src/olympia/devhub/tests/test_views_submit.py | 18 +-- .../devhub/tests/test_views_validation.py | 8 +- .../devhub/tests/test_views_versions.py | 2 +- src/olympia/files/models.py | 5 +- src/olympia/files/tasks.py | 2 +- src/olympia/files/tests/test_tasks.py | 26 ++-- src/olympia/files/utils.py | 23 ++-- src/olympia/hero/models.py | 9 +- src/olympia/landfill/images.py | 6 +- src/olympia/landfill/serializers.py | 8 +- src/olympia/ratings/admin.py | 2 +- src/olympia/ratings/tests/test_admin.py | 5 +- src/olympia/reviewers/forms.py | 12 +- .../management/commands/auto_approve.py | 24 ++-- .../reviewers/tests/test_decorators.py | 2 +- src/olympia/reviewers/tests/test_forms.py | 15 +-- src/olympia/reviewers/tests/test_models.py | 2 +- src/olympia/reviewers/views.py | 13 +- src/olympia/signing/views.py | 15 +-- src/olympia/stats/tests/test_views.py | 2 +- src/olympia/stats/utils.py | 2 +- src/olympia/stats/views.py | 2 +- src/olympia/translations/tests/test_fields.py | 2 +- src/olympia/users/tests/test_admin.py | 27 ++-- src/olympia/users/tests/test_tasks.py | 12 +- tests/make/test_health_check.py | 2 +- 61 files changed, 380 insertions(+), 378 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 33a22afb5a22..f65d64ddc8be 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,15 +41,11 @@ ignore = [ "DTZ", # flake8-datetimez (except DTZ003) - we only run in UTC so unnecessary "ISC004", # implicit-string-concatenation-in-collection-literal - too many false positives "RUF015", # unnecessary-iterable-allocation-for-first-element - makes code unnecessarily complex + "BLE001", # blind-except - the majority of matches are intentional (health checks, best-effort tasks) # The following rules fail currently, and should probably eventually be addressed # They are in most-error-occuring order, descending "UP031", # printf-string-formatting "RUF012", # mutable-class-default - "SIM115", # open-file-with-context-handler - "BLE001", # blind-except - "RUF059", # unused-unpacked-variable - "SIM102", # collapsible-if - "FLY002", # static-join-to-f-string ] extend-select = [ "B", # flake8-bugbear diff --git a/src/olympia/abuse/management/commands/fake_cinder_webhook.py b/src/olympia/abuse/management/commands/fake_cinder_webhook.py index be3bdcce58d1..da80d45ba888 100644 --- a/src/olympia/abuse/management/commands/fake_cinder_webhook.py +++ b/src/olympia/abuse/management/commands/fake_cinder_webhook.py @@ -26,7 +26,8 @@ def handle(self, *args, **options): if settings.ENV != 'local': raise CommandError('Only works in local environments') try: - body = open(options['payload_filename'], 'rb').read() + with open(options['payload_filename'], 'rb') as payload_file: + body = payload_file.read() except FileNotFoundError as exc: raise CommandError( 'Cannot find payload file. Try using --payload=.' diff --git a/src/olympia/abuse/tests/test_cinder.py b/src/olympia/abuse/tests/test_cinder.py index 75329365746c..9b626238d8a5 100644 --- a/src/olympia/abuse/tests/test_cinder.py +++ b/src/olympia/abuse/tests/test_cinder.py @@ -1626,7 +1626,7 @@ def test_post_queue_move_no_versions_to_flag(self): assert ActivityLog.objects.count() == 0 def test_post_queue_move_with_multiple_reports_including_one_with_no_versions(self): - cinder_instance, cinder_job, listed_version, unlisted_version = ( + cinder_instance, cinder_job, listed_version, _unlisted_version = ( self._setup_post_queue_move_test() ) other_version = version_factory( diff --git a/src/olympia/access/acl.py b/src/olympia/access/acl.py index a4c781cf65ec..c3e44bdcb531 100644 --- a/src/olympia/access/acl.py +++ b/src/olympia/access/acl.py @@ -7,9 +7,10 @@ def match_rules(rules, app, action): """ for rule in rules.split(','): rule_app, rule_action = rule.split(':') - if rule_app == '*' or rule_app == app: - if rule_action == '*' or rule_action == action or action == '%': - return True + if (rule_app == '*' or rule_app == app) and ( + rule_action == '*' or rule_action == action or action == '%' + ): + return True return False diff --git a/src/olympia/accounts/views.py b/src/olympia/accounts/views.py index f03e8ddf4f70..3b4bbac5221e 100644 --- a/src/olympia/accounts/views.py +++ b/src/olympia/accounts/views.py @@ -393,11 +393,10 @@ def get(self, request, user, identity, next_path, token_data): # on, we extract that information from the next_path if present # and set locale/app on the prefixer instance that reverse() will # use automatically. - if next_path: - if prefixer := get_url_prefix(): - splitted = prefixer.split_path(next_path) - prefixer.locale = splitted[0] - prefixer.app = splitted[1] + if next_path and (prefixer := get_url_prefix()): + splitted = prefixer.split_path(next_path) + prefixer.locale = splitted[0] + prefixer.app = splitted[1] edit_page = reverse('users.edit') if next_path: next_path = f'{edit_page}?to={quote_plus(next_path)}' diff --git a/src/olympia/activity/models.py b/src/olympia/activity/models.py index 0e978c11cc20..83ef83b843c1 100644 --- a/src/olympia/activity/models.py +++ b/src/olympia/activity/models.py @@ -139,7 +139,7 @@ def transfer(self, new_addon): # arguments is a structure: # ``arguments = [{'addons.addon':12}, {'addons.addon':1}, ... ]`` arguments = json.loads(self.activity_log._arguments) - except Exception: + except (TypeError, json.JSONDecodeError): log.info( 'unserializing data from addon_log failed: %s' % self.activity_log.id ) @@ -614,7 +614,7 @@ def handle_renames(value): # `arguments_data` will be a list of dicts like: # `[{'addons.addon':12}, {'addons.addon':1}, ... ]` activity.arguments_data = json.loads(activity._arguments) - except Exception as e: + except (TypeError, json.JSONDecodeError) as e: log.info('unserializing data from activity_log failed: %s', activity.id) log.info(e) activity.arguments_data = [] diff --git a/src/olympia/addons/indexers.py b/src/olympia/addons/indexers.py index 8d0309cd5f11..c31e7c9084e5 100644 --- a/src/olympia/addons/indexers.py +++ b/src/olympia/addons/indexers.py @@ -643,9 +643,8 @@ def extract_document(cls, obj): data['colors'] = None # Extract dominant colors from static themes. - if obj.type == amo.ADDON_STATICTHEME: - if obj.current_previews: - data['colors'] = obj.current_previews[0].colors + if obj.type == amo.ADDON_STATICTHEME and obj.current_previews: + data['colors'] = obj.current_previews[0].colors data['app'] = [app.id for app in obj.compatible_apps] # We can use all_categories because the indexing code goes through the diff --git a/src/olympia/addons/serializers.py b/src/olympia/addons/serializers.py index f81ecce43d83..0b20e1edf630 100644 --- a/src/olympia/addons/serializers.py +++ b/src/olympia/addons/serializers.py @@ -1270,12 +1270,13 @@ def run_validation(self, data=serializers.empty): def validate_slug(self, value): slug_validator(value) - if not self.instance or value != self.instance.slug: - # DeniedSlug.blocked checks for all numeric slugs as well as being denied. - if DeniedSlug.blocked(value): - raise exceptions.ValidationError( - gettext('This slug cannot be used. Please choose another.') - ) + # DeniedSlug.blocked checks for all numeric slugs as well as being denied. + if (not self.instance or value != self.instance.slug) and DeniedSlug.blocked( + value + ): + raise exceptions.ValidationError( + gettext('This slug cannot be used. Please choose another.') + ) return value diff --git a/src/olympia/addons/tasks.py b/src/olympia/addons/tasks.py index 656065ae3710..47351bee0b89 100644 --- a/src/olympia/addons/tasks.py +++ b/src/olympia/addons/tasks.py @@ -141,20 +141,21 @@ def restore_all_addon_media_from_backup(id, **kwargs): if disabled_addon_content: log.info('Found some disable content to restore for addon %s', addon.pk) if backup_storage_enabled(): - if disabled_addon_content.icon_backup_name: - if icon_contents := download_file_contents_from_backup_storage( + if disabled_addon_content.icon_backup_name and ( + icon_contents := download_file_contents_from_backup_storage( disabled_addon_content.icon_backup_name - ): - icon_path = addon.get_icon_path('original') - log.info('Restoring icon %s for addon %s', icon_path, addon.pk) - with storage.open(icon_path, 'wb') as original_file: - original_file.write(icon_contents) - resize_icon.delay( - icon_path, - addon.pk, - amo.ADDON_ICON_SIZES, - set_modified_on=addon.serializable_reference(), - ) + ) + ): + icon_path = addon.get_icon_path('original') + log.info('Restoring icon %s for addon %s', icon_path, addon.pk) + with storage.open(icon_path, 'wb') as original_file: + original_file.write(icon_contents) + resize_icon.delay( + icon_path, + addon.pk, + amo.ADDON_ICON_SIZES, + set_modified_on=addon.serializable_reference(), + ) for deleted_preview in disabled_addon_content.deletedpreviewfile_set.all(): preview = deleted_preview.preview if preview_contents := download_file_contents_from_backup_storage( diff --git a/src/olympia/addons/tests/test_decorators.py b/src/olympia/addons/tests/test_decorators.py index 4f3cc8f4504c..3b8f4f501a0b 100644 --- a/src/olympia/addons/tests/test_decorators.py +++ b/src/olympia/addons/tests/test_decorators.py @@ -105,7 +105,7 @@ def test_slug_isdigit(self): addon.update(slug=str(addon.id)) r = self.view(self.request, addon.slug) assert r == mock.sentinel.OK - request, addon_ = self.func.call_args[0] + addon_ = self.func.call_args[0][1] assert addon_ == addon @mock.patch( @@ -167,7 +167,7 @@ def test_unlisted_addon_owner(self): """Addon owners have access.""" self.change_channel_for_addon(self.addon, amo.CHANNEL_UNLISTED) assert self.view(self.request, self.addon.slug) == mock.sentinel.OK - request, addon = self.func.call_args[0] + addon = self.func.call_args[0][1] assert addon == self.addon @mock.patch( @@ -180,5 +180,5 @@ def test_unlisted_addon_unlisted_admin(self): """Unlisted addon reviewers have access.""" self.change_channel_for_addon(self.addon, amo.CHANNEL_UNLISTED) assert self.view(self.request, self.addon.slug) == mock.sentinel.OK - request, addon = self.func.call_args[0] + addon = self.func.call_args[0][1] assert addon == self.addon diff --git a/src/olympia/addons/tests/test_tasks.py b/src/olympia/addons/tests/test_tasks.py index fe382d774df6..c49a52c0f670 100644 --- a/src/olympia/addons/tests/test_tasks.py +++ b/src/olympia/addons/tests/test_tasks.py @@ -541,7 +541,7 @@ def _uploader(self, resize_size, final_size): img = get_image_path('mozilla.png') original_size = (339, 128) - src = tempfile.NamedTemporaryFile( + src = tempfile.NamedTemporaryFile( # noqa: SIM115 (temp file used across the test (delete=False)) mode='r+b', suffix='.png', delete=False, dir=settings.TMP_PATH ) diff --git a/src/olympia/addons/tests/test_views.py b/src/olympia/addons/tests/test_views.py index 7d1162478d09..f2dc7d87adfb 100644 --- a/src/olympia/addons/tests/test_views.py +++ b/src/olympia/addons/tests/test_views.py @@ -3363,7 +3363,7 @@ def _submit_source(self, filepath, error=False): raise NotImplementedError def _generate_source_tar(self, suffix='.tar.gz', data=b't' * (2**21), mode=None): - source = tempfile.NamedTemporaryFile(suffix=suffix, dir=settings.TMP_PATH) + source = tempfile.NamedTemporaryFile(suffix=suffix, dir=settings.TMP_PATH) # noqa: SIM115 (temp file returned to caller) if mode is None: mode = 'w:bz2' if suffix.endswith('.tar.bz2') else 'w:gz' with tarfile.open(fileobj=source, mode=mode) as tar_file: @@ -3377,7 +3377,7 @@ def _generate_source_tar(self, suffix='.tar.gz', data=b't' * (2**21), mode=None) def _generate_source_zip( self, suffix='.zip', data='z' * (2**21), compression=zipfile.ZIP_DEFLATED ): - source = tempfile.NamedTemporaryFile(suffix=suffix, dir=settings.TMP_PATH) + source = tempfile.NamedTemporaryFile(suffix=suffix, dir=settings.TMP_PATH) # noqa: SIM115 (temp file returned to caller) with zipfile.ZipFile(source, 'w', compression=compression) as zip_file: zip_file.writestr('foo', data) source.seek(0) @@ -4323,11 +4323,12 @@ def test_compatibility_with_appversion_locked_from_manifest(self): def _submit_source(self, filepath, error=False): _, filename = os.path.split(filepath) - src = SimpleUploadedFile( - filename, - open(filepath, 'rb').read(), - content_type=mimetypes.guess_type(filename)[0], - ) + with open(filepath, 'rb') as source_file: + src = SimpleUploadedFile( + filename, + source_file.read(), + content_type=mimetypes.guess_type(filename)[0], + ) response = self.client.post( self.url, data={**self.minimal_data, 'source': src}, format='multipart' ) @@ -4801,11 +4802,12 @@ def test_delete_source_formdata(self): def _submit_source(self, filepath, error=False): _, filename = os.path.split(filepath) - src = SimpleUploadedFile( - filename, - open(filepath, 'rb').read(), - content_type=mimetypes.guess_type(filename)[0], - ) + with open(filepath, 'rb') as source_file: + src = SimpleUploadedFile( + filename, + source_file.read(), + content_type=mimetypes.guess_type(filename)[0], + ) response = self.client.patch(self.url, data={'source': src}, format='multipart') if not error: assert response.status_code == 200, response.content @@ -4882,7 +4884,7 @@ def test_submit_source_pending_rejection_triggers_needs_human_review(self): pending_rejection_by=user_factory(), pending_content_rejection=False, ) - response, self.version = self._submit_source(new_source) + _response, self.version = self._submit_source(new_source) self.addon.reload() assert self.version.source assert self.version.needshumanreview_set.filter(is_active=True).exists() @@ -7242,7 +7244,7 @@ def test_exclude_addons(self): # Exclude addon2 and addon3 by slug. data = self.perform_search( - self.url, {'exclude_addons': ','.join((addon2.slug, addon3.slug))} + self.url, {'exclude_addons': f'{addon2.slug},{addon3.slug}'} ) assert len(data['results']) == 1 diff --git a/src/olympia/amo/models.py b/src/olympia/amo/models.py index b1cb0d99adca..515888579b33 100644 --- a/src/olympia/amo/models.py +++ b/src/olympia/amo/models.py @@ -583,7 +583,7 @@ def delete_preview_files(cls, sender, instance, **kw): try: log.info(f'Removing filename: {filename} for preview: {instance.pk}') storage.delete(filename) - except Exception as e: + except OSError as e: log.error(f'Error deleting preview file ({filename}): {e}') diff --git a/src/olympia/amo/reverse.py b/src/olympia/amo/reverse.py index 5a0afc03e906..1bbddef36fa7 100644 --- a/src/olympia/amo/reverse.py +++ b/src/olympia/amo/reverse.py @@ -54,7 +54,7 @@ def resolve(path, urlconf=None): """Wraps django's resolve to remove the locale and app from the path.""" from olympia.amo.urlresolvers import Prefixer - _lang, application, path_fragment = Prefixer.split_path(path) + _lang, _application, path_fragment = Prefixer.split_path(path) return django_resolve(f'/{path_fragment}', urlconf) diff --git a/src/olympia/amo/tests/test_amo_utils.py b/src/olympia/amo/tests/test_amo_utils.py index e6fb9ad0ac92..f3ff85a22b23 100644 --- a/src/olympia/amo/tests/test_amo_utils.py +++ b/src/olympia/amo/tests/test_amo_utils.py @@ -26,7 +26,7 @@ def test_slug_validator(): assert slug_validator(u.lower()) is None - assert slug_validator('-'.join([u.lower(), u.lower()])) is None + assert slug_validator(f'{u.lower()}-{u.lower()}') is None pytest.raises(ValidationError, slug_validator, '234.add') pytest.raises(ValidationError, slug_validator, 'a a a') pytest.raises(ValidationError, slug_validator, 'tags/') @@ -38,8 +38,8 @@ def test_slug_validator(): ('xx x - "#$@ x', 'xx-x-x'), ('Bän...g (bang)', 'bäng-bang'), (u, u.lower()), - ('-'.join([u, u]), '-'.join([u, u]).lower()), - (' - '.join([u, u]), '-'.join([u, u]).lower()), + (f'{u}-{u}', f'{u}-{u}'.lower()), + (f'{u} - {u}', f'{u}-{u}'.lower()), (' a ', 'a'), ('tags/', 'tags'), ('holy_wars', 'holy_wars'), diff --git a/src/olympia/amo/tests/test_commands.py b/src/olympia/amo/tests/test_commands.py index a75e92020ddb..2b80a5e19f7c 100644 --- a/src/olympia/amo/tests/test_commands.py +++ b/src/olympia/amo/tests/test_commands.py @@ -783,25 +783,15 @@ def _test_full_run_typical_response(self): expected_below = ( 'The following locales are below threshold of 80% or completely ' 'absent in one of our projects in Pontoon:\n- ' - + '\n- '.join( - ( - 'Norwegian (Nynorsk) [nn-NO]', - 'Portuguese (Brazilian) [pt-BR]', - 'Romanian [ro]', - ) - ) + 'Norwegian (Nynorsk) [nn-NO]\n- ' + 'Portuguese (Brazilian) [pt-BR]\n- ' + 'Romanian [ro]' ) assert expected_below in mail.outbox[0].body expected_above = ( 'The following locales are above threshold and not yet enabled:\n- ' - + '\n- '.join( - ( - 'Bulgarian [bg]', - 'Danish [da]', - 'Indonesian [id]', - ) - ) + + 'Bulgarian [bg]\n- Danish [da]\n- Indonesian [id]' ) assert expected_above in mail.outbox[0].body diff --git a/src/olympia/amo/tests/test_helpers.py b/src/olympia/amo/tests/test_helpers.py index 08b586ded48e..75bb368829ff 100644 --- a/src/olympia/amo/tests/test_helpers.py +++ b/src/olympia/amo/tests/test_helpers.py @@ -318,7 +318,8 @@ def get_image_path(name): def get_uploaded_file(name): - data = open(get_image_path(name), mode='rb').read() + with open(get_image_path(name), mode='rb') as f: + data = f.read() return SimpleUploadedFile(name, data, content_type=mimetypes.guess_type(name)[0]) @@ -328,21 +329,20 @@ def get_addon_file(name): class TestAnimatedImages(TestCase): def test_animated_images(self): - img = ImageCheck(open(get_image_path('animated.png'), mode='rb')) - assert img.is_animated() - img = ImageCheck(open(get_image_path('non-animated.png'), mode='rb')) - assert not img.is_animated() - - img = ImageCheck(open(get_image_path('animated.gif'), mode='rb')) - assert img.is_animated() - img = ImageCheck(open(get_image_path('non-animated.gif'), mode='rb')) - assert not img.is_animated() + with open(get_image_path('animated.png'), mode='rb') as f: + assert ImageCheck(f).is_animated() + with open(get_image_path('non-animated.png'), mode='rb') as f: + assert not ImageCheck(f).is_animated() + with open(get_image_path('animated.gif'), mode='rb') as f: + assert ImageCheck(f).is_animated() + with open(get_image_path('non-animated.gif'), mode='rb') as f: + assert not ImageCheck(f).is_animated() def test_junk(self): - img = ImageCheck(open(__file__, 'rb')) - assert not img.is_image() - img = ImageCheck(open(get_image_path('non-animated.gif'), mode='rb')) - assert img.is_image() + with open(__file__, 'rb') as f: + assert not ImageCheck(f).is_image() + with open(get_image_path('non-animated.gif'), mode='rb') as f: + assert ImageCheck(f).is_image() def test_jinja_trans_monkeypatch(): diff --git a/src/olympia/amo/tests/test_monitor.py b/src/olympia/amo/tests/test_monitor.py index 1d1f861821e4..c33c3b01f26e 100644 --- a/src/olympia/amo/tests/test_monitor.py +++ b/src/olympia/amo/tests/test_monitor.py @@ -48,7 +48,7 @@ def test_libraries(self): @pytest.mark.requires_elasticsearch def test_elastic(self): - status, elastic_result = monitors.elastic() + status, _elastic_result = monitors.elastic() assert status == '' @patch('olympia.amo.monitors.get_es', side_effect=Exception('Connection error')) @@ -68,7 +68,7 @@ def test_elastic_status_red(self): @patch('os.path.exists') @patch('os.access') def test_path(self, mock_exists, mock_access): - status, path_result = monitors.path() + status, _path_result = monitors.path() assert status == '' @override_settings(TMP_PATH='foo') @@ -88,7 +88,7 @@ def test_rabbitmq(self, mock_connection): def test_signer(self): responses.add_passthru(settings.AUTOGRAPH_CONFIG['server_url']) - status, signer_result = monitors.signer() + status, _signer_result = monitors.signer() assert status == '' def test_database(self): diff --git a/src/olympia/amo/tests/test_settings.py b/src/olympia/amo/tests/test_settings.py index 4e0ba9303b25..45d0e5969e67 100644 --- a/src/olympia/amo/tests/test_settings.py +++ b/src/olympia/amo/tests/test_settings.py @@ -43,9 +43,10 @@ def test_sentry_data_scrubbing(): assert before_send assert before_breadcrumb assert sentry_client.options.get('send_default_pii') is True - event_raw = open( + with open( os.path.join(settings.ROOT, 'src/olympia/amo/fixtures/sentry_event.json') - ).read() + ) as f: + event_raw = f.read() event = json.loads(event_raw) assert '@bar.com' in event_raw assert '172.18.0.1' in event_raw diff --git a/src/olympia/amo/tests/test_url_prefix.py b/src/olympia/amo/tests/test_url_prefix.py index 35c4bf43e328..dcbcac21e128 100644 --- a/src/olympia/amo/tests/test_url_prefix.py +++ b/src/olympia/amo/tests/test_url_prefix.py @@ -237,16 +237,16 @@ def test_reverse(self): def test_resolve(self): # 'home' is now a frontend view - func, args, kwargs = resolve('/') + func, _args, _kwargs = resolve('/') assert func.__name__ == 'frontend_view' # a django view works too - func, args, kwargs = resolve('/developers/') + func, _args, _kwargs = resolve('/developers/') assert func.__name__ == 'index' # With a request with locale and app prefixes, it still works. Client().get('/') - func, args, kwargs = resolve('/en-US/firefox/addon/foo/statistics/') + func, _args, _kwargs = resolve('/en-US/firefox/addon/foo/statistics/') assert func.__name__ == 'stats_report' def test_script_name(self): diff --git a/src/olympia/amo/tests/test_utils.py b/src/olympia/amo/tests/test_utils.py index e8f62ea51fbd..ca72a8b416a8 100644 --- a/src/olympia/amo/tests/test_utils.py +++ b/src/olympia/amo/tests/test_utils.py @@ -220,9 +220,9 @@ def test_has_urls(): def test_walkfiles(): basedir = tempfile.mkdtemp(dir=settings.TMP_PATH) subdir = tempfile.mkdtemp(dir=basedir) - file1, file1path = tempfile.mkstemp(dir=basedir, suffix='_foo') - file2, file2path = tempfile.mkstemp(dir=subdir, suffix='_foo') - file3, file3path = tempfile.mkstemp(dir=subdir, suffix='_bar') + _file1, file1path = tempfile.mkstemp(dir=basedir, suffix='_foo') + _file2, file2path = tempfile.mkstemp(dir=subdir, suffix='_foo') + _file3, file3path = tempfile.mkstemp(dir=subdir, suffix='_bar') # Only files ending with _foo. assert list(walkfiles(basedir, suffix='_foo')) == [file1path, file2path] diff --git a/src/olympia/amo/urlresolvers.py b/src/olympia/amo/urlresolvers.py index d22b322324a7..8c770119ad0d 100644 --- a/src/olympia/amo/urlresolvers.py +++ b/src/olympia/amo/urlresolvers.py @@ -41,7 +41,7 @@ def split_path(path_): second, _, rest = first_rest.partition('/') first_lower = first.lower() - lang, dash, territory = first_lower.partition('-') + lang, dash, _territory = first_lower.partition('-') # First test shorter languages shortcuts. if not dash and first in settings.SHORTER_LANGUAGES: diff --git a/src/olympia/amo/utils.py b/src/olympia/amo/utils.py index e86759d32dfe..faebb8e083aa 100644 --- a/src/olympia/amo/utils.py +++ b/src/olympia/amo/utils.py @@ -232,10 +232,9 @@ def send_mail( if not from_email: from_email = settings.DEFAULT_FROM_EMAIL - if cc: - # If not str, assume it is already a list. - if isinstance(cc, str): - cc = [cc] + # If not str, assume it is already a list. + if cc and isinstance(cc, str): + cc = [cc] if not headers: headers = {} @@ -629,11 +628,10 @@ def parse_html(tree): tree.tail = tree.tail[1:] for child in tree: # Recurse down the tree. - if tree.tag in html_blocks: - # Strip new lines directly inside block level elements: remove - # the last new lines from the children's tails. - if child.tail: - child.tail = child.tail.rstrip('\n') + # Strip new lines directly inside block level elements: remove + # the last new lines from the children's tails. + if tree.tag in html_blocks and child.tail: + child.tail = child.tail.rstrip('\n') parse_html(child) return tree @@ -675,7 +673,7 @@ def pngcrush_image(src, **kw): # for our docker container). cmd = [settings.PNGCRUSH_BIN, '-q', '-reduce', '-ow', src, tmp_path] process = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - stdout, stderr = process.communicate() + _stdout, stderr = process.communicate() if process.returncode != 0: log.error(f'Error optimizing image: {src}; {stderr.strip()}') diff --git a/src/olympia/api/fields.py b/src/olympia/api/fields.py index 2c3493f39f64..a260e27bd128 100644 --- a/src/olympia/api/fields.py +++ b/src/olympia/api/fields.py @@ -379,7 +379,7 @@ def to_representation(self, obj): def to_internal_value(self, data): try: return self.queryset.get(pk=data) - except Exception: + except (ValueError, TypeError, self.queryset.model.DoesNotExist): try: return self.queryset.get(**{self.slug_field: data}) except ObjectDoesNotExist as exc: diff --git a/src/olympia/api/tests/test_exceptions.py b/src/olympia/api/tests/test_exceptions.py index cb7ee59265e9..70fb84a04a9a 100644 --- a/src/olympia/api/tests/test_exceptions.py +++ b/src/olympia/api/tests/test_exceptions.py @@ -104,7 +104,7 @@ def test_api_exception_handler_returns_response(self): with self.settings(DEBUG_PROPAGATE_EXCEPTIONS=False): try: raise APIException() - except Exception as exc: + except APIException as exc: response = exception_handler(exc, {}) assert isinstance(response, Response) assert response.status_code == 500 @@ -115,7 +115,7 @@ def test_exception_handler_returns_response_for_404(self): with self.settings(DEBUG_PROPAGATE_EXCEPTIONS=False): try: raise Http404() - except Exception as exc: + except Http404 as exc: response = exception_handler(exc, {}) assert isinstance(response, Response) assert response.status_code == 404 @@ -126,7 +126,7 @@ def test_exception_handler_returns_response_for_403(self): with self.settings(DEBUG_PROPAGATE_EXCEPTIONS=False): try: raise PermissionDenied() - except Exception as exc: + except PermissionDenied as exc: response = exception_handler(exc, {}) assert isinstance(response, Response) assert response.status_code == 403 @@ -138,8 +138,10 @@ def test_non_api_exception_handler_returns_response(self): with self.settings(DEBUG_PROPAGATE_EXCEPTIONS=False): try: - raise Exception() # noqa: TRY002 (tests generic handler) - except Exception as exc: + # Any non-DRF exception (here KeyError) should still produce a + # Response rather than propagating. + raise KeyError() + except KeyError as exc: response = exception_handler(exc, {}) assert isinstance(response, Response) assert response.status_code == 500 @@ -153,7 +155,7 @@ def test_api_exception_handler_with_propagation(self): ): try: raise APIException() - except Exception as exc: + except APIException as exc: exception_handler(exc, {}) def test_exception_handler_404_with_propagation(self): @@ -165,7 +167,7 @@ def test_exception_handler_404_with_propagation(self): ): try: raise Http404() - except Exception as exc: + except Http404 as exc: exception_handler(exc, {}) def test_exception_handler_403_with_propagation(self): @@ -177,7 +179,7 @@ def test_exception_handler_403_with_propagation(self): ): try: raise PermissionDenied() - except Exception as exc: + except PermissionDenied as exc: exception_handler(exc, {}) def test_non_api_exception_handler_with_propagation(self): @@ -191,5 +193,5 @@ def test_non_api_exception_handler_with_propagation(self): ): try: raise KeyError() - except Exception as exc: + except KeyError as exc: exception_handler(exc, {}) diff --git a/src/olympia/blocklist/tests/test_mlbf.py b/src/olympia/blocklist/tests/test_mlbf.py index 08d98e503f3b..d69cb83abd0e 100644 --- a/src/olympia/blocklist/tests/test_mlbf.py +++ b/src/olympia/blocklist/tests/test_mlbf.py @@ -881,9 +881,7 @@ def test_generate_empty_stash_when_all_items_in_filter( file_kw={'is_signed': True}, block_type=BlockType.BLOCKED ) hard_block = block.blockversion_set.first() - (hard_block_hash,) = MLBF.hash_filter_inputs( - [(block.guid, hard_block.version.version)] - ) + MLBF.hash_filter_inputs([(block.guid, hard_block.version.version)]) soft_blocks = [ self._block_version( block, self._version(addon), block_type=BlockType.SOFT_BLOCKED diff --git a/src/olympia/core/db/mysql/base.py b/src/olympia/core/db/mysql/base.py index 05e8cfe398d1..9e09c5e7cf0e 100644 --- a/src/olympia/core/db/mysql/base.py +++ b/src/olympia/core/db/mysql/base.py @@ -4,10 +4,12 @@ class DatabaseIntrospection(mysql_base.DatabaseIntrospection): def get_field_type(self, data_type, description): field_type = super().get_field_type(data_type, description) - if 'auto_increment' in description.extra: - if field_type == 'IntegerField': - if description.is_unsigned: - return 'PositiveAutoField' + if ( + 'auto_increment' in description.extra + and field_type == 'IntegerField' + and description.is_unsigned + ): + return 'PositiveAutoField' return field_type diff --git a/src/olympia/devhub/file_validation_annotations.py b/src/olympia/devhub/file_validation_annotations.py index 3acac075bb6c..071b20e6b2f3 100644 --- a/src/olympia/devhub/file_validation_annotations.py +++ b/src/olympia/devhub/file_validation_annotations.py @@ -77,16 +77,15 @@ def annotate_search_plugin_restriction(results, file_path, channel): def annotate_validation_results(*, results, parsed_data): """Annotate validation results with potential add-on restrictions like denied origins.""" - if waffle.switch_is_active('record-install-origins'): - if install_origins := parsed_data.get('install_origins'): - denied_origins = sorted( - DeniedInstallOrigin.find_denied_origins(install_origins) + if waffle.switch_is_active('record-install-origins') and ( + install_origins := parsed_data.get('install_origins') + ): + denied_origins = sorted( + DeniedInstallOrigin.find_denied_origins(install_origins) + ) + for origin in denied_origins: + insert_validation_message( + results, + message=str(DeniedInstallOrigin.ERROR_MESSAGE).format(origin=origin), ) - for origin in denied_origins: - insert_validation_message( - results, - message=str(DeniedInstallOrigin.ERROR_MESSAGE).format( - origin=origin - ), - ) return results diff --git a/src/olympia/devhub/forms.py b/src/olympia/devhub/forms.py index e696c90607c0..1d75d394c1e8 100644 --- a/src/olympia/devhub/forms.py +++ b/src/olympia/devhub/forms.py @@ -707,13 +707,12 @@ def save(self, *args, **kw): # database. license = super().save(*args, **kw) - if self.version: - if (changed and is_other) or license != self.version.license: - self.version.update(license=license) - if log: - ActivityLog.objects.create( - amo.LOG.CHANGE_LICENSE, license, self.version.addon - ) + if self.version and ((changed and is_other) or license != self.version.license): + self.version.update(license=license) + if log: + ActivityLog.objects.create( + amo.LOG.CHANGE_LICENSE, license, self.version.addon + ) return license @@ -1183,13 +1182,13 @@ def clean(self): if self.addon: self.check_for_existing_versions(parsed_data.get('version')) - if self.cleaned_data['upload'].channel == amo.CHANNEL_LISTED: - if error_message := ( - validate_version_number_is_gt_latest_signed_listed_version( - self.addon, parsed_data.get('version') - ) - ): - raise forms.ValidationError(error_message) + if self.cleaned_data['upload'].channel == amo.CHANNEL_LISTED and ( + error_message + := validate_version_number_is_gt_latest_signed_listed_version( + self.addon, parsed_data.get('version') + ) + ): + raise forms.ValidationError(error_message) self.cleaned_data['parsed_data'] = parsed_data return self.cleaned_data diff --git a/src/olympia/devhub/tasks.py b/src/olympia/devhub/tasks.py index c64c246384d7..5ed0481a14e4 100644 --- a/src/olympia/devhub/tasks.py +++ b/src/olympia/devhub/tasks.py @@ -487,9 +487,11 @@ def run_addons_linter(path, channel): if not os.path.exists(path): raise ValueError(f'Path "{path}" is not a file or directory or does not exist.') - stdout, stderr = (tempfile.TemporaryFile(), tempfile.TemporaryFile()) - - with statsd.timer('devhub.linter'): + with ( + tempfile.TemporaryFile() as stdout, + tempfile.TemporaryFile() as stderr, + statsd.timer('devhub.linter'), + ): process = subprocess.Popen( args, stdout=stdout, @@ -505,11 +507,6 @@ def run_addons_linter(path, channel): output, error = stdout.read(), stderr.read() - # Make sure we close all descriptors, otherwise they'll hang around - # and could cause a nasty exception. - stdout.close() - stderr.close() - if error: raise ValueError(error) diff --git a/src/olympia/devhub/tests/test_forms.py b/src/olympia/devhub/tests/test_forms.py index 4e15cc648ef8..b4541754570c 100644 --- a/src/olympia/devhub/tests/test_forms.py +++ b/src/olympia/devhub/tests/test_forms.py @@ -538,8 +538,11 @@ def test_preview_modified(self, update_mock): form = forms.PreviewForm( {'caption': 'test', 'upload_hash': upload_hash, 'position': 1} ) - with storage.open(os.path.join(self.dest, upload_hash), 'wb') as f: - shutil.copyfileobj(open(get_image_path(name), 'rb'), f) + with ( + storage.open(os.path.join(self.dest, upload_hash), 'wb') as f, + open(get_image_path(name), 'rb') as copy_src, + ): + shutil.copyfileobj(copy_src, f) assert form.is_valid() form.save(addon) assert update_mock.called @@ -571,8 +574,11 @@ def test_preview_transparency(self): form = forms.PreviewForm( {'caption': 'test', 'upload_hash': upload_hash, 'position': 1} ) - with storage.open(os.path.join(self.dest, upload_hash), 'wb') as f: - shutil.copyfileobj(open(get_image_path(name + '.png'), 'rb'), f) + with ( + storage.open(os.path.join(self.dest, upload_hash), 'wb') as f, + open(get_image_path(name + '.png'), 'rb') as copy_src, + ): + shutil.copyfileobj(copy_src, f) assert form.is_valid() form.save(addon) preview = addon.previews.all()[0] @@ -591,8 +597,11 @@ def test_preview_size(self, pngcrush_image_mock): form = forms.PreviewForm( {'caption': 'test', 'upload_hash': upload_hash, 'position': 1} ) - with storage.open(os.path.join(self.dest, upload_hash), 'wb') as f: - shutil.copyfileobj(open(get_image_path(name), 'rb'), f) + with ( + storage.open(os.path.join(self.dest, upload_hash), 'wb') as f, + open(get_image_path(name), 'rb') as copy_src, + ): + shutil.copyfileobj(copy_src, f) assert form.is_valid() form.save(addon) preview = addon.previews.all()[0] @@ -1298,8 +1307,11 @@ def test_icon_modified(self, update_mock): instance=self.addon, ) dest = os.path.join(self.icon_path, icon_upload_hash) - with storage.open(dest, 'wb') as f: - shutil.copyfileobj(open(get_image_path(name), 'rb'), f) + with ( + storage.open(dest, 'wb') as f, + open(get_image_path(name), 'rb') as copy_src, + ): + shutil.copyfileobj(copy_src, f) assert form.is_valid() form.save(addon=self.addon) assert update_mock.called diff --git a/src/olympia/devhub/tests/test_models.py b/src/olympia/devhub/tests/test_models.py index 0ba48e48f6b4..7c52cd5f1d1d 100644 --- a/src/olympia/devhub/tests/test_models.py +++ b/src/olympia/devhub/tests/test_models.py @@ -45,7 +45,7 @@ def test_file_delete_status_null(self): assert Addon.objects.get(pk=3615).status == amo.STATUS_NULL def test_file_delete_status_null_multiple(self): - version_two, file_two = self._extra_version_and_file(amo.STATUS_NULL) + _version_two, file_two = self._extra_version_and_file(amo.STATUS_NULL) self.file.delete() assert self.addon.status == amo.STATUS_APPROVED file_two.delete() diff --git a/src/olympia/devhub/tests/test_tasks.py b/src/olympia/devhub/tests/test_tasks.py index e8e0414d3f8e..a4bf5cbf0f3d 100644 --- a/src/olympia/devhub/tests/test_tasks.py +++ b/src/olympia/devhub/tests/test_tasks.py @@ -34,18 +34,33 @@ def test_recreate_previews(pngcrush_image_mock): addon = addon_factory() # Set up the preview so it has files in the right places. preview_no_original = Preview.objects.create(addon=addon) - with root_storage.open(preview_no_original.image_path, 'wb') as dest: - shutil.copyfileobj(open(get_image_path('preview_landscape.jpg'), 'rb'), dest) - with root_storage.open(preview_no_original.thumbnail_path, 'wb') as dest: - shutil.copyfileobj(open(get_image_path('mozilla.png'), 'rb'), dest) + with ( + root_storage.open(preview_no_original.image_path, 'wb') as dest, + open(get_image_path('preview_landscape.jpg'), 'rb') as copy_src, + ): + shutil.copyfileobj(copy_src, dest) + with ( + root_storage.open(preview_no_original.thumbnail_path, 'wb') as dest, + open(get_image_path('mozilla.png'), 'rb') as copy_src, + ): + shutil.copyfileobj(copy_src, dest) # And again but this time with an "original" image. preview_has_original = Preview.objects.create(addon=addon) - with root_storage.open(preview_has_original.image_path, 'wb') as dest: - shutil.copyfileobj(open(get_image_path('preview_landscape.jpg'), 'rb'), dest) - with root_storage.open(preview_has_original.thumbnail_path, 'wb') as dest: - shutil.copyfileobj(open(get_image_path('mozilla.png'), 'rb'), dest) - with root_storage.open(preview_has_original.original_path, 'wb') as dest: - shutil.copyfileobj(open(get_image_path('teamaddons.jpg'), 'rb'), dest) + with ( + root_storage.open(preview_has_original.image_path, 'wb') as dest, + open(get_image_path('preview_landscape.jpg'), 'rb') as copy_src, + ): + shutil.copyfileobj(copy_src, dest) + with ( + root_storage.open(preview_has_original.thumbnail_path, 'wb') as dest, + open(get_image_path('mozilla.png'), 'rb') as copy_src, + ): + shutil.copyfileobj(copy_src, dest) + with ( + root_storage.open(preview_has_original.original_path, 'wb') as dest, + open(get_image_path('teamaddons.jpg'), 'rb') as copy_src, + ): + shutil.copyfileobj(copy_src, dest) tasks.recreate_previews([addon.id]) diff --git a/src/olympia/devhub/tests/test_views.py b/src/olympia/devhub/tests/test_views.py index 9c3b0643bdd7..bad38f008af8 100644 --- a/src/olympia/devhub/tests/test_views.py +++ b/src/olympia/devhub/tests/test_views.py @@ -1394,11 +1394,12 @@ def setUp(self): self.xpi_path = self.file_path('webextension_no_id.xpi') def post(self, theme_specific=False, **kwargs): - data = { - 'upload': open(self.xpi_path, 'rb'), - 'theme_specific': 'True' if theme_specific else 'False', - } - return self.client.post(self.url, data, **kwargs) + with open(self.xpi_path, 'rb') as upload: + data = { + 'upload': upload, + 'theme_specific': 'True' if theme_specific else 'False', + } + return self.client.post(self.url, data, **kwargs) def test_submissions_disabled(self): self.create_flag('enable-submissions', note=':-(', everyone=False) diff --git a/src/olympia/devhub/tests/test_views_edit.py b/src/olympia/devhub/tests/test_views_edit.py index 0c6307827b9b..0a262d1f80aa 100644 --- a/src/olympia/devhub/tests/test_views_edit.py +++ b/src/olympia/devhub/tests/test_views_edit.py @@ -1072,11 +1072,9 @@ def test_edit_media_shows_proper_labels(self): def test_edit_media_uploadedicon(self): img = get_image_path('mozilla.png') - src_image = open(img, 'rb') - - data = {'upload_image': src_image} - - response = self.client.post(self.icon_upload, data) + with open(img, 'rb') as src_image: + data = {'upload_image': src_image} + response = self.client.post(self.icon_upload, data) response_json = json.loads(force_str(response.content)) addon = self.get_addon() @@ -1118,11 +1116,9 @@ def test_edit_media_icon_log(self): def test_edit_media_uploadedicon_noresize(self): img = 'static/img/notifications/error.png' - src_image = open(img, 'rb') - - data = {'upload_image': src_image} - - response = self.client.post(self.icon_upload, data) + with open(img, 'rb') as src_image: + data = {'upload_image': src_image} + response = self.client.post(self.icon_upload, data) response_json = json.loads(force_str(response.content)) addon = self.get_addon() @@ -1160,9 +1156,8 @@ def test_edit_media_uploadedicon_noresize(self): def check_image_type(self, url, msg): img = 'static/js/zamboni/devhub.js' - src_image = open(img, 'rb') - - res = self.client.post(url, {'upload_image': src_image}) + with open(img, 'rb') as src_image: + res = self.client.post(url, {'upload_image': src_image}) response_json = json.loads(force_str(res.content)) assert response_json['errors'][0] == msg @@ -1174,10 +1169,11 @@ def test_edit_media_screenshot_wrong_type(self): @override_settings(MAX_IMAGE_UPLOAD_SIZE=10 * 1024) def test_image_too_big(self): - response = self.client.post( - self.preview_upload, - {'upload_image': open(get_image_path('mozilla-small.png'), 'rb')}, - ) + with open(get_image_path('mozilla-small.png'), 'rb') as upload_image: + response = self.client.post( + self.preview_upload, + {'upload_image': upload_image}, + ) data = json.loads(force_str(response.content)) assert data == { 'errors': ['Please use images smaller than 0MB.'], @@ -1186,10 +1182,11 @@ def test_image_too_big(self): @override_settings(MAX_ICON_UPLOAD_SIZE=10 * 1024) def test_icon_too_big(self): - response = self.client.post( - self.icon_upload, - {'upload_image': open(get_image_path('mozilla-small.png'), 'rb')}, - ) + with open(get_image_path('mozilla-small.png'), 'rb') as upload_image: + response = self.client.post( + self.icon_upload, + {'upload_image': upload_image}, + ) data = json.loads(force_str(response.content)) assert data == { 'errors': ['Please use images smaller than 0MB.'], @@ -1245,9 +1242,8 @@ def test_preview_status_fails(self): assert not result['previews'] def check_image_animated(self, url, msg): - filehandle = open(get_image_path('animated.png'), 'rb') - - res = self.client.post(url, {'upload_image': filehandle}) + with open(get_image_path('animated.png'), 'rb') as filehandle: + res = self.client.post(url, {'upload_image': filehandle}) response_json = json.loads(force_str(res.content)) assert response_json['errors'][0] == msg @@ -1263,43 +1259,47 @@ def test_icon_dimensions_and_ratio(self): ratio_msg = 'Icon must be square (same width and height).' # mozilla-snall.png is too small and not square - response = self.client.post( - self.icon_upload, - {'upload_image': open(get_image_path('mozilla-small.png'), 'rb')}, - ) + with open(get_image_path('mozilla-small.png'), 'rb') as upload_image: + response = self.client.post( + self.icon_upload, + {'upload_image': upload_image}, + ) assert json.loads(force_str(response.content))['errors'] == [ size_msg, ratio_msg, ] # icon64.png is the right ratio, but only 64x64 - response = self.client.post( - self.icon_upload, {'upload_image': open(get_image_path('icon64.png'), 'rb')} - ) + with open(get_image_path('icon64.png'), 'rb') as upload_image: + response = self.client.post( + self.icon_upload, + {'upload_image': upload_image}, + ) assert json.loads(force_str(response.content))['errors'] == [size_msg] # mozilla.png is big enough but still not square - response = self.client.post( - self.icon_upload, - {'upload_image': open(get_image_path('mozilla.png'), 'rb')}, - ) + with open(get_image_path('mozilla.png'), 'rb') as upload_image: + response = self.client.post( + self.icon_upload, + {'upload_image': upload_image}, + ) assert json.loads(force_str(response.content))['errors'] == [ratio_msg] # and mozilla-sq is the right ratio and big enough - response = self.client.post( - self.icon_upload, - {'upload_image': open(get_image_path('mozilla-sq.png'), 'rb')}, - ) + with open(get_image_path('mozilla-sq.png'), 'rb') as upload_image: + response = self.client.post( + self.icon_upload, + {'upload_image': upload_image}, + ) assert json.loads(force_str(response.content))['errors'] == [] assert json.loads(force_str(response.content))['upload_hash'] def preview_add(self, amount=1, image_name='preview_4x3.jpg'): - src_image = open(get_image_path(image_name), 'rb') - - data = {'upload_image': src_image} - data_formset = self.formset_media(**data) - url = self.preview_upload - response = self.client.post(url, data_formset) + with open(get_image_path(image_name), 'rb') as src_image: + data = {'upload_image': src_image} + data_formset = self.formset_media(**data) + url = self.preview_upload + response = self.client.post(url, data_formset) details = json.loads(force_str(response.content)) upload_hash = details['upload_hash'] @@ -1332,34 +1332,38 @@ def test_preview_dimensions_and_ratio(self): ratio_msg = 'Image dimensions must be in the ratio 4:3.' # mozilla.png is too small and the wrong ratio now - response = self.client.post( - self.preview_upload, - {'upload_image': open(get_image_path('mozilla.png'), 'rb')}, - ) + with open(get_image_path('mozilla.png'), 'rb') as upload_image: + response = self.client.post( + self.preview_upload, + {'upload_image': upload_image}, + ) assert json.loads(force_str(response.content))['errors'] == [ size_msg, ratio_msg, ] # preview_landscape.jpg is the right ratio-ish, but too small - response = self.client.post( - self.preview_upload, - {'upload_image': open(get_image_path('preview_landscape.jpg'), 'rb')}, - ) + with open(get_image_path('preview_landscape.jpg'), 'rb') as upload_image: + response = self.client.post( + self.preview_upload, + {'upload_image': upload_image}, + ) assert json.loads(force_str(response.content))['errors'] == [size_msg] # teamaddons.jpg is big enough but still wrong ratio. - response = self.client.post( - self.preview_upload, - {'upload_image': open(get_image_path('teamaddons.jpg'), 'rb')}, - ) + with open(get_image_path('teamaddons.jpg'), 'rb') as upload_image: + response = self.client.post( + self.preview_upload, + {'upload_image': upload_image}, + ) assert json.loads(force_str(response.content))['errors'] == [ratio_msg] # and preview_4x3.jpg is the right ratio and big enough - response = self.client.post( - self.preview_upload, - {'upload_image': open(get_image_path('preview_4x3.jpg'), 'rb')}, - ) + with open(get_image_path('preview_4x3.jpg'), 'rb') as upload_image: + response = self.client.post( + self.preview_upload, + {'upload_image': upload_image}, + ) assert json.loads(force_str(response.content))['errors'] == [] assert json.loads(force_str(response.content))['upload_hash'] diff --git a/src/olympia/devhub/tests/test_views_submit.py b/src/olympia/devhub/tests/test_views_submit.py index 4468468cc0b9..1a814496db2e 100644 --- a/src/olympia/devhub/tests/test_views_submit.py +++ b/src/olympia/devhub/tests/test_views_submit.py @@ -546,10 +546,13 @@ def post( url = url or reverse(urlname, args=['listed' if listed else 'unlisted']) response = self.client.post(url, data, follow=True, **(extra_kwargs or {})) assert response.status_code == status_code - if not expect_errors: - # Show any unexpected form errors. - if response.context and 'new_addon_form' in response.context: - assert response.context['new_addon_form'].errors.as_text() == '' + # Show any unexpected form errors. + if ( + not expect_errors + and response.context + and 'new_addon_form' in response.context + ): + assert response.context['new_addon_form'].errors.as_text() == '' return response def test_redirect_back_to_agreement_if_restricted(self): @@ -989,10 +992,9 @@ def post(self, has_source, source, expect_errors=False, status_code=200): data['source'] = source response = self.client.post(self.url, data, follow=True) assert response.status_code == status_code - if not expect_errors: - # Show any unexpected form errors. - if response.context and 'source_form' in response.context: - assert response.context['source_form'].errors == {} + # Show any unexpected form errors. + if not expect_errors and response.context and 'source_form' in response.context: + assert response.context['source_form'].errors == {} return response @override_settings(FILE_UPLOAD_MAX_MEMORY_SIZE=1) diff --git a/src/olympia/devhub/tests/test_views_validation.py b/src/olympia/devhub/tests/test_views_validation.py index 20a91098b224..7aa86cf65803 100644 --- a/src/olympia/devhub/tests/test_views_validation.py +++ b/src/olympia/devhub/tests/test_views_validation.py @@ -117,10 +117,10 @@ def test_date_on_upload(self): assert doc('td').text() == 'Dec. 6, 2010' def test_upload_processed_validation_error(self): - addon_file = open(get_addon_file('invalid_webextension.xpi'), 'rb') - response = self.client.post( - reverse('devhub.upload'), {'name': 'addon.xpi', 'upload': addon_file} - ) + with open(get_addon_file('invalid_webextension.xpi'), 'rb') as addon_file: + response = self.client.post( + reverse('devhub.upload'), {'name': 'addon.xpi', 'upload': addon_file} + ) uuid = response.url.split('/')[-2] upload = FileUpload.objects.get(uuid=uuid) assert upload.processed_validation['errors'] == 1 diff --git a/src/olympia/devhub/tests/test_views_versions.py b/src/olympia/devhub/tests/test_views_versions.py index 816e8c9df8b9..f71a7588e543 100644 --- a/src/olympia/devhub/tests/test_views_versions.py +++ b/src/olympia/devhub/tests/test_views_versions.py @@ -841,7 +841,7 @@ def test_pending_activity_count(self): v1 = self.addon.current_version v1.update(created=self.days_ago(1)) v2, _ = self._extra_version_and_file(amo.STATUS_AWAITING_REVIEW) - v3, _ = self._extra_version_and_file(amo.STATUS_APPROVED) + self._extra_version_and_file(amo.STATUS_APPROVED) # Add some activity log messages ActivityLog.objects.create( amo.LOG.REVIEWER_REPLY_VERSION, v1.addon, v1, user=self.user diff --git a/src/olympia/files/models.py b/src/olympia/files/models.py index 7a0c4ad7c3e9..ef386d8639dd 100644 --- a/src/olympia/files/models.py +++ b/src/olympia/files/models.py @@ -453,9 +453,8 @@ def __str__(self): return str(self.uuid.hex) def save(self, *args, **kw): - if self.validation: - if self.load_validation()['errors'] == 0: - self.valid = True + if self.validation and self.load_validation()['errors'] == 0: + self.valid = True if not self.access_token: self.access_token = self.generate_access_token() super().save(*args, **kw) diff --git a/src/olympia/files/tasks.py b/src/olympia/files/tasks.py index b8bd7e052a6f..312a5cdc24ef 100644 --- a/src/olympia/files/tasks.py +++ b/src/olympia/files/tasks.py @@ -100,7 +100,7 @@ def repack_fileupload(results, upload_pk): log.info('Zip from upload %s extracted, repackaging', upload_pk) # We'll move the file to its final location below with move_stored_file(), # so don't let tempfile delete it. - file_ = tempfile.NamedTemporaryFile( + file_ = tempfile.NamedTemporaryFile( # noqa: SIM115 (moved after the block, so cannot use a context manager) dir=settings.TMP_PATH, suffix='.zip', delete=False ) shutil.make_archive(os.path.splitext(file_.name)[0], 'zip', tempdir) diff --git a/src/olympia/files/tests/test_tasks.py b/src/olympia/files/tests/test_tasks.py index 7057d8cb1271..a7444329da24 100644 --- a/src/olympia/files/tests/test_tasks.py +++ b/src/olympia/files/tests/test_tasks.py @@ -150,13 +150,9 @@ def test_normalize_manifest_json_with_bom(self): # Make sure it is valid JSON assert json.loads(manifest.read()) manifest.seek(0) - assert manifest.read().decode() == '\n'.join( - [ - '{', - ' "manifest_version": 2,', - ' "name": "..."', - '}', - ] + assert ( + manifest.read().decode() + == '{\n "manifest_version": 2,\n "name": "..."\n}' ) @override_switch('enable-manifest-normalization', active=True) @@ -215,13 +211,11 @@ def test_normalize_manifest_json(self): # Read the content again to make sure comments have been # removed with a string comparison. manifest.seek(0) - assert manifest.read().decode() == '\n'.join( - [ - '{', - ' "manifest_version": 2,', - ' "name": "My Extension",', - ' "version": "versionString",', - ' "description": "haupt_stra\\u00dfe"', - '}', - ] + assert manifest.read().decode() == ( + '{\n' + ' "manifest_version": 2,\n' + ' "name": "My Extension",\n' + ' "version": "versionString",\n' + ' "description": "haupt_stra\\u00dfe"\n' + '}' ) diff --git a/src/olympia/files/utils.py b/src/olympia/files/utils.py index b03d266b74fc..8696377351f2 100644 --- a/src/olympia/files/utils.py +++ b/src/olympia/files/utils.py @@ -572,18 +572,17 @@ def archive_member_validator(member, ignore_filename_errors=False): ) raise InvalidArchiveFile(msg) from exc - if not ignore_filename_errors: - if ( - '\\' in filename - or '../' in filename - or '..' == filename - or filename.startswith('/') - or any(unicodedata.category(c)[0] == 'C' for c in filename) - ): - log.warning('Extraction error, invalid file name: %s', filename) - # L10n: {0} is the name of the invalid file. - msg = gettext('Invalid file name in archive: {0}') - raise InvalidArchiveFile(msg.format(filename)) + if not ignore_filename_errors and ( + '\\' in filename + or '../' in filename + or '..' == filename + or filename.startswith('/') + or any(unicodedata.category(c)[0] == 'C' for c in filename) + ): + log.warning('Extraction error, invalid file name: %s', filename) + # L10n: {0} is the name of the invalid file. + msg = gettext('Invalid file name in archive: {0}') + raise InvalidArchiveFile(msg.format(filename)) if filesize > settings.FILE_UNZIP_SIZE_LIMIT: log.warning( diff --git a/src/olympia/hero/models.py b/src/olympia/hero/models.py index 2121cd7b471b..b5b8f5385fd8 100644 --- a/src/olympia/hero/models.py +++ b/src/olympia/hero/models.py @@ -280,11 +280,10 @@ def __str__(self): def clean(self): super().clean() - if not self.enabled: - if list(SecondaryHero.objects.filter(enabled=True)) == [self]: - raise ValidationError( - "You can't disable the only enabled secondary shelf." - ) + if not self.enabled and list(SecondaryHero.objects.filter(enabled=True)) == [ + self + ]: + raise ValidationError("You can't disable the only enabled secondary shelf.") class SecondaryHeroModule(CTACheckMixin, ModelBase): diff --git a/src/olympia/landfill/images.py b/src/olympia/landfill/images.py index fc22bdbd6db8..e104c5cc4db6 100644 --- a/src/olympia/landfill/images.py +++ b/src/olympia/landfill/images.py @@ -18,6 +18,6 @@ def generate_addon_preview(addon): color = random.choice(list(ImageColor.colormap.keys())) im = Image.new('RGB', (320, 480), color) p = Preview.objects.create(addon=addon, caption='Screenshot 1', position=1) - f = tempfile.NamedTemporaryFile(dir=settings.TMP_PATH) - im.save(f, 'png') - resize_preview(f.name, p.pk) + with tempfile.NamedTemporaryFile(dir=settings.TMP_PATH) as f: + im.save(f, 'png') + resize_preview(f.name, p.pk) diff --git a/src/olympia/landfill/serializers.py b/src/olympia/landfill/serializers.py index 3991df195b01..623777c12d56 100644 --- a/src/olympia/landfill/serializers.py +++ b/src/olympia/landfill/serializers.py @@ -4,6 +4,7 @@ from django.conf import settings from django.core.files.uploadedfile import SimpleUploadedFile +from django.db import IntegrityError from django.test.client import RequestFactory from django.utils.translation import activate @@ -66,7 +67,7 @@ def _create_addon_user(self): display_name='uitest', last_login_ip='127.0.0.1', ) - except Exception as e: + except IntegrityError as e: log.info( f'There was a problem creating the user: {e}.' ' Returning user from database' @@ -123,7 +124,7 @@ def create_named_addon_with_author(self, name, author=None): username=author, email=f'{author}@email.com' ) user.update(id=settings.TASK_USER_ID) - except Exception: # django.db.utils.IntegrityError + except IntegrityError: # If the user is already made, use that same user, # if not use created user addon = addon_factory( @@ -358,7 +359,8 @@ def create_installable_addon(self): # temporary one to avoid the files get moved somewhere else and # deleted from source tree with copy_file_to_temp(file_path) as temporary_path: - data = open(temporary_path, 'rb').read() + with open(temporary_path, 'rb') as temporary_file: + data = temporary_file.read() filedata = SimpleUploadedFile( file_to_upload, data, diff --git a/src/olympia/ratings/admin.py b/src/olympia/ratings/admin.py index 5ab7b31605e2..93b47df05353 100644 --- a/src/olympia/ratings/admin.py +++ b/src/olympia/ratings/admin.py @@ -103,7 +103,7 @@ def lookups(self, request, model_admin): if ( search_term := model_admin.get_search_query(request) ) and model_admin.ip_addresses_and_networks_from_query(search_term): - qs, search_use_distinct = model_admin.get_search_results( + qs, _search_use_distinct = model_admin.get_search_results( request, model_admin.get_queryset(request), search_term ) lookups_from_queryset = dict( diff --git a/src/olympia/ratings/tests/test_admin.py b/src/olympia/ratings/tests/test_admin.py index e94343eb721c..c17ec0f7e9a4 100644 --- a/src/olympia/ratings/tests/test_admin.py +++ b/src/olympia/ratings/tests/test_admin.py @@ -277,8 +277,9 @@ def test_search_by_ip(self): assert response.status_code == 200 doc = pq(response.content) assert len(doc('#result_list .field-id')) == 6 - assert doc('#result_list .field-known_ip_addresses').text().strip() == ' '.join( - ['4.8.15.16', '4.8.15.16', '125.5.6.7', '125.1.2.3\n4.8.15.16 125.1.1.2'] + assert ( + doc('#result_list .field-known_ip_addresses').text().strip() + == '4.8.15.16 4.8.15.16 125.5.6.7 125.1.2.3\n4.8.15.16 125.1.1.2' ) def test_can_delete_on_changelist_while_sorting_by_ip(self): diff --git a/src/olympia/reviewers/forms.py b/src/olympia/reviewers/forms.py index a5e5634f101b..62833307fefe 100644 --- a/src/olympia/reviewers/forms.py +++ b/src/olympia/reviewers/forms.py @@ -835,11 +835,13 @@ def clean(self): return self.cleaned_data def clean_delayed_rejection_date(self): - if self.cleaned_data.get('delayed_rejection_date'): - if self.cleaned_data['delayed_rejection_date'] < self.min_rejection_date: - raise ValidationError( - 'Delayed rejection date should be at least one day in the future' - ) + if ( + self.cleaned_data.get('delayed_rejection_date') + and self.cleaned_data['delayed_rejection_date'] < self.min_rejection_date + ): + raise ValidationError( + 'Delayed rejection date should be at least one day in the future' + ) return self.cleaned_data.get('delayed_rejection_date') def clean_version_pk(self): diff --git a/src/olympia/reviewers/management/commands/auto_approve.py b/src/olympia/reviewers/management/commands/auto_approve.py index 285c8697990e..76bc52fd5906 100644 --- a/src/olympia/reviewers/management/commands/auto_approve.py +++ b/src/olympia/reviewers/management/commands/auto_approve.py @@ -120,18 +120,18 @@ def process(self, version_id): summary = AutoApprovalSummary.objects.filter(version=version).first() - if waffle.switch_is_active('enable-narc'): - # We want to execute `run_narc()` only once. - if not summary: - # NARC scanner rules depend on the Add-on and can't be - # run reliably at validation as it might not be - # attached to the upload at that point. - # This needs to be run before run_actions() and before - # auto-approval is attempted, and has to be triggered - # synchronously (no .delay()). In this case we pass - # run_actions_on_match=False since we're going to call - # ScannerResult.run_actions() below. - run_narc_on_version(version.pk, run_actions_on_match=False) + # We want to execute `run_narc()` only once (when there is no + # summary yet). + # NARC scanner rules depend on the Add-on and can't be + # run reliably at validation as it might not be + # attached to the upload at that point. + # This needs to be run before run_actions() and before + # auto-approval is attempted, and has to be triggered + # synchronously (no .delay()). In this case we pass + # run_actions_on_match=False since we're going to call + # ScannerResult.run_actions() below. + if waffle.switch_is_active('enable-narc') and not summary: + run_narc_on_version(version.pk, run_actions_on_match=False) scanner_actions_executed = False # NULL means the summary predates this field, back when diff --git a/src/olympia/reviewers/tests/test_decorators.py b/src/olympia/reviewers/tests/test_decorators.py index c7f77a0a22f1..a1d65747d7f5 100644 --- a/src/olympia/reviewers/tests/test_decorators.py +++ b/src/olympia/reviewers/tests/test_decorators.py @@ -67,5 +67,5 @@ def test_slug_isdigit(self): addon.update(slug=str(addon.id)) res = self.view(self.request, addon.slug) assert res == mock.sentinel.OK - request, addon_ = self.func.call_args[0] + _request, addon_ = self.func.call_args[0] assert addon_ == addon diff --git a/src/olympia/reviewers/tests/test_forms.py b/src/olympia/reviewers/tests/test_forms.py index ff0de399404b..066c68ff4b24 100644 --- a/src/olympia/reviewers/tests/test_forms.py +++ b/src/olympia/reviewers/tests/test_forms.py @@ -1619,18 +1619,13 @@ def test_cinder_jobs_to_resolve_choices(self): assert label_forward.attr['class'] == 'data-toggle-hide' assert label_forward.attr['data-value'] == 'appeal_deny appeal_override' assert label_rep_appeal.attr['class'] == 'data-toggle-hide appeal' - assert label_rep_appeal.attr['data-value'] == ' '.join( - ('appeal_override', 'resolve_reports_job') + assert ( + label_rep_appeal.attr['data-value'] == 'appeal_override resolve_reports_job' ) assert label_dev_appeal.attr['class'] == 'data-toggle-hide appeal' - assert label_dev_appeal.attr['data-value'] == ' '.join( - ( - 'review_with_policy_approve', - 'review_with_policy', - 'reject', - 'reject_multiple_versions', - 'resolve_reports_job', - ) + assert label_dev_appeal.attr['data-value'] == ( + 'review_with_policy_approve review_with_policy reject ' + 'reject_multiple_versions resolve_reports_job' ) assert label_two_reports.attr['class'] == 'data-toggle-hide' assert label_two_reports.attr['data-value'] == 'appeal_deny appeal_override' diff --git a/src/olympia/reviewers/tests/test_models.py b/src/olympia/reviewers/tests/test_models.py index 57cb1ab8f1f2..ba7e1f7bc43e 100644 --- a/src/olympia/reviewers/tests/test_models.py +++ b/src/olympia/reviewers/tests/test_models.py @@ -1597,7 +1597,7 @@ def test_create_summary_for_version_no_mocks(self): } ) ) - summary, info = AutoApprovalSummary.create_summary_for_version( + summary, _info = AutoApprovalSummary.create_summary_for_version( self.version, ) assert summary.verdict == amo.AUTO_APPROVED diff --git a/src/olympia/reviewers/views.py b/src/olympia/reviewers/views.py index dfc459fbcb4f..8713694a9dc3 100644 --- a/src/olympia/reviewers/views.py +++ b/src/olympia/reviewers/views.py @@ -553,9 +553,12 @@ def review(request, addon, channel=None): # cached validation, since reviewers will almost certainly need to access # them. But only if we're not running in eager mode, since that could mean # blocking page load for several minutes. - if version and not getattr(settings, 'CELERY_TASK_ALWAYS_EAGER', False): - if not version.file.has_been_validated: - devhub_tasks.validate(version.file) + if ( + version + and not getattr(settings, 'CELERY_TASK_ALWAYS_EAGER', False) + and not version.file.has_been_validated + ): + devhub_tasks.validate(version.file) actions = form.helper.actions.items() @@ -945,7 +948,7 @@ def abuse_reports(request, addon): @reviewer_addon_view_factory def whiteboard(request, addon, channel): channel_as_text = channel - channel, content_review = determine_channel(channel) + channel, _content_review = determine_channel(channel) unlisted_only = ( channel == amo.CHANNEL_UNLISTED @@ -980,7 +983,7 @@ def policy_viewer(request, addon, eula_or_privacy, page_title, long_title): if not eula_or_privacy: raise http.Http404 channel_text = request.GET.get('channel') - channel, content_review = determine_channel(channel_text) + _channel, _content_review = determine_channel(channel_text) review_url = reverse( 'reviewers.review', diff --git a/src/olympia/signing/views.py b/src/olympia/signing/views.py index b11a29dd6def..54013d148a80 100644 --- a/src/olympia/signing/views.py +++ b/src/olympia/signing/views.py @@ -184,14 +184,13 @@ def handle_upload(self, request, addon, version_string, guid=None): elif not guid and package_guid: guid = package_guid - if guid: - # If we did get a guid, regardless of its source, validate it now - # before creating anything. - if not amo.ADDON_GUID_PATTERN.match(guid): - raise forms.ValidationError( - gettext('Invalid Add-on ID in URL or package'), - status.HTTP_400_BAD_REQUEST, - ) + # If we did get a guid, regardless of its source, validate it now + # before creating anything. + if guid and not amo.ADDON_GUID_PATTERN.match(guid): + raise forms.ValidationError( + gettext('Invalid Add-on ID in URL or package'), + status.HTTP_400_BAD_REQUEST, + ) # channel will be ignored for new addons. if addon is None: diff --git a/src/olympia/stats/tests/test_views.py b/src/olympia/stats/tests/test_views.py index 0f7b80555a0f..1d0a82bcbccb 100644 --- a/src/olympia/stats/tests/test_views.py +++ b/src/olympia/stats/tests/test_views.py @@ -1262,4 +1262,4 @@ def test_handles_null_keys(self): fields=fields, ) - assert '\r\n'.join([',a', '1,2', '0,4']) in force_str(response.content) + assert ',a\r\n1,2\r\n0,4' in force_str(response.content) diff --git a/src/olympia/stats/utils.py b/src/olympia/stats/utils.py index c5f0591fd65a..82bcf1f1afe4 100644 --- a/src/olympia/stats/utils.py +++ b/src/olympia/stats/utils.py @@ -35,7 +35,7 @@ def make_fully_qualified_view_name(view): if waffle.switch_is_active('2026-amo-stats'): return settings.BIGQUERY_AMO_STATS_PREFIX + view - return '.'.join([settings.BIGQUERY_PROJECT, settings.BIGQUERY_AMO_DATASET, view]) + return f'{settings.BIGQUERY_PROJECT}.{settings.BIGQUERY_AMO_DATASET}.{view}' def get_amo_stats_dau_view_name(): diff --git a/src/olympia/stats/views.py b/src/olympia/stats/views.py index 88fff50d6ad7..4f2115a0c04a 100644 --- a/src/olympia/stats/views.py +++ b/src/olympia/stats/views.py @@ -278,7 +278,7 @@ def flatten_applications(series): # str() to decode the gettext proxy. appname = str(app.pretty) for ver, count in versions.items(): - key = ' '.join([appname, ver]) + key = f'{appname} {ver}' new[key] = count row['data'] = new yield row diff --git a/src/olympia/translations/tests/test_fields.py b/src/olympia/translations/tests/test_fields.py index bbee934d8c97..b18391194a42 100644 --- a/src/olympia/translations/tests/test_fields.py +++ b/src/olympia/translations/tests/test_fields.py @@ -41,7 +41,7 @@ def test_translated_field_supports_migration(): def test_user_foreign_key_field_deconstruct(): field = TranslatedField(require_locale=False) - name, path, args, kwargs = field.deconstruct() + _name, _path, _args, kwargs = field.deconstruct() new_field_instance = TranslatedField(require_locale=False) assert kwargs['require_locale'] == new_field_instance.require_locale diff --git a/src/olympia/users/tests/test_admin.py b/src/olympia/users/tests/test_admin.py index ba4e4a1a927f..3c895d9ff647 100644 --- a/src/olympia/users/tests/test_admin.py +++ b/src/olympia/users/tests/test_admin.py @@ -274,12 +274,9 @@ def test_search_for_single_ip_multiple_results_for_different_reasons(self): assert response.status_code == 200 doc = pq(response.content) # Make sure it's the right users. - assert doc('.field-email').text() == ' '.join( - [ - extra_extra_user.email, - extra_user.email, - self.user.email, - ] + assert ( + doc('.field-email').text() + == f'{extra_extra_user.email} {extra_user.email} {self.user.email}' ) # Make sure IPs displayed, and has the right values. The first and # third users only have the one we're looking for, the one in the @@ -410,12 +407,9 @@ def test_search_for_multiple_ips_with_deduplication(self): doc = pq(response.content) assert len(doc('#result_list tbody tr')) == 3 # Make sure it's the right users. - assert doc('.field-email').text() == ' '.join( - [ - extra_extra_user.email, - extra_user.email, - self.user.email, - ] + assert ( + doc('.field-email').text() + == f'{extra_extra_user.email} {extra_user.email} {self.user.email}' ) # Make sure each IP only appears once for each row. assert doc('.field-known_ip_addresses').text() == ( @@ -455,12 +449,9 @@ def test_search_for_ip_network_with_deduplication(self): doc = pq(response.content) assert len(doc('#result_list tbody tr')) == 3 # Make sure it's the right users. - assert doc('.field-email').text() == ' '.join( - [ - extra_extra_user.email, - extra_user.email, - self.user.email, - ] + assert ( + doc('.field-email').text() + == f'{extra_extra_user.email} {extra_user.email} {self.user.email}' ) # Make sure each IP only appears once for each row. assert doc('.field-known_ip_addresses').text() == ( diff --git a/src/olympia/users/tests/test_tasks.py b/src/olympia/users/tests/test_tasks.py index c89342a6d8c5..a461669bcfa1 100644 --- a/src/olympia/users/tests/test_tasks.py +++ b/src/olympia/users/tests/test_tasks.py @@ -162,10 +162,10 @@ def test_delete_photo_not_banned_no_backup(self): def test_resize_photo(): somepic = get_image_path('sunbird-small.png') - src = tempfile.NamedTemporaryFile( + src = tempfile.NamedTemporaryFile( # noqa: SIM115 (temp file used across the test (delete=False)) mode='r+b', suffix='.png', delete=False, dir=settings.TMP_PATH ) - dest = tempfile.NamedTemporaryFile(mode='r+b', suffix='.png', dir=settings.TMP_PATH) + dest = tempfile.NamedTemporaryFile(mode='r+b', suffix='.png', dir=settings.TMP_PATH) # noqa: SIM115 (temp file used across the test) shutil.copyfile(somepic, src.name) @@ -181,7 +181,7 @@ def test_resize_photo(): def test_resize_photo_poorly(): """If we attempt to set the src/dst, we do nothing.""" somepic = get_image_path('mozilla.png') - src = tempfile.NamedTemporaryFile( + src = tempfile.NamedTemporaryFile( # noqa: SIM115 (temp file used across the test (delete=False)) mode='r+b', suffix='.png', delete=False, dir=settings.TMP_PATH ) shutil.copyfile(somepic, src.name) @@ -341,10 +341,8 @@ def test_socket_labs_returns_404(self): status=404, ) - try: - send_suppressed_email_confirmation.apply([verification.id]) - except Exception as err: - pytest.fail(f'Unexpected exception: {err}') + # Should not raise. + send_suppressed_email_confirmation.apply([verification.id]) def test_socket_labs_returns_5xx(self): verification = SuppressedEmailVerification.objects.create( diff --git a/tests/make/test_health_check.py b/tests/make/test_health_check.py index fa07da50b05c..cbd8e6a86bb8 100644 --- a/tests/make/test_health_check.py +++ b/tests/make/test_health_check.py @@ -117,7 +117,7 @@ def test_failing_monitors(self): ('__version__', 200, {'version': '1.0.0'}), ] ): - results, has_failures = main('container') + _results, has_failures = main('container') self.assertTrue(has_failures) def test_request_retries(self): From 8d63fab3300c731e282bb581babca5a7e0f0c8f0 Mon Sep 17 00:00:00 2001 From: eviljeff Date: Mon, 21 Sep 2026 11:32:43 +0100 Subject: [PATCH 2/2] review fixes --- src/olympia/reviewers/tests/test_decorators.py | 2 +- src/olympia/reviewers/views.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/olympia/reviewers/tests/test_decorators.py b/src/olympia/reviewers/tests/test_decorators.py index a1d65747d7f5..fda7ee139c0b 100644 --- a/src/olympia/reviewers/tests/test_decorators.py +++ b/src/olympia/reviewers/tests/test_decorators.py @@ -67,5 +67,5 @@ def test_slug_isdigit(self): addon.update(slug=str(addon.id)) res = self.view(self.request, addon.slug) assert res == mock.sentinel.OK - _request, addon_ = self.func.call_args[0] + addon_ = self.func.call_args[0][1] assert addon_ == addon diff --git a/src/olympia/reviewers/views.py b/src/olympia/reviewers/views.py index 8713694a9dc3..fb10a5ddec49 100644 --- a/src/olympia/reviewers/views.py +++ b/src/olympia/reviewers/views.py @@ -983,7 +983,6 @@ def policy_viewer(request, addon, eula_or_privacy, page_title, long_title): if not eula_or_privacy: raise http.Http404 channel_text = request.GET.get('channel') - _channel, _content_review = determine_channel(channel_text) review_url = reverse( 'reviewers.review',