uranusjr commented on a change in pull request #16889:
URL: https://github.com/apache/airflow/pull/16889#discussion_r672920031
##########
File path: tests/jobs/test_local_task_job.py
##########
@@ -55,45 +54,35 @@
TEST_DAG_FOLDER = os.environ['AIRFLOW__CORE__DAGS_FOLDER']
-class TestLocalTaskJob(unittest.TestCase):
- def setUp(self):
- db.clear_db_dags()
- db.clear_db_jobs()
- db.clear_db_runs()
- db.clear_db_task_fail()
- patcher = patch('airflow.jobs.base_job.sleep')
- self.addCleanup(patcher.stop)
- self.mock_base_job_sleep = patcher.start()
-
- def tearDown(self) -> None:
+class TestLocalTaskJob:
+ @pytest.fixture(autouse=True)
+ def clean_db_and_set_instance_attrs(self):
db.clear_db_dags()
db.clear_db_jobs()
db.clear_db_runs()
db.clear_db_task_fail()
Review comment:
Yes but you only use the `clear_db` on the class. I’m not sure tbh what
that actually does (pytest documentation is quite sparse on this), but I assume
it either applies the fixture to be run for every test in the class, or once
for that class. Which means you wouldn’t be running the fixture for all of the
cases listed above.
##########
File path: tests/jobs/test_local_task_job.py
##########
@@ -55,45 +54,35 @@
TEST_DAG_FOLDER = os.environ['AIRFLOW__CORE__DAGS_FOLDER']
-class TestLocalTaskJob(unittest.TestCase):
- def setUp(self):
- db.clear_db_dags()
- db.clear_db_jobs()
- db.clear_db_runs()
- db.clear_db_task_fail()
- patcher = patch('airflow.jobs.base_job.sleep')
- self.addCleanup(patcher.stop)
- self.mock_base_job_sleep = patcher.start()
-
- def tearDown(self) -> None:
+class TestLocalTaskJob:
+ @pytest.fixture(autouse=True)
+ def clean_db_and_set_instance_attrs(self):
db.clear_db_dags()
db.clear_db_jobs()
db.clear_db_runs()
db.clear_db_task_fail()
Review comment:
Just tried, it seems that `usefixtures()` on a class is the same as
marking `usefixtures()` on every test in the class. So we are missing a
class-level teardown after all tests on the tests are run.
##########
File path: tests/jobs/test_local_task_job.py
##########
@@ -55,45 +54,35 @@
TEST_DAG_FOLDER = os.environ['AIRFLOW__CORE__DAGS_FOLDER']
-class TestLocalTaskJob(unittest.TestCase):
- def setUp(self):
- db.clear_db_dags()
- db.clear_db_jobs()
- db.clear_db_runs()
- db.clear_db_task_fail()
- patcher = patch('airflow.jobs.base_job.sleep')
- self.addCleanup(patcher.stop)
- self.mock_base_job_sleep = patcher.start()
-
- def tearDown(self) -> None:
+class TestLocalTaskJob:
+ @pytest.fixture(autouse=True)
+ def clean_db_and_set_instance_attrs(self):
db.clear_db_dags()
db.clear_db_jobs()
db.clear_db_runs()
db.clear_db_task_fail()
Review comment:
The fixture is defined like this
```python
@pytest.fixture
def clear_db():
db.clear_db_dags()
db.clear_db_jobs()
db.clear_db_runs()
db.clear_db_task_fail()
yield
```
The db clear calls are done in the fixture’s *setup* phase (before yield),
so after the last test runs, the fixture itself is destroyed, but that destroy
phase does not clear db. So I think we need one more fixture:
```
@pytest.fixture(scope="class")
def clear_db_class():
yield
db.clear_db_dags()
db.clear_db_jobs()
db.clear_db_runs()
db.clear_db_task_fail()
```
to handle that?
##########
File path: tests/jobs/test_local_task_job.py
##########
@@ -55,45 +54,35 @@
TEST_DAG_FOLDER = os.environ['AIRFLOW__CORE__DAGS_FOLDER']
-class TestLocalTaskJob(unittest.TestCase):
- def setUp(self):
- db.clear_db_dags()
- db.clear_db_jobs()
- db.clear_db_runs()
- db.clear_db_task_fail()
- patcher = patch('airflow.jobs.base_job.sleep')
- self.addCleanup(patcher.stop)
- self.mock_base_job_sleep = patcher.start()
-
- def tearDown(self) -> None:
+class TestLocalTaskJob:
+ @pytest.fixture(autouse=True)
+ def clean_db_and_set_instance_attrs(self):
db.clear_db_dags()
db.clear_db_jobs()
db.clear_db_runs()
db.clear_db_task_fail()
Review comment:
The fixture is defined like this
```python
@pytest.fixture
def clear_db():
db.clear_db_dags()
db.clear_db_jobs()
db.clear_db_runs()
db.clear_db_task_fail()
yield
```
The db clear calls are done in the fixture’s *setup* phase (before yield),
so after the last test runs, the fixture itself is destroyed, but that destroy
phase does not clear db. So I think we need one more fixture:
```python
@pytest.fixture(scope="class")
def clear_db_class():
yield
db.clear_db_dags()
db.clear_db_jobs()
db.clear_db_runs()
db.clear_db_task_fail()
```
to handle that?
--
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]
