john-bodley opened a new pull request #14366: URL: https://github.com/apache/superset/pull/14366
### SUMMARY Yak shaving at its finest. This PR started due to the fact that some of the database engine specs had the following import logic, ```python from superset import app ``` which was very problematic due to potential circular imports and app contexts which needed to be invoked. For testing we required magical logic like ```python from tests.test_app import app ``` prior to importing the actual engine spec. This change ensures that the engine specs are decoupled from the Flask app and use the `flask.current_app` where necessary when the application configuration is required. Note what started out as somewhat of a benign change quickly spiraled out of control in order to ensure that the tests passed as more and more code needed to be touched. The TL;DR is: - Even though this logic could further be improved, in my opinion the approach is in the direction of the north star, i.e., removing the dependency for `tests.test_app` and using the `app` `pytest` fixture instead. - The `app` fixture is scoped to the function which ensures that the tests are idempotent and thus the `app.config` neither i) needs to be mocked, or ii) reset upon completion of the test. - Migrates a subset of tests to be more `pytest` like so we can leverage the `app` fixture. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF <!--- Skip this if not applicable --> ### TEST PLAN <!--- What steps should be taken to verify the changes --> ### ADDITIONAL INFORMATION <!--- Check any relevant boxes with "x" --> <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue --> - [ ] Has associated issue: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [ ] Introduces new feature or API - [ ] Removes existing feature or API -- 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. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
