john-bodley commented on PR #26909:
URL: https://github.com/apache/superset/pull/26909#issuecomment-1922328534
Thanks @michael-s-molina for the review. I think there's a `pytest` pattern
which can be somewhat confusing related to fixtures. There are numerous tests
which have a signature of the form,
```
def test_foo(session: Session, ...):
...
```
or
```
def test_foo(session_with_data: Session, ...):
...
```
though these are in fact SQLAlchemy sessions (which can be accessed
directly, i.e., `session_with_data.query(...)` they are [pytest
fixtures](https://docs.pytest.org/en/6.2.x/fixture.html) and their primary
purpose in the function signature is for the test to leverage the fixture
(given they're scope is non-global), i,.e., an in-memory session which may
contain various objects
([example](https://github.com/apache/superset/blob/813783382cbbe63c9caf6e8732e94f8d29521b50/tests/unit_tests/dashboards/dao_tests.py#L24-L42))
and thus are still required. These sessions are accessible via the
Flask-SQLAlchemy global session (`db.session`) given that it's been patched
[here](https://github.com/apache/superset/blob/813783382cbbe63c9caf6e8732e94f8d29521b50/tests/unit_tests/conftest.py#L65).
The TL;DR you can think of these fixtures as providing the tests with
pre-loaded ephemeral data which isn't persisted to the database.
Per the PR description I wanted to make it really clear that we should
always be querying the session via the Flask-SQLAlchemy interface. It's a
clearer/simpler pattern in my opinion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
