You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/03/18 11:15:00 UTC

[GitHub] [airflow] uranusjr commented on a change in pull request #14863: Convert tests/www/test_views.py to use Pytest fixtures

uranusjr commented on a change in pull request #14863:
URL: https://github.com/apache/airflow/pull/14863#discussion_r596760905



##########
File path: tests/www/test_views.py
##########
@@ -122,31 +122,39 @@ def local_context(self):
         return result
 
 
-class TestBase(unittest.TestCase):
-    @classmethod
-    @dont_initialize_flask_app_submodules(
-        skip_all_except=[
-            "init_appbuilder",
-            "init_appbuilder_views",
-            "init_flash_views",
-            "init_jinja_globals",
-        ]
-    )
-    def setUpClass(cls):
+class TestBase:
+    @pytest.fixture(scope="class")

Review comment:
       But can we? `minimal_app` is mutated in a lot of test preparations, and making the fixture module-scoped would make those side effects leak out to other tests. For example, I don’t think `init_api_connexion` can be called multiple times on the same app. `TestAirflowBaseViews` also sets `app.dag_bag` which would affect subsequent tests using the same app.
   
   I agree with moving it to `conftest.py` though. I’m keeping things as local as possible for now, and will move it when I start working on other files in `tests/www`.




----------------------------------------------------------------
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:
users@infra.apache.org