Do extra code style checks with flake8-bugbear

Flake8 Bugbear checks for some extra things that aren’t code style
errors, but are likely to introduce bugs or unexpected behaviour. A
good example is having mutable default function arguments, which get
shared between every call to the function and therefore mutating a value
in one place can unexpectedly cause it to change in another.

This commit enables all the extra warnings provided by Flake8 Bugbear,
except for:
- the line length one (because we already lint for that separately)
- B903 Data class should either be immutable or use `__slots__` because
  this seems to false-positive on some of our custom exceptions
- B902 Invalid first argument 'cls' used for instance method because
  some SQLAlchemy decorators (eg `declared_attr`) make things that
  aren’t formally class methods take a class not an instance as their
  first argument

It disables:
- _B306: BaseException.message is removed in Python 3_ because I think
  our exceptions have a custom structure that means the `.message`
  attribute is still present

Matches the work done in other repos:
- https://github.com/alphagov/notifications-admin/pull/3172/files
This commit is contained in:
Chris Hill-Scott
2020-12-22 15:46:31 +00:00
parent 7152ac7cba
commit 3b0b96834d
29 changed files with 88 additions and 68 deletions

View File

@@ -511,7 +511,7 @@ def test_get_notification_for_job(sample_notification):
def test_get_all_notifications_for_job(sample_job):
for i in range(0, 5):
for _ in range(0, 5):
try:
create_notification(template=sample_job.template, job=sample_job)
except IntegrityError:
@@ -546,7 +546,7 @@ def test_dao_get_notification_count_for_job_id(notify_db_session):
service = create_service()
template = create_template(service)
job = create_job(template, notification_count=3)
for i in range(3):
for _ in range(3):
create_notification(job=job)
create_notification(template)

View File

@@ -138,13 +138,13 @@ def test_should_not_delete_invitations_less_than_two_days_old(
assert InvitedUser.query.first().email_address == "valid@2.com"
def make_invitation(user, service, age=timedelta(hours=0), email_address="test@test.com"):
def make_invitation(user, service, age=None, email_address="test@test.com"):
verify_code = InvitedUser(
email_address=email_address,
from_user=user,
service=service,
status='pending',
created_at=datetime.utcnow() - age,
created_at=datetime.utcnow() - (age or timedelta(hours=0)),
permissions='manage_settings',
folder_permissions=[str(uuid.uuid4())]
)

View File

@@ -73,7 +73,7 @@ def test_add_reply_to_email_address_for_service_creates_another_email_for_servic
elif x.email_address == 'second@address.com':
assert not x.is_default
else:
assert False
raise AssertionError()
def test_add_reply_to_email_address_new_reply_to_is_default_existing_reply_to_is_not(notify_db_session):
@@ -89,7 +89,7 @@ def test_add_reply_to_email_address_new_reply_to_is_default_existing_reply_to_is
elif x.email_address == 'second@address.com':
assert x.is_default
else:
assert False
raise AssertionError()
def test_add_reply_to_email_address_can_add_a_third_reply_to_address(sample_service):
@@ -112,7 +112,7 @@ def test_add_reply_to_email_address_can_add_a_third_reply_to_address(sample_serv
elif x.email_address == 'third@address.com':
assert not x.is_default
else:
assert False
raise AssertionError()
def test_add_reply_to_email_address_ensures_first_reply_to_is_default(sample_service):
@@ -161,7 +161,7 @@ def test_update_reply_to_email_address_set_updated_to_default(sample_service):
elif x.email_address == 'first@address.com':
assert not x.is_default
else:
assert False
raise AssertionError()
def test_update_reply_to_email_address_raises_exception_if_single_reply_to_and_setting_default_to_false(sample_service):

View File

@@ -512,6 +512,8 @@ def test_removing_all_permission_returns_service_with_no_permissions(notify_db_s
dao_remove_service_permission(service_id=service.id, permission=EMAIL_TYPE)
dao_remove_service_permission(service_id=service.id, permission=LETTER_TYPE)
dao_remove_service_permission(service_id=service.id, permission=INTERNATIONAL_SMS_TYPE)
dao_remove_service_permission(service_id=service.id, permission=UPLOAD_LETTERS)
dao_remove_service_permission(service_id=service.id, permission=INTERNATIONAL_LETTERS)
service = dao_fetch_service_by_id(service.id)
assert len(service.permissions) == 0
@@ -1089,22 +1091,22 @@ def test_dao_find_services_sending_to_tv_numbers(notify_db_session, fake_uuid):
for service in services:
template = create_template(service)
for x in range(0, 5):
for _ in range(0, 5):
create_notification(template, normalised_to=tv_number, status="permanent-failure")
service_6 = create_service(service_name="Service 6") # notifications too old are excluded
with freeze_time("2019-11-30 15:00:00.000000"):
template_6 = create_template(service_6)
for x in range(0, 5):
for _ in range(0, 5):
create_notification(template_6, normalised_to=tv_number, status="permanent-failure")
service_2 = create_service(service_name="Service 2") # below threshold is excluded
template_2 = create_template(service_2)
create_notification(template_2, normalised_to=tv_number, status="permanent-failure")
for x in range(0, 5):
for _ in range(0, 5):
# test key type is excluded
create_notification(template_2, normalised_to=tv_number, status="permanent-failure", key_type='test')
for x in range(0, 5):
for _ in range(0, 5):
# normal numbers are not counted by the query
create_notification(template_2, normalised_to=normal_number, status="delivered")
create_notification(template_2, normalised_to=normal_number_resembling_tv_number, status="delivered")
@@ -1126,7 +1128,7 @@ def test_dao_find_services_with_high_failure_rates(notify_db_session, fake_uuid)
for service in services:
template = create_template(service)
for x in range(0, 3):
for _ in range(0, 3):
create_notification(template, status="permanent-failure")
create_notification(template, status="delivered")
create_notification(template, status="sending")
@@ -1135,12 +1137,12 @@ def test_dao_find_services_with_high_failure_rates(notify_db_session, fake_uuid)
service_6 = create_service(service_name="Service 6")
with freeze_time("2019-11-30 15:00:00.000000"):
template_6 = create_template(service_6)
for x in range(0, 4):
for _ in range(0, 4):
create_notification(template_6, status="permanent-failure") # notifications too old are excluded
service_2 = create_service(service_name="Service 2")
template_2 = create_template(service_2)
for x in range(0, 4):
for _ in range(0, 4):
create_notification(template_2, status="permanent-failure", key_type='test') # test key type is excluded
create_notification(template_2, status="permanent-failure") # below threshold is excluded

View File

@@ -320,28 +320,28 @@ def test_get_uploaded_letters_by_print_date(sample_template):
letter_template = create_uploaded_template(sample_template.service)
# Letters for the previous days run
for i in range(3):
for _ in range(3):
create_uploaded_letter(
letter_template, sample_template.service, status='delivered',
created_at=datetime.utcnow().replace(day=1, hour=17, minute=29, second=59)
)
# Letters from yesterday that rolled into todays run
for i in range(30):
for _ in range(30):
create_uploaded_letter(
letter_template, sample_template.service, status='delivered',
created_at=datetime.utcnow().replace(day=1, hour=17, minute=30, second=0)
)
# Letters that just made todays run
for i in range(30):
for _ in range(30):
create_uploaded_letter(
letter_template, sample_template.service, status='delivered',
created_at=datetime.utcnow().replace(hour=17, minute=29, second=59)
)
# Letters that just missed todays run
for i in range(3):
for _ in range(3):
create_uploaded_letter(
letter_template, sample_template.service, status='delivered',
created_at=datetime.utcnow().replace(hour=17, minute=30, second=0)

View File

@@ -124,12 +124,12 @@ def test_should_not_delete_verification_codes_less_than_one_day_old(sample_user)
assert VerifyCode.query.one()._code == "12345"
def make_verify_code(user, age=timedelta(hours=0), expiry_age=timedelta(0), code="12335", code_used=False):
def make_verify_code(user, age=None, expiry_age=None, code="12335", code_used=False):
verify_code = VerifyCode(
code_type='sms',
_code=code,
created_at=datetime.utcnow() - age,
expiry_datetime=datetime.utcnow() - expiry_age,
created_at=datetime.utcnow() - (age or timedelta(hours=0)),
expiry_datetime=datetime.utcnow() - (expiry_age or timedelta(0)),
user=user,
code_used=code_used
)