Skip to content

Commit

Permalink
Have migration test actually fail if value isn't missing
Browse files Browse the repository at this point in the history
flake8-bugbear noticed that the second loop was checking the wrong
value. I noticed that the test wouldn't actually fail if the expected
exception wasn't raised, so use pytest.raises, which will correctly fail
if it doesn't work as expected.
  • Loading branch information
legoktm committed Jan 15, 2025
1 parent 900942f commit 25b4c8c
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 13 deletions.
2 changes: 1 addition & 1 deletion securedrop/journalist_app/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def manage_config() -> Union[str, werkzeug.Response]:
)
return redirect(url_for("admin.manage_config") + "#config-logoimage")
else:
for errors in list(logo_form.errors.values()):
for errors in logo_form.errors.values():
for error in errors:
flash(error, "logo-error")
return render_template(
Expand Down
5 changes: 2 additions & 3 deletions securedrop/pretty_bad_protocol/_meta.py
Original file line number Diff line number Diff line change
Expand Up @@ -296,9 +296,8 @@ def update_path(environment, path): # type: ignore[no-untyped-def]
:param list path: A list of strings to update the PATH with.
"""
log.debug("Updating system path...")
# This assignment is mostly to reset the monkey patch, but it doesn't
# actually reset the environment. Leaving it alone since it works as-is
# despite being wrong.
# This assignment doesn't reset the environment, but it does reset the monkey-patch
# as intended. Leaving as-is from upstream.
os.environ = environment # noqa: B003
new_path = ":".join([p for p in path])
if "PATH" in os.environ:
Expand Down
13 changes: 4 additions & 9 deletions securedrop/tests/migrations/migration_b58139cfdc8c.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from os import path
from unittest import mock

import pytest
from db import db
from journalist_app import create_app
from sqlalchemy import text
Expand Down Expand Up @@ -192,20 +193,14 @@ def check_downgrade(self):
sql = "SELECT * FROM submissions"
submissions = db.engine.execute(text(sql)).fetchall()
for submission in submissions:
try:
# this should produce an exception since the column is gone
with pytest.raises(NoSuchColumnError):
submission["checksum"]
except NoSuchColumnError:
pass

sql = "SELECT * FROM replies"
replies = db.engine.execute(text(sql)).fetchall()
for reply in replies:
try:
# this should produce an exception since the column is gone
submission["checksum"]
except NoSuchColumnError:
pass
with pytest.raises(NoSuchColumnError):
reply["checksum"]

def add_revoked_token(self):
params = {
Expand Down

0 comments on commit 25b4c8c

Please sign in to comment.