Status
Motivation
Airflow currently uses nose test framework which is rather an old package. Moreover its documentation does not belong to the best ones and its features are rather limited. This includes configuration of the test suite as well as formatting test output.
On the other hand pytest is one of the most popular python test framework (two times more usages than nose according to github stats). It also provides very good and extensive documentation. Pytest can be configured using an .ini file that allows to keep whole configuration in one place. Moreover pytest comes with two powerful features:
- Fixtures - “The purpose of test fixtures is to provide a fixed baseline upon which tests can reliably and repeatedly execute.” https://docs.pytest.org/en/latest/fixture.html
- Marks - a simple way to marks test. This could be used to mark tests as flaky, system test, gcp test, aws test etc. http://doc.pytest.org/en/latest/example/markers.html
Additionally pytest has a lot of plugins.
Considerations
To understand how much work migration will required I've created PoC branch (https://github.com/PolideaInternal/airflow/pull/343). As a result I obtained a build that results in: 15 failed, 3592 passed, 83 skipped, 1 warnings in 1181.60s (0:19:41). This required to run two tests:
- tests/jobs/test_scheduler_job.py
- tests/utils/test_dag_processing.py
before any others because otherwise those tests fail and build timeouts. And to suppress number of errors I have to skip
- tests/test_logging_config.py::TestLoggingSettings::test_loading_remote_logging_with_wasb_handler
tests/dags/test_dag_serialization.py (but I think it's not run on actual master)
The first one creates breaking side effects and the second one needs changes to not fail.
What change do you propose to make?
Use pytest instead of nose for running Airflow test suite.
We should also consider if we want migrate from unittests way of writing tests to pytest-like approach. This change means we will have to abandon subclassing unittests.TestCase and using pytest's fixtures for setUp and tearDown.
What problem does it solve?
During the migration Airflow tests will be improved and some side effects will be gone.
Are there any downsides to this change?
There should not be any problems.
Which users are affected by the change?
The change will only affect Airflow developers.
How are users affected by the change?
Users will not be affected by this change.
What defines this AIP as "done"?
To migrate the following steps are required:
- use pytest for running test on CI. This will required few improvements to our test.
- rewrite tests to be pytest-like (if agreed to do so).
Other considerations?
If we agree that we want to start writing our test in pytest-like way then we may use https://pypi.org/project/unittest2pytest/. For the transition moment it's worth to consider same approach as we already have with pylint: pre-commit hook check + TODO list with files that are not yet migrated. There is also a case of parameterised test that seems to be interesting for developers. Pytest allow to run a single parameterise test method. However, this is not possible when subclassing unittests.TestCase.
Other ideas worth considering:
- use reset_environment fixture for all test to assure in a quick and simple way that the tests do not relay on environment variables set by other tests.
- in future we can use pytest-xdist for faster, distributed running of tests: https://github.com/pytest-dev/pytest-xdist
- we can use pytests.mark to mark for system tests. This would simplify building custom CI builds that run selected tests: http://doc.pytest.org/en/latest/example/markers.html
- using flaky to mark flaky test to run more than once: https://github.com/box/flaky
3 Comments
Ash Berlin-Taylor
{{tests/dags/test_dag_serialization.py}} is probably not meant to be a test file, but a dag used in testing. We have already fixed that in https://github.com/apache/airflow/pull/5743
Ash Berlin-Taylor
I'd like to hold off on something as large-scale as unittest2pytest until 2.0 is out/until we no longer have to backport changes to the v1-10-* branches.
Tomek Urbaszek
Yes, I think this is a reasonable approach.