Remove transaction.rollback() and use session fixture
#1117
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
michplunkett
commented
Jul 24, 2024
| @@ -320,11 +320,11 @@ def session(db): | |||
| session = scoped_session(session_factory=sessionmaker(bind=connection)) | |||
| db.session = session | |||
|
|
|||
| yield session | |||
|
|
|||
| transaction.rollback() | |||
There was a problem hiding this comment.
The transaction.rollback() function call here was redundant. SQLAlchemy was already taking care of it for any of the tests that needed it (i.e., test__uuid_uniqueness_constraint, test_redirect_add_salary, test_add_department__duplicate).
michplunkett
changed the title
transaction.rollback() and use session fixturetransaction.rollback() and use session fixture
michplunkett
commented
Jul 24, 2024
| @@ -4,4 +4,4 @@ | |||
| Running `make dev` in the docker environment will create and persist the database. | |||
|
|
|||
| ## Database Diagram | |||
|  | |||
|  | |||
There was a problem hiding this comment.
I updated the database relationship diagram so that it's in-sync with the current state of things.
There was a problem hiding this comment.
thanks for doing that the updated database diagram, I think that's super helpful for contributors.
sea-kelp
approved these changes
Jul 28, 2024
sea-kelp
reviewed
Jul 29, 2024
| yield session | ||
| finally: | ||
| session.remove() | ||
| connection.close() |
There was a problem hiding this comment.
Sorry, quick comment - we shouldn't need this connection.close anymore since the context manager should automatically close the connection for us
There was a problem hiding this comment.
I remember trying to remove both of those individually when I made the change and I got some failed tests. If i remove the connection.close() statement, I get 53 failed tests.
============================================================================ 53 failed, 436 passed, 2 skipped, 12 errors in 21.03s =============================================================================
sea-kelp
pushed a commit
to OrcaCollective/OpenOversight
that referenced
this pull request
Sep 2, 2024
…s#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% ```
3 tasks
AetherUnbound
pushed a commit
to OrcaCollective/OpenOversight
that referenced
this pull request
Sep 2, 2024
* 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>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes issue
#1054
Description of Changes
Removed the redundant
transaction.rollback()function call in thesessionfixture and changed all references todb.sessiontosessionso that tests are using the correct scoping.Addressed this warning:
Tests and Linting
developbranch.pytestpasses on my local development environment.pre-commitpasses on my local development environment.make testcommand.