Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix database fixture and password unit test #1120

Merged
merged 3 commits into from
Aug 2, 2024
Merged

Fix database fixture and password unit test #1120

merged 3 commits into from
Aug 2, 2024

Conversation

sea-kelp
Copy link
Collaborator

@sea-kelp sea-kelp commented Aug 1, 2024

Description of Changes

  1. Simplify database fixtures
  2. Fix password unit test - This test currently doesn't fail if user.password returns a value. Using pytest.raises fails the test if AttributeError is not raised.

Notes for Deployment

None!

Screenshots (if appropriate)

N/A

Tests and Linting

  • This branch is up-to-date with the develop branch.
  • pytest passes on my local development environment.
  • pre-commit passes on my local development environment.

@sea-kelp sea-kelp changed the title Chore/minor cleanup Fix database fixture and password unit test Aug 1, 2024
sea-kelp
sea-kelp commented Aug 1, 2024
View reviewed changes
OpenOversight/tests/conftest.py
Comment on lines +321 to +323
# If an error was raised during test, tx is already rolled back
if tx.is_active:
tx.rollback()
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were getting the warning SAWarning: transaction already deassociated from connection from 2 unit tests which intentionally violated database constraints, causing the transaction to be automatically rolled back when the commit failed. Then, when the fixture was being cleaned up, it was trying to roll back the transaction a second time.

Instead, this change checks whether the transaction is still active and rolls back if needed.

michplunkett
michplunkett approved these changes Aug 1, 2024
View reviewed changes
Copy link
Collaborator

@michplunkett michplunkett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

OpenOversight/tests/conftest.py
session.commit()
connection.close()
session.remove()
with _db.engine.begin() as connection:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much more succinct. 🤌🏻

OpenOversight/tests/test_models.py
@@ -173,11 +173,8 @@ def test_salary_repr(mockdata):
def test_password_not_printed(mockdata):
"""Validate that password fields cannot be directly accessed."""
user = User(password="bacon")
try:
with raises(AttributeError):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mixed feelings on this one, but it's more of a style thing than anything else.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you change this line to return "hello world", the previous test logic passes while this test now fails

OpenOversight/OpenOversight/app/models/database.py

Line 796 in 43f3ac9

raise AttributeError("password is not a readable attribute")

@michplunkett michplunkett merged commit d7260c6 into develop Aug 2, 2024
3 checks passed
@michplunkett michplunkett deleted the chore/minor-cleanup branch August 2, 2024 18:35
sea-kelp added a commit to OrcaCollective/OpenOversight that referenced this pull request Sep 2, 2024
@sea-kelp
Fix database fixture and password unit test (lucyparsons#1120)
323aba2
AetherUnbound pushed a commit to OrcaCollective/OpenOversight that referenced this pull request Sep 2, 2024
@sea-kelp @michplunkett
Merge from upstream (#500)
57167f2 
* Replace `flake8` with `ruff` (lucyparsons#1101)

Replacing `flake8` with `ruff`, which has approximate parity and is more
than 10x faster:
https://docs.astral.sh/ruff/faq/#how-does-ruffs-linter-compare-to-flake8

 - [x] This branch is up-to-date with the `develop` branch.
 - [x] `pytest` passes on my local development environment.
 - [x] `pre-commit` passes on my local development environment.

* Remove `black` and use `ruff` formatter (lucyparsons#1106)

* Address `SQLAlchemy` deprecation warnings (lucyparsons#1111)

Part of lucyparsons#1054

Address warnings for for `RemovedIn20Warning` and `LegacyAPIWarning`
while using the [`SQLALCHEMY_WARN_20`
flag](https://docs.sqlalchemy.org/en/20/changelog/migration_14.html#sqlalchemy-2-0-deprecations-mode).
If you would like me to do any additional manual checks, etc. to verify
these changes, please don't hesitate to ask.

<details><summary>Unmodified warnings</summary>

```console
=============================================================================================== warnings summary ===============================================================================================
../../local/lib/python3.11/site-packages/flask_wtf/recaptcha/widgets.py:2: 18 warnings
  /usr/local/lib/python3.11/site-packages/flask_wtf/recaptcha/widgets.py:2: DeprecationWarning: 'flask.Markup' is deprecated and will be removed in Flask 2.4. Import 'markupsafe.Markup' instead.
    from flask import Markup

OpenOversight/tests/test_commands.py::test_add_department__success
OpenOversight/tests/test_database_cache.py::test_get_database_cache_entry
OpenOversight/tests/test_utils.py::test_department_filter
OpenOversight/tests/test_models.py::test_department_repr
OpenOversight/tests/test_alembic.py::test_alembic_has_single_head
OpenOversight/tests/routes/test_image_tagging.py::test_routes_ok[/labels]
OpenOversight/tests/test_email_client.py::test_smtp_email_provider_send_email
  /usr/src/app/OpenOversight/tests/conftest.py:678: RemovedIn20Warning: "Incident" object is being merged into a Session along the backref cascade path for relationship "Location.incidents"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    Incident(

OpenOversight/tests/test_commands.py::test_add_department__success
OpenOversight/tests/test_database_cache.py::test_get_database_cache_entry
OpenOversight/tests/test_utils.py::test_department_filter
OpenOversight/tests/test_models.py::test_department_repr
OpenOversight/tests/test_alembic.py::test_alembic_has_single_head
OpenOversight/tests/routes/test_image_tagging.py::test_routes_ok[/labels]
OpenOversight/tests/test_email_client.py::test_smtp_email_provider_send_email
  /usr/src/app/OpenOversight/tests/conftest.py:691: RemovedIn20Warning: "Incident" object is being merged into a Session along the backref cascade path for relationship "Location.incidents"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    Incident(

OpenOversight/tests/test_commands.py::test_add_department__success
OpenOversight/tests/test_database_cache.py::test_get_database_cache_entry
OpenOversight/tests/test_utils.py::test_department_filter
OpenOversight/tests/test_models.py::test_department_repr
OpenOversight/tests/test_alembic.py::test_alembic_has_single_head
OpenOversight/tests/routes/test_image_tagging.py::test_routes_ok[/labels]
OpenOversight/tests/test_email_client.py::test_smtp_email_provider_send_email
  /usr/src/app/OpenOversight/tests/conftest.py:704: RemovedIn20Warning: "Incident" object is being merged into a Session along the backref cascade path for relationship "Location.incidents"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    Incident(

OpenOversight/tests/test_commands.py::test_add_department__success
OpenOversight/tests/test_database_cache.py::test_get_database_cache_entry
OpenOversight/tests/test_utils.py::test_department_filter
OpenOversight/tests/test_models.py::test_department_repr
OpenOversight/tests/test_alembic.py::test_alembic_has_single_head
OpenOversight/tests/routes/test_image_tagging.py::test_routes_ok[/labels]
OpenOversight/tests/test_email_client.py::test_smtp_email_provider_send_email
  /usr/src/app/OpenOversight/tests/conftest.py:725: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    first_officer = Officer.query.get(1)

OpenOversight/tests/test_commands.py::test_add_department__success
OpenOversight/tests/test_database_cache.py::test_get_database_cache_entry
OpenOversight/tests/test_utils.py::test_department_filter
OpenOversight/tests/test_models.py::test_department_repr
OpenOversight/tests/test_alembic.py::test_alembic_has_single_head
OpenOversight/tests/routes/test_image_tagging.py::test_routes_ok[/labels]
OpenOversight/tests/test_email_client.py::test_smtp_email_provider_send_email
  /usr/src/app/OpenOversight/tests/conftest.py:740: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    first_officer = Officer.query.get(1)

OpenOversight/tests/test_commands.py::test_add_department__duplicate
OpenOversight/tests/test_models.py::test__uuid_uniqueness_constraint
OpenOversight/tests/routes/test_singular_redirects.py::test_redirect_add_salary
  /usr/src/app/OpenOversight/tests/conftest.py:325: SAWarning: transaction already deassociated from connection
    transaction.rollback()

OpenOversight/tests/test_commands.py::test_add_job_title__success
OpenOversight/tests/test_commands.py::test_add_job_title__different_departments
  /usr/src/app/OpenOversight/app/commands.py:645: RemovedIn20Warning: "Job" object is being merged into a Session along the backref cascade path for relationship "Department.jobs"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    job = Job(

OpenOversight/tests/test_utils.py::test_filters_do_not_exclude_officers_without_assignments
  /usr/src/app/OpenOversight/tests/test_utils.py:102: RemovedIn20Warning: "Officer" object is being merged into a Session along the backref cascade path for relationship "Department.officers"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    officer = Officer(

OpenOversight/tests/test_models.py::test_salary_repr
  /usr/src/app/OpenOversight/tests/test_models.py:170: SAWarning: Dialect sqlite+pysqlite does *not* support Decimal objects natively, and SQLAlchemy must convert from floating point - rounding errors and other issues may occur. Please consider storing Decimal numbers as strings or integers on this platform for lossless storage.
    salary = Salary.query.first()

OpenOversight/tests/test_commands.py::test_csv_import_new
  /usr/src/app/OpenOversight/tests/conftest.py:855: SAWarning: Dialect sqlite+pysqlite does *not* support Decimal objects natively, and SQLAlchemy must convert from floating point - rounding errors and other issues may occur. Please consider storing Decimal numbers as strings or integers on this platform for lossless storage.
    if len(list(officer.salaries)) > 0:

OpenOversight/tests/test_database_cache.py::test_documented_assignments
OpenOversight/tests/routes/test_image_tagging.py::test_admin_can_delete_tag
OpenOversight/tests/routes/test_descriptions.py::test_officer_descriptions_markdown
OpenOversight/tests/routes/test_notes.py::test_officer_notes_markdown
  /usr/local/lib/python3.11/site-packages/jinja2/environment.py:487: SAWarning: Dialect sqlite+pysqlite does *not* support Decimal objects natively, and SQLAlchemy must convert from floating point - rounding errors and other issues may occur. Please consider storing Decimal numbers as strings or integers on this platform for lossless storage.
    return getattr(obj, attribute)

OpenOversight/tests/test_database_cache.py: 1 warning
OpenOversight/tests/routes/test_incidents.py: 9 warnings
OpenOversight/tests/routes/test_officer_and_department.py: 1 warning
  /usr/src/app/OpenOversight/app/utils/forms.py:203: RemovedIn20Warning: "Incident" object is being merged into a Session along the backref cascade path for relationship "Location.incidents"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    return Incident(

OpenOversight/tests/test_database_cache.py: 2 warnings
OpenOversight/tests/routes/test_descriptions.py: 29 warnings
OpenOversight/tests/routes/test_incidents.py: 34 warnings
OpenOversight/tests/routes/test_notes.py: 29 warnings
OpenOversight/tests/routes/test_singular_redirects.py: 21 warnings
OpenOversight/tests/routes/test_officer_and_department.py: 52 warnings
  /usr/local/lib/python3.11/site-packages/flask_sqlalchemy/query.py:30: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    rv = self.get(ident)

OpenOversight/tests/test_database_cache.py: 1 warning
OpenOversight/tests/routes/test_officer_and_department.py: 11 warnings
  /usr/src/app/OpenOversight/app/utils/forms.py:78: RemovedIn20Warning: "Assignment" object is being merged into a Session along the backref cascade path for relationship "Officer.assignments"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    assignment = Assignment(

OpenOversight/tests/routes/test_image_tagging.py::test_ac_cannot_delete_tag_not_in_their_dept
  /usr/src/app/OpenOversight/tests/routes/test_image_tagging.py:126: RemovedIn20Warning: The ``aliased`` and ``from_joinpoint`` keyword arguments to Query.join() are deprecated and will be removed in SQLAlchemy 2.0. (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    Face.query.join(Face.officer, aliased=True)

OpenOversight/tests/routes/test_image_tagging.py::test_user_is_redirected_to_correct_department_after_tagging
  /usr/src/app/OpenOversight/tests/routes/test_image_tagging.py:285: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    department = Department.query.get(department_id)

OpenOversight/tests/test_commands.py::test_csv_new_salary
  /usr/src/app/OpenOversight/tests/test_commands.py:463: FutureWarning: Setting an item of incompatible dtype is deprecated and will raise an error in a future version of pandas. Value '123456.78' has dtype incompatible with float64, please explicitly cast to a compatible dtype first.
    df.loc[0, "salary"] = "123456.78"

OpenOversight/tests/test_commands.py::test_bulk_add_officers__success
  /usr/src/app/OpenOversight/tests/test_commands.py:511: RemovedIn20Warning: "Assignment" object is being merged into a Session along the backref cascade path for relationship "Officer.assignments"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    assignment = Assignment(base_officer=first_officer, job_id=job.id)

OpenOversight/tests/test_commands.py::test_bulk_add_officers__success
  /usr/src/app/OpenOversight/tests/test_commands.py:519: RemovedIn20Warning: "Assignment" object is being merged into a Session along the backref cascade path for relationship "Officer.assignments"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    assignment = Assignment(base_officer=different_officer, job=job, created_by=user.id)

OpenOversight/tests/routes/test_incidents.py::test_admins_can_edit_incident_date_and_address
  /usr/src/app/OpenOversight/tests/routes/test_incidents.py:253: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    updated = Incident.query.get(inc_id)

OpenOversight/tests/routes/test_descriptions.py::test_admins_can_delete_descriptions
  /usr/src/app/OpenOversight/tests/routes/test_descriptions.py:293: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    deleted = Description.query.get(description_id)

OpenOversight/tests/routes/test_image_tagging.py::test_ac_cannot_set_featured_tag_not_in_their_dept
  /usr/src/app/OpenOversight/tests/routes/test_image_tagging.py:326: RemovedIn20Warning: The ``aliased`` and ``from_joinpoint`` keyword arguments to Query.join() are deprecated and will be removed in SQLAlchemy 2.0. (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    Face.query.join(Face.officer, aliased=True)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__success
  /usr/src/app/OpenOversight/tests/test_commands.py:889: RemovedIn20Warning: "Incident" object is being merged into a Session along the backref cascade path for relationship "Officer.incidents"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    incident.officers = [officer]

OpenOversight/tests/test_commands.py::test_advanced_csv_import__success
OpenOversight/tests/test_commands.py::test_advanced_csv_import__success
OpenOversight/tests/test_commands.py::test_advanced_csv_import__force_create
  /usr/src/app/OpenOversight/app/models/database_imports.py:308: RemovedIn20Warning: "Incident" object is being merged into a Session along the backref cascade path for relationship "Officer.incidents"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    incident.officers = data.get("officers", [])

OpenOversight/tests/test_commands.py::test_advanced_csv_import__success
  /usr/src/app/OpenOversight/tests/test_commands.py:1012: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    incident3 = Incident.query.get(123456)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__success
  /usr/src/app/OpenOversight/tests/test_commands.py:1035: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    updated_link = Link.query.get(55051)

OpenOversight/tests/routes/test_descriptions.py::test_acs_can_delete_their_descriptions_in_their_department
  /usr/src/app/OpenOversight/tests/routes/test_descriptions.py:322: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    deleted = Description.query.get(description_id)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__force_create
  /usr/src/app/OpenOversight/app/csv_imports.py:560: RemovedIn20Warning: Using plain strings to indicate SQL statements without using the text() construct is  deprecated and will be removed in version 2.0.  Ensure plain SQL statements are passed using the text() construct. (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    db.session.execute(raw_sql)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__force_create
  /usr/src/app/OpenOversight/tests/test_commands.py:1160: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    cop1 = Officer.query.get(99001)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__force_create
  /usr/src/app/OpenOversight/tests/test_commands.py:1163: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    cop2 = Officer.query.get(99002)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__force_create
  /usr/src/app/OpenOversight/tests/test_commands.py:1165: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    assert cop2.assignments[0] == Assignment.query.get(98001)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__force_create
  /usr/src/app/OpenOversight/tests/test_commands.py:1167: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    cop3 = Officer.query.get(99003)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__force_create
  /usr/src/app/OpenOversight/tests/test_commands.py:1169: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    assert cop3.salaries[0] == Salary.query.get(77001)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__force_create
  /usr/src/app/OpenOversight/tests/test_commands.py:1171: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    incident = Incident.query.get(66001)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__force_create
  /usr/src/app/OpenOversight/tests/test_commands.py:1176: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    link = Link.query.get(55001)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__overwrite_assignments
  /usr/src/app/OpenOversight/tests/test_commands.py:1279: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    cop1 = Officer.query.get(cop1_id)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__overwrite_assignments
  /usr/src/app/OpenOversight/tests/test_commands.py:1283: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    cop2 = Officer.query.get(cop2_id)

OpenOversight/tests/test_commands.py::test_advanced_csv_import__overwrite_assignments
  /usr/src/app/OpenOversight/tests/test_commands.py:1285: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    assert cop2.assignments[0] == Assignment.query.get(a2_id)

OpenOversight/tests/routes/test_descriptions.py::test_acs_cannot_delete_descriptions_not_in_their_department
  /usr/src/app/OpenOversight/tests/routes/test_descriptions.py:354: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    not_deleted = Description.query.get(description_id)

OpenOversight/tests/routes/test_incidents.py: 12 warnings
  /usr/src/app/OpenOversight/app/main/forms.py:505: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    officer = Officer.query.get(officer_id)

OpenOversight/tests/routes/test_notes.py::test_admins_can_delete_notes
  /usr/src/app/OpenOversight/tests/routes/test_notes.py:263: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    deleted = Note.query.get(note.id)

OpenOversight/tests/routes/test_notes.py::test_acs_can_delete_their_notes_in_their_department
  /usr/src/app/OpenOversight/tests/routes/test_notes.py:288: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    deleted = Note.query.get(note.id)

OpenOversight/tests/routes/test_notes.py::test_acs_cannot_delete_notes_not_in_their_department
  /usr/src/app/OpenOversight/tests/routes/test_notes.py:317: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    not_deleted = Note.query.get(note.id)

OpenOversight/tests/routes/test_incidents.py::test_admins_can_delete_incidents
  /usr/src/app/OpenOversight/tests/routes/test_incidents.py:683: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    deleted = Incident.query.get(inc_id)

OpenOversight/tests/routes/test_incidents.py::test_acs_can_delete_incidents_in_their_department
  /usr/src/app/OpenOversight/tests/routes/test_incidents.py:697: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    deleted = Incident.query.get(inc_id)

OpenOversight/tests/routes/test_incidents.py::test_acs_cannot_delete_incidents_not_in_their_department
  /usr/src/app/OpenOversight/tests/routes/test_incidents.py:713: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    not_deleted = Incident.query.get(inc_id)

OpenOversight/tests/routes/test_user_api.py: 21 warnings
  /usr/src/app/OpenOversight/app/auth/views.py:295: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    user = User.query.get(user_id)

OpenOversight/tests/routes/test_user_api.py::test_admin_can_delete_user
OpenOversight/tests/routes/test_user_api.py::test_admin_can_delete_user
OpenOversight/tests/routes/test_user_api.py::test_admin_cannot_delete_other_admin
  /usr/src/app/OpenOversight/app/auth/views.py:342: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    user = User.query.get(user_id)

OpenOversight/tests/routes/test_user_api.py::test_admin_can_delete_user
  /usr/src/app/OpenOversight/tests/routes/test_user_api.py:137: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    assert not User.query.get(user.id)

OpenOversight/tests/routes/test_user_api.py::test_admin_cannot_delete_other_admin
  /usr/src/app/OpenOversight/tests/routes/test_user_api.py:153: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    assert User.query.get(user.id) is not None

OpenOversight/tests/routes/test_user_api.py::test_admin_can_disable_user
  /usr/src/app/OpenOversight/tests/routes/test_user_api.py:178: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    user = User.query.get(user.id)

OpenOversight/tests/routes/test_user_api.py::test_admin_cannot_disable_self
  /usr/src/app/OpenOversight/tests/routes/test_user_api.py:201: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    user = User.query.get(user.id)

OpenOversight/tests/routes/test_user_api.py::test_admin_can_enable_user
  /usr/src/app/OpenOversight/tests/routes/test_user_api.py:213: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    user = User.query.get(user.id)

OpenOversight/tests/routes/test_user_api.py::test_admin_can_enable_user
  /usr/src/app/OpenOversight/tests/routes/test_user_api.py:229: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    user = User.query.get(user.id)

OpenOversight/tests/routes/test_user_api.py::test_admin_can_approve_user
  /usr/src/app/OpenOversight/tests/routes/test_user_api.py:296: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    user = User.query.get(user.id)

OpenOversight/tests/routes/test_user_api.py::test_admin_can_approve_user
  /usr/src/app/OpenOversight/tests/routes/test_user_api.py:312: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    user = User.query.get(user.id)

OpenOversight/tests/routes/test_user_api.py::test_admin_approval_sends_confirmation_email[False-False-True-True]
OpenOversight/tests/routes/test_user_api.py::test_admin_approval_sends_confirmation_email[False-False-False-False]
OpenOversight/tests/routes/test_user_api.py::test_admin_approval_sends_confirmation_email[True-False-True-False]
OpenOversight/tests/routes/test_user_api.py::test_admin_approval_sends_confirmation_email[False-True-True-False]
  /usr/src/app/OpenOversight/tests/routes/test_user_api.py:349: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    user = User.query.get(user.id)

OpenOversight/tests/routes/test_user_api.py::test_admin_approval_sends_confirmation_email[False-False-True-True]
OpenOversight/tests/routes/test_user_api.py::test_admin_approval_sends_confirmation_email[False-False-False-False]
OpenOversight/tests/routes/test_user_api.py::test_admin_approval_sends_confirmation_email[True-False-True-False]
OpenOversight/tests/routes/test_user_api.py::test_admin_approval_sends_confirmation_email[False-True-True-False]
  /usr/src/app/OpenOversight/tests/routes/test_user_api.py:366: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    user = User.query.get(user.id)

OpenOversight/tests/routes/test_officer_and_department.py::test_admin_can_add_new_officer
  /usr/src/app/OpenOversight/app/utils/forms.py:97: RemovedIn20Warning: "Note" object is being merged into a Session along the backref cascade path for relationship "Officer.notes"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    new_note = Note(

OpenOversight/tests/routes/test_officer_and_department.py::test_admin_can_add_new_officer
  /usr/src/app/OpenOversight/app/utils/forms.py:108: RemovedIn20Warning: "Description" object is being merged into a Session along the backref cascade path for relationship "Officer.descriptions"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    new_description = Description(

OpenOversight/tests/routes/test_officer_and_department.py::test_admin_can_add_new_officer
  /usr/src/app/OpenOversight/app/utils/forms.py:119: RemovedIn20Warning: "Salary" object is being merged into a Session along the backref cascade path for relationship "Officer.salaries"; in SQLAlchemy 2.0, this reverse cascade will not take place.  Set cascade_backrefs to False in either the relationship() or backref() function for the 2.0 behavior; or to set globally for the whole Session, set the future=True flag (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    new_salary = Salary(

OpenOversight/tests/routes/test_officer_and_department.py: 109 warnings
  /usr/src/app/OpenOversight/tests/routes/test_officer_and_department.py:1777: LegacyAPIWarning: The Query.get() method is considered legacy as of the 1.x series of SQLAlchemy and becomes a legacy construct in 2.0. The method is now available as Session.get() (deprecated since: 1.4) (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)
    assert (

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

---------- coverage: platform linux, python 3.11.9-final-0 -----------
```
</details>

 - [x] This branch is up-to-date with the `develop` branch.
 - [x] `pytest` passes on my local development environment.
 - [x] `pre-commit` passes on my local development environment.

* Address database warnings and deprecated syntax (lucyparsons#1115)

lucyparsons#1054

Addressed the following error in tests by modifying the `Salary` table
and deprecated syntax throughout the application.

```console
FutureWarning: Setting an item of incompatible dtype is deprecated and will raise an error in a future version of pandas. Value '123456.78' has dtype incompatible with float64, please explicitly cast to a compatible dtype first.
SAWarning: Dialect sqlite+pysqlite does *not* support Decimal objects natively, and SQLAlchemy must convert from floating point - rounding errors and other issues may occur. Please consider storing Decimal numbers as strings or integers on this platform for lossless storage.
```

<details><summary>Warnings before changes</summary>

```console
=============================================================================================== warnings summary ===============================================================================================
OpenOversight/tests/test_models.py::test_salary_repr
  /usr/src/app/OpenOversight/tests/test_models.py:170: SAWarning: Dialect sqlite+pysqlite does *not* support Decimal objects natively, and SQLAlchemy must convert from floating point - rounding errors and other issues may occur. Please consider storing Decimal numbers as strings or integers on this platform for lossless storage.
    salary = Salary.query.first()

OpenOversight/tests/test_commands.py::test_add_department__duplicate
OpenOversight/tests/test_models.py::test__uuid_uniqueness_constraint
OpenOversight/tests/routes/test_singular_redirects.py::test_redirect_add_salary
  /usr/src/app/OpenOversight/tests/conftest.py:325: SAWarning: transaction already deassociated from connection
    transaction.rollback()

OpenOversight/tests/test_commands.py::test_csv_import_new
  /usr/src/app/OpenOversight/tests/conftest.py:855: SAWarning: Dialect sqlite+pysqlite does *not* support Decimal objects natively, and SQLAlchemy must convert from floating point - rounding errors and other issues may occur. Please consider storing Decimal numbers as strings or integers on this platform for lossless storage.
    if len(list(officer.salaries)) > 0:

OpenOversight/tests/routes/test_image_tagging.py::test_admin_can_delete_tag
OpenOversight/tests/routes/test_descriptions.py::test_officer_descriptions_markdown
OpenOversight/tests/test_database_cache.py::test_documented_assignments
  /usr/local/lib/python3.11/site-packages/jinja2/environment.py:487: SAWarning: Dialect sqlite+pysqlite does *not* support Decimal objects natively, and SQLAlchemy must convert from floating point - rounding errors and other issues may occur. Please consider storing Decimal numbers as strings or integers on this platform for lossless storage.
    return getattr(obj, attribute)

OpenOversight/tests/test_commands.py::test_csv_new_salary
  /usr/src/app/OpenOversight/tests/test_commands.py:463: FutureWarning: Setting an item of incompatible dtype is deprecated and will raise an error in a future version of pandas. Value '123456.78' has dtype incompatible with float64, please explicitly cast to a compatible dtype first.
    df.loc[0, "salary"] = "123456.78"

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

---------- coverage: platform linux, python 3.11.9-final-0 -----------
```
</details>

- [x] This branch is up-to-date with the `develop` branch.
- [x] `pytest` passes on my local development environment.
- [x] `pre-commit` passes on my local development environment.
- [x] Validated warning is gone.

```console
=============================================================================================== warnings summary ===============================================================================================
OpenOversight/tests/test_commands.py::test_add_department__duplicate
OpenOversight/tests/test_models.py::test__uuid_uniqueness_constraint
OpenOversight/tests/routes/test_singular_redirects.py::test_redirect_add_salary
  /usr/src/app/OpenOversight/tests/conftest.py:325: SAWarning: transaction already deassociated from connection
    transaction.rollback()

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

---------- coverage: platform linux, python 3.11.9-final-0 -----------
```

- [x] Validated data migration works.

```console
I have no name!@36326fd7801f:/usr/src/app$ flask db stamp head
...
INFO  [alembic.runtime.migration] Will assume transactional DDL.
I have no name!@36326fd7801f:/usr/src/app$ flask db upgrade
...
INFO  [alembic.runtime.migration] Running upgrade 939ea0f2b26d -> 5865f488470c, change salary column types
I have no name!@36326fd7801f:/usr/src/app$ flask db downgrade
...
INFO  [alembic.runtime.migration] Running downgrade 5865f488470c -> 939ea0f2b26d, change salary column types
I have no name!@36326fd7801f:/usr/src/app$ flask db upgrade
...
INFO  [alembic.runtime.migration] Running upgrade 939ea0f2b26d -> 5865f488470c, change salary column types
I have no name!@36326fd7801f:/usr/src/app$
```

* Remove `transaction.rollback()` and use `session` fixture (lucyparsons#1117)

lucyparsons#1054
- This is the last PR for this issue. ❗❗❗❗❗❗

Removed the redundant `transaction.rollback()` function call in the
`session` fixture and changed all references to `db.session` to
`session` so that tests are using the correct scoping.

Addressed this warning:
```console
=============================================================================================== warnings summary ===============================================================================================
OpenOversight/tests/test_commands.py::test_add_department__duplicate
OpenOversight/tests/test_models.py::test__uuid_uniqueness_constraint
OpenOversight/tests/routes/test_singular_redirects.py::test_redirect_add_salary
  /usr/src/app/OpenOversight/tests/conftest.py:325: SAWarning: transaction already deassociated from connection
    transaction.rollback()

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

---------- coverage: platform linux, python 3.11.9-final-0 -----------
```

- [x] This branch is up-to-date with the `develop` branch.
- [x] `pytest` passes on my local development environment.
- [x] `pre-commit` passes on my local development environment.
- [x] Validate that no more warnings show up in the `make test` command.

```console
---------- coverage: platform linux, python 3.11.9-final-0 -----------
Name                                                                                                         Stmts   Miss  Cover
--------------------------------------------------------------------------------------------------------------------------------
OpenOversight/__init__.py                                                                                        0      0   100%
OpenOversight/app/__init__.py                                                                                   74      1    99%
```

* Fix database fixture and password unit test (lucyparsons#1120)

* Add user profile tests (lucyparsons#1119)

## Fixes issue
lucyparsons#436

## Description of Changes
Added tests to validate `/user/` route logic and correct profile logic
to match pre-specified tests.

<img width="497" alt="Screenshot 2024-07-31 at 5 27 37 PM"
src="https://github.com/user-attachments/assets/78703665-1623-4703-8fa9-a1cca59ba319">

There is not a `/users/` route, so I marked it out.

## Tests and Linting
- [x] This branch is up-to-date with the `develop` branch.
- [x] `pytest` passes on my local development environment.
- [x] `pre-commit` passes on my local development environment.

* Upgrade pre-commit libraries

---------

Co-authored-by: Michael Plunkett <5885605+michplunkett@users.noreply.github.com>
b-meson pushed a commit that referenced this pull request Sep 5, 2024
@sea-kelp @b-meson
Fix database fixture and password unit test (#1120)
a61b309
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michplunkett michplunkett michplunkett approved these changes

Assignees

@sea-kelp sea-kelp

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sea-kelp @michplunkett