Skip to content

fix: async middleware is equivalent to sync middleware - DIA-61984#107

Merged
arththebird merged 7 commits intomasterfrom
fix/DIA-61984/test-refactor
Nov 6, 2023
Merged

fix: async middleware is equivalent to sync middleware - DIA-61984#107
arththebird merged 7 commits intomasterfrom
fix/DIA-61984/test-refactor

Conversation

@arththebird
Copy link
Copy Markdown
Contributor

@arththebird arththebird commented Nov 3, 2023

Description

  • Update async middleware and context manager to be equivalent to the sync version.
  • Refactor async tests to be equivalent to sync version.
  • Update pytest plugin to be consistent between sync and async version.

Related PRs

Extracted from #105

Related JIRA issues

Validation

New behaviour is thoroughly tested with unit and integration tests ✅

Comment thread tests/conftest.py
# reload fastapi_sqla to clear sqla deferred reflection mapping stored in Base
importlib.reload(fastapi_sqla.models)
importlib.reload(fastapi_sqla.sqla)
importlib.reload(fastapi_sqla.async_sqla)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Forgotten in previous refactor

@@ -0,0 +1,83 @@
from unittest.mock import Mock
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The middleware tests were refactored to improve coverage for the async version by adopting a similar pattern as the pagination tests.

Comment thread tests/test_middleware.py
@@ -1,191 +0,0 @@
from unittest.mock import Mock, patch
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The tests related to startup were moved to test_startup and the ones related to open_session moved to test_open_session.

Comment thread tests/test_startup.py


@fixture(params=[True, False])
def case_sensitive_environ(environ, request):
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It was causing issue with the PYTEST_CURRENT_TEST env var, so moved to its own test

@codecov
Copy link
Copy Markdown

codecov Bot commented Nov 3, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e9510ac) 100.00% compared to head (2bd71b4) 100.00%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #107   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines          420       434   +14     
  Branches        56        58    +2     
=========================================
+ Hits           420       434   +14     
Flag Coverage Δ
python_version3.10-sqlalchemy1.4-pydantic1.10-asyncpg-aws_rds_iam 97.92% <96.77%> (+0.78%) ⬆️
python_version3.10-sqlalchemy1.4-pydantic1.10-asyncpg-noaws_rds_iam 96.08% <96.77%> (+0.84%) ⬆️
python_version3.10-sqlalchemy1.4-pydantic1.10-noasyncpg-aws_rds_iam 74.88% <22.58%> (-3.93%) ⬇️
python_version3.10-sqlalchemy1.4-pydantic1.10-noasyncpg-noaws_rds_iam 73.04% <22.58%> (-3.87%) ⬇️
python_version3.10-sqlalchemy1.4-pydantic2.0-asyncpg-aws_rds_iam 97.92% <96.77%> (+0.78%) ⬆️
python_version3.10-sqlalchemy1.4-pydantic2.0-asyncpg-noaws_rds_iam 96.08% <96.77%> (+0.84%) ⬆️
python_version3.10-sqlalchemy1.4-pydantic2.0-noasyncpg-aws_rds_iam 74.88% <22.58%> (-3.93%) ⬇️
python_version3.10-sqlalchemy1.4-pydantic2.0-noasyncpg-noaws_rds_iam 73.04% <22.58%> (-3.87%) ⬇️
python_version3.10-sqlalchemy1.4-pydantic2.1-asyncpg-aws_rds_iam 97.92% <96.77%> (+0.78%) ⬆️
python_version3.10-sqlalchemy1.4-pydantic2.1-asyncpg-noaws_rds_iam 96.08% <96.77%> (+0.84%) ⬆️
python_version3.10-sqlalchemy1.4-pydantic2.1-noasyncpg-aws_rds_iam 74.88% <22.58%> (-3.93%) ⬇️
python_version3.10-sqlalchemy1.4-pydantic2.1-noasyncpg-noaws_rds_iam 73.04% <22.58%> (-3.87%) ⬇️
python_version3.10-sqlalchemy2.0-pydantic1.10-asyncpg-aws_rds_iam 97.23% <96.77%> (+0.80%) ⬆️
python_version3.10-sqlalchemy2.0-pydantic1.10-asyncpg-noaws_rds_iam 95.39% <96.77%> (+0.86%) ⬆️
python_version3.10-sqlalchemy2.0-pydantic1.10-noasyncpg-aws_rds_iam 74.19% <22.58%> (-3.91%) ⬇️
python_version3.10-sqlalchemy2.0-pydantic1.10-noasyncpg-noaws_rds_iam 72.35% <22.58%> (-3.85%) ⬇️
python_version3.10-sqlalchemy2.0-pydantic2.0-asyncpg-aws_rds_iam 97.23% <96.77%> (+0.80%) ⬆️
python_version3.10-sqlalchemy2.0-pydantic2.0-asyncpg-noaws_rds_iam 95.39% <96.77%> (+0.86%) ⬆️
python_version3.10-sqlalchemy2.0-pydantic2.0-noasyncpg-aws_rds_iam 74.19% <22.58%> (-3.91%) ⬇️
python_version3.10-sqlalchemy2.0-pydantic2.0-noasyncpg-noaws_rds_iam 72.35% <22.58%> (-3.85%) ⬇️
python_version3.10-sqlalchemy2.0-pydantic2.1-asyncpg-aws_rds_iam 97.23% <96.77%> (+0.80%) ⬆️
python_version3.10-sqlalchemy2.0-pydantic2.1-asyncpg-noaws_rds_iam 95.39% <96.77%> (+0.86%) ⬆️
python_version3.10-sqlalchemy2.0-pydantic2.1-noasyncpg-aws_rds_iam 74.19% <22.58%> (-3.91%) ⬇️
python_version3.10-sqlalchemy2.0-pydantic2.1-noasyncpg-noaws_rds_iam 72.35% <22.58%> (-3.85%) ⬇️
python_version3.11-sqlalchemy1.4-pydantic1.10-asyncpg-aws_rds_iam 97.92% <96.77%> (+0.78%) ⬆️
python_version3.11-sqlalchemy1.4-pydantic1.10-asyncpg-noaws_rds_iam 96.08% <96.77%> (+0.84%) ⬆️
python_version3.11-sqlalchemy1.4-pydantic1.10-noasyncpg-aws_rds_iam 74.88% <22.58%> (-3.93%) ⬇️
python_version3.11-sqlalchemy1.4-pydantic1.10-noasyncpg-noaws_rds_iam 73.04% <22.58%> (-3.87%) ⬇️
python_version3.11-sqlalchemy1.4-pydantic2.0-asyncpg-aws_rds_iam 97.92% <96.77%> (+0.78%) ⬆️
python_version3.11-sqlalchemy1.4-pydantic2.0-asyncpg-noaws_rds_iam 96.08% <96.77%> (+0.84%) ⬆️
python_version3.11-sqlalchemy1.4-pydantic2.0-noasyncpg-aws_rds_iam 74.88% <22.58%> (-3.93%) ⬇️
python_version3.11-sqlalchemy1.4-pydantic2.0-noasyncpg-noaws_rds_iam 73.04% <22.58%> (-3.87%) ⬇️
python_version3.11-sqlalchemy1.4-pydantic2.1-asyncpg-aws_rds_iam 97.92% <96.77%> (+0.78%) ⬆️
python_version3.11-sqlalchemy1.4-pydantic2.1-asyncpg-noaws_rds_iam 96.08% <96.77%> (+0.84%) ⬆️
python_version3.11-sqlalchemy1.4-pydantic2.1-noasyncpg-aws_rds_iam 74.88% <22.58%> (-3.93%) ⬇️
python_version3.11-sqlalchemy1.4-pydantic2.1-noasyncpg-noaws_rds_iam 73.04% <22.58%> (-3.87%) ⬇️
python_version3.11-sqlalchemy2.0-pydantic1.10-asyncpg-aws_rds_iam 97.23% <96.77%> (+0.80%) ⬆️
python_version3.11-sqlalchemy2.0-pydantic1.10-asyncpg-noaws_rds_iam 95.39% <96.77%> (+0.86%) ⬆️
python_version3.11-sqlalchemy2.0-pydantic1.10-noasyncpg-aws_rds_iam 74.19% <22.58%> (-3.91%) ⬇️
python_version3.11-sqlalchemy2.0-pydantic1.10-noasyncpg-noaws_rds_iam 72.35% <22.58%> (-3.85%) ⬇️
python_version3.11-sqlalchemy2.0-pydantic2.0-asyncpg-aws_rds_iam 97.23% <96.77%> (+0.80%) ⬆️
python_version3.11-sqlalchemy2.0-pydantic2.0-asyncpg-noaws_rds_iam 95.39% <96.77%> (+0.86%) ⬆️
python_version3.11-sqlalchemy2.0-pydantic2.0-noasyncpg-aws_rds_iam 74.19% <22.58%> (-3.91%) ⬇️
python_version3.11-sqlalchemy2.0-pydantic2.0-noasyncpg-noaws_rds_iam 72.35% <22.58%> (-3.85%) ⬇️
python_version3.11-sqlalchemy2.0-pydantic2.1-asyncpg-aws_rds_iam 97.23% <96.77%> (+0.80%) ⬆️
python_version3.11-sqlalchemy2.0-pydantic2.1-asyncpg-noaws_rds_iam 95.39% <96.77%> (+0.86%) ⬆️
python_version3.11-sqlalchemy2.0-pydantic2.1-noasyncpg-aws_rds_iam 74.19% <22.58%> (-3.91%) ⬇️
python_version3.11-sqlalchemy2.0-pydantic2.1-noasyncpg-noaws_rds_iam 72.35% <22.58%> (-3.85%) ⬇️
python_version3.9-sqlalchemy1.4-pydantic1.10-asyncpg-aws_rds_iam 97.92% <96.77%> (+0.78%) ⬆️
python_version3.9-sqlalchemy1.4-pydantic1.10-asyncpg-noaws_rds_iam 96.08% <96.77%> (+0.84%) ⬆️
python_version3.9-sqlalchemy1.4-pydantic1.10-noasyncpg-aws_rds_iam 74.88% <22.58%> (-3.93%) ⬇️
python_version3.9-sqlalchemy1.4-pydantic1.10-noasyncpg-noaws_rds_iam 73.04% <22.58%> (-3.87%) ⬇️
python_version3.9-sqlalchemy1.4-pydantic2.0-asyncpg-aws_rds_iam 97.92% <96.77%> (+0.78%) ⬆️
python_version3.9-sqlalchemy1.4-pydantic2.0-asyncpg-noaws_rds_iam 96.08% <96.77%> (+0.84%) ⬆️
python_version3.9-sqlalchemy1.4-pydantic2.0-noasyncpg-aws_rds_iam 74.88% <22.58%> (-3.93%) ⬇️
python_version3.9-sqlalchemy1.4-pydantic2.0-noasyncpg-noaws_rds_iam 73.04% <22.58%> (-3.87%) ⬇️
python_version3.9-sqlalchemy1.4-pydantic2.1-asyncpg-aws_rds_iam 97.92% <96.77%> (+0.78%) ⬆️
python_version3.9-sqlalchemy1.4-pydantic2.1-asyncpg-noaws_rds_iam 96.08% <96.77%> (+0.84%) ⬆️
python_version3.9-sqlalchemy1.4-pydantic2.1-noasyncpg-aws_rds_iam 74.88% <22.58%> (-3.93%) ⬇️
python_version3.9-sqlalchemy1.4-pydantic2.1-noasyncpg-noaws_rds_iam 73.04% <22.58%> (-3.87%) ⬇️
python_version3.9-sqlalchemy2.0-pydantic1.10-asyncpg-aws_rds_iam 97.23% <96.77%> (+0.80%) ⬆️
python_version3.9-sqlalchemy2.0-pydantic1.10-asyncpg-noaws_rds_iam 95.39% <96.77%> (+0.86%) ⬆️
python_version3.9-sqlalchemy2.0-pydantic1.10-noasyncpg-aws_rds_iam 74.19% <22.58%> (-3.91%) ⬇️
python_version3.9-sqlalchemy2.0-pydantic1.10-noasyncpg-noaws_rds_iam 72.35% <22.58%> (-3.85%) ⬇️
python_version3.9-sqlalchemy2.0-pydantic2.0-asyncpg-aws_rds_iam 97.23% <96.77%> (+0.80%) ⬆️
python_version3.9-sqlalchemy2.0-pydantic2.0-asyncpg-noaws_rds_iam 95.39% <96.77%> (+0.86%) ⬆️
python_version3.9-sqlalchemy2.0-pydantic2.0-noasyncpg-aws_rds_iam 74.19% <22.58%> (-3.91%) ⬇️
python_version3.9-sqlalchemy2.0-pydantic2.0-noasyncpg-noaws_rds_iam 72.35% <22.58%> (-3.85%) ⬇️
python_version3.9-sqlalchemy2.0-pydantic2.1-asyncpg-aws_rds_iam 97.23% <96.77%> (+0.80%) ⬆️
python_version3.9-sqlalchemy2.0-pydantic2.1-asyncpg-noaws_rds_iam 95.39% <96.77%> (+0.86%) ⬆️
python_version3.9-sqlalchemy2.0-pydantic2.1-noasyncpg-aws_rds_iam 74.19% <22.58%> (-3.91%) ⬇️
python_version3.9-sqlalchemy2.0-pydantic2.1-noasyncpg-noaws_rds_iam 72.35% <22.58%> (-3.85%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
fastapi_sqla/_pytest_plugin.py 100.00% <100.00%> (ø)
fastapi_sqla/async_sqla.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

from fastapi_sqla.sqla import _Session

if "dont_patch_engines" in request.keywords:
if "dont_patch_engines" in request.keywords: # pragma: no cover
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We weren't actually testing it, and it was flagged as not covered after I cleaned up test_startup. I don't think it should be that PR responsibility to add coverage for this case in this fixture

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤔 weird that it was flagged as not covered as that marker is used in tests.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(Anyway, agree it is not the responsibility of current PR to cover this)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤔 weird that it was flagged as not covered as that marker is used in tests.

It was previously covered in test_startup, but since we were setting the mark.usefixtures("patch_engine_from_config", "patch_new_engine") at the top of the file, but then for every test we were using @mark.dont_patch_engines, we weren't actually testing anything in terms of what the expected behavior is.

@arththebird arththebird merged commit c2d73f6 into master Nov 6, 2023
@arththebird arththebird deleted the fix/DIA-61984/test-refactor branch November 6, 2023 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants