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 2022/06/26 13:12:34 UTC
[GitHub] [airflow] potiuk opened a new pull request, #24666: Add more selective provider tests
potiuk opened a new pull request, #24666:
URL: https://github.com/apache/airflow/pull/24666
After implementing #24610 and few follow-up fixes, it is now easy
to add more optimizations to our unit test execution in CI (and
to give this capability back to our contributors).
This PR adds capability of running tests for selected set of
providers - not for the whole "Providers" group. You can
specify `--test-type "Providers[airbyte,http]" to only run tests
for the two selected providers.
This is the step towards separating providers to separate
repositories, but it also allows to optimize the experience of
the contributors developing only single provider changes (which
is vast majority of contributions).
This also allows to optimize build and elapsed time needd to run
tests for those PRs that only affects selected providers (again -
vast majority of PRs).
The CI selection of which provider tests is done now in Selective
Checkcs - they are a bit smarter in just selecting the providers
that has been changed, they also check if there are any other
providers that depend on it (we keep automatically updated by
pre-commit dependencies.json file and this file determines
which files should be run.
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragement file, named `{pr_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham commented on a diff in pull request #24666: Add more selective provider tests
Posted by GitBox <gi...@apache.org>.
jedcunningham commented on code in PR #24666:
URL: https://github.com/apache/airflow/pull/24666#discussion_r914990030
##########
BREEZE.rst:
##########
@@ -524,8 +522,53 @@ Additional management tasks:
Tests
-----
-* Run docker-compose tests with ``breeze docker-compose-tests`` command.
-* Run test specified with ``breeze tests`` command.
+You can regular unit tests with ``breeze`` in two different ways, either interactively run tests with
+the default ``shell`` command or via the ``tests`` command.
+
+Iterate on tests interactively
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+You can simply enter the ``breeze`` container and run ``pytest`` command there. You can enter the
+container via just ``breeze`` command or ``breeze shell`` command (the latter has more options
+useful when you run integration or system tests). This is the best way if you want to interactively
+run selected tests and iterate with the tests. Once you enter ``breeze`` environment it is ready
+out-of-the-box to run your tests by running the right ``pytest`` command (autocomplete should help
+you with autocompleting test name if you start typing ``pytest tests<TAB>``).
+
+Here are few examples:
+
+Running single test:
+
+.. code-block:: bash
+
+ pytest tests/core/test_core.py::TestCore::test_check_operators
+
+To run the whole test class:
+
+.. code-block:: bash
+
+ pytest tests/core/test_core.py::TestCore
+
+You can re-run the tests interactively, add extra parameters to pytest and modify the files before
+re-running the test to iterate over the tests. You can also add more flags when starting the
+``breeze shell`` command when you run integration tests or system tests. Read more details about it
+in the ``TESTING.rst <TESTING.rst#>`` where all the test types of our are explained and more information
+on how to run them.
+
+Running group of tests
+~~~~~~~~~~~~~~~~~~~~~~
+
+You can also run tests via built-in ``breeze tests`` command - similarly as iterative ``pytest`` command
+allows to run test individually, or by class or in any other way pytest allows to test them, but it
+also allows to run the tests in the same test "types" that are used to run the tests in CI: Core, Always
+API, Providers. This how our Ci runs them - running each group in parallel to other groups and you can
Review Comment:
```suggestion
API, Providers. This how our CI runs them - running each group in parallel to other groups and you can
```
##########
TESTING.rst:
##########
@@ -188,6 +189,20 @@ You can also limit the tests to execute to specific group of tests
breeze tests --test-type Core
+In case of Providers tests, you can run tests for all providers
+
+.. code-block:: bash
+
+ breeze tests --test-type Providers
+
+You can also limit the set of providers you would like to run tests of
+
+In case of Providers tests, you can run tests for all providers
+
Review Comment:
```suggestion
```
##########
BREEZE.rst:
##########
@@ -524,8 +522,53 @@ Additional management tasks:
Tests
-----
-* Run docker-compose tests with ``breeze docker-compose-tests`` command.
-* Run test specified with ``breeze tests`` command.
+You can regular unit tests with ``breeze`` in two different ways, either interactively run tests with
+the default ``shell`` command or via the ``tests`` command.
+
+Iterate on tests interactively
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+You can simply enter the ``breeze`` container and run ``pytest`` command there. You can enter the
+container via just ``breeze`` command or ``breeze shell`` command (the latter has more options
+useful when you run integration or system tests). This is the best way if you want to interactively
+run selected tests and iterate with the tests. Once you enter ``breeze`` environment it is ready
+out-of-the-box to run your tests by running the right ``pytest`` command (autocomplete should help
+you with autocompleting test name if you start typing ``pytest tests<TAB>``).
+
+Here are few examples:
+
+Running single test:
+
+.. code-block:: bash
+
+ pytest tests/core/test_core.py::TestCore::test_check_operators
+
+To run the whole test class:
+
+.. code-block:: bash
+
+ pytest tests/core/test_core.py::TestCore
+
+You can re-run the tests interactively, add extra parameters to pytest and modify the files before
+re-running the test to iterate over the tests. You can also add more flags when starting the
+``breeze shell`` command when you run integration tests or system tests. Read more details about it
+in the ``TESTING.rst <TESTING.rst#>`` where all the test types of our are explained and more information
+on how to run them.
+
+Running group of tests
+~~~~~~~~~~~~~~~~~~~~~~
+
+You can also run tests via built-in ``breeze tests`` command - similarly as iterative ``pytest`` command
+allows to run test individually, or by class or in any other way pytest allows to test them, but it
+also allows to run the tests in the same test "types" that are used to run the tests in CI: Core, Always
+API, Providers. This how our Ci runs them - running each group in parallel to other groups and you can
+replicate this behaviour.
+
+Another interesting use of the ``breeze tests`` command is that you can easily specify sub-set of the
+tests for Providers. ``breeze tests --test-type "Providers[airbyte,http]" for example will only run
Review Comment:
```suggestion
tests for Providers. ``breeze tests --test-type "Providers[airbyte,http]`` for example will only run
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #24666: Add more selective provider tests
Posted by GitBox <gi...@apache.org>.
potiuk merged PR #24666:
URL: https://github.com/apache/airflow/pull/24666
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a diff in pull request #24666: Add more selective provider tests
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #24666:
URL: https://github.com/apache/airflow/pull/24666#discussion_r907216198
##########
dev/breeze/src/airflow_breeze/utils/selective_checks.py:
##########
@@ -176,11 +185,64 @@ def __hash__(self):
SelectiveUnitTestTypes.PROVIDERS: [
"^airflow/providers/",
"^tests/providers/",
+ "^tests/system/",
],
SelectiveUnitTestTypes.WWW: ["^airflow/www", "^tests/www", "^airflow/ui"],
}
)
+TESTS_PROVIDERS_ROOT = AIRFLOW_SOURCES_ROOT / "tests" / "providers"
+SYSTEM_TESTS_PROVIDERS_ROOT = AIRFLOW_SOURCES_ROOT / "tests" / "system" / "providers"
+AIRFLOW_PROVIDERS_ROOT = AIRFLOW_SOURCES_ROOT / "airflow" / "providers"
+
+
+def find_provider_affected(changed_file: str) -> str | None:
+ file_path = AIRFLOW_SOURCES_ROOT / changed_file
+ # is_relative_to is only available in Python 3.9 - we should simplify this check when we are Python 3.9+
+ try:
+ file_path.relative_to(TESTS_PROVIDERS_ROOT)
+ relative_base_path = TESTS_PROVIDERS_ROOT
+ except ValueError:
+ try:
+ file_path.relative_to(SYSTEM_TESTS_PROVIDERS_ROOT)
+ relative_base_path = SYSTEM_TESTS_PROVIDERS_ROOT
+ except ValueError:
+ try:
+ file_path.relative_to(AIRFLOW_PROVIDERS_ROOT)
+ relative_base_path = AIRFLOW_PROVIDERS_ROOT
+ except ValueError:
+ return None
Review Comment:
BTW. TIL. I had NO IDEA for loop has an else clause :)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #24666: Add more selective provider tests
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #24666:
URL: https://github.com/apache/airflow/pull/24666#issuecomment-1176402240
Nice I added tests :). In the meantime I changed the provider_dependencies.json structure and the tests nicely caught it :)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bhirsz commented on a diff in pull request #24666: Add more selective provider tests
Posted by GitBox <gi...@apache.org>.
bhirsz commented on code in PR #24666:
URL: https://github.com/apache/airflow/pull/24666#discussion_r907043729
##########
dev/breeze/src/airflow_breeze/utils/selective_checks.py:
##########
@@ -176,11 +185,64 @@ def __hash__(self):
SelectiveUnitTestTypes.PROVIDERS: [
"^airflow/providers/",
"^tests/providers/",
+ "^tests/system/",
],
SelectiveUnitTestTypes.WWW: ["^airflow/www", "^tests/www", "^airflow/ui"],
}
)
+TESTS_PROVIDERS_ROOT = AIRFLOW_SOURCES_ROOT / "tests" / "providers"
+SYSTEM_TESTS_PROVIDERS_ROOT = AIRFLOW_SOURCES_ROOT / "tests" / "system" / "providers"
+AIRFLOW_PROVIDERS_ROOT = AIRFLOW_SOURCES_ROOT / "airflow" / "providers"
+
+
+def find_provider_affected(changed_file: str) -> str | None:
+ file_path = AIRFLOW_SOURCES_ROOT / changed_file
+ # is_relative_to is only available in Python 3.9 - we should simplify this check when we are Python 3.9+
+ try:
+ file_path.relative_to(TESTS_PROVIDERS_ROOT)
+ relative_base_path = TESTS_PROVIDERS_ROOT
+ except ValueError:
+ try:
+ file_path.relative_to(SYSTEM_TESTS_PROVIDERS_ROOT)
+ relative_base_path = SYSTEM_TESTS_PROVIDERS_ROOT
+ except ValueError:
+ try:
+ file_path.relative_to(AIRFLOW_PROVIDERS_ROOT)
+ relative_base_path = AIRFLOW_PROVIDERS_ROOT
+ except ValueError:
+ return None
Review Comment:
Other way of writing this
```suggestion
for provider_root in (TESTS_PROVIDERS_ROOT, SYSTEM_TESTS_PROVIDERS_ROOT, AIRFLOW_PROVIDERS_ROOT):
try:
file_path.relative_to(provider_root )
relative_base_path = provider_root
break
except ValueError:
pass
else:
return None
```
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #24666: Add more selective provider tests
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #24666:
URL: https://github.com/apache/airflow/pull/24666#issuecomment-1174402678
Anyone ? This should speed up the tests in most cases and adds a way how to run tests per provider (will be useful in the future when we split providers).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #24666: Add more selective provider tests
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #24666:
URL: https://github.com/apache/airflow/pull/24666#issuecomment-1167194684
BTW. This one is going to help quite a bit with the elapsed time neeed for "localized" PRs to providers :) and together with #24671 it will be even more localised as any changes to provider including it's dependencies will limit tests to only this provider, they won't need the "core" tests at all.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #24666: Add more selective provider tests
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #24666:
URL: https://github.com/apache/airflow/pull/24666#issuecomment-1167161582
Resolved @bhirsz :). Thanks for feedback. The for/else clause made my day.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bhirsz commented on a diff in pull request #24666: Add more selective provider tests
Posted by GitBox <gi...@apache.org>.
bhirsz commented on code in PR #24666:
URL: https://github.com/apache/airflow/pull/24666#discussion_r907040466
##########
dev/breeze/src/airflow_breeze/commands/testing_commands.py:
##########
@@ -272,6 +273,9 @@ def tests(
os.environ["RUN_TESTS"] = "true"
if test_type:
os.environ["TEST_TYPE"] = test_type
+ if "[" and not test_type.startswith("Providers"):
Review Comment:
``if "["``? looks like a typo
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a diff in pull request #24666: Add more selective provider tests
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #24666:
URL: https://github.com/apache/airflow/pull/24666#discussion_r907095020
##########
dev/breeze/src/airflow_breeze/utils/selective_checks.py:
##########
@@ -176,11 +185,64 @@ def __hash__(self):
SelectiveUnitTestTypes.PROVIDERS: [
"^airflow/providers/",
"^tests/providers/",
+ "^tests/system/",
],
SelectiveUnitTestTypes.WWW: ["^airflow/www", "^tests/www", "^airflow/ui"],
}
)
+TESTS_PROVIDERS_ROOT = AIRFLOW_SOURCES_ROOT / "tests" / "providers"
+SYSTEM_TESTS_PROVIDERS_ROOT = AIRFLOW_SOURCES_ROOT / "tests" / "system" / "providers"
+AIRFLOW_PROVIDERS_ROOT = AIRFLOW_SOURCES_ROOT / "airflow" / "providers"
+
+
+def find_provider_affected(changed_file: str) -> str | None:
+ file_path = AIRFLOW_SOURCES_ROOT / changed_file
+ # is_relative_to is only available in Python 3.9 - we should simplify this check when we are Python 3.9+
+ try:
+ file_path.relative_to(TESTS_PROVIDERS_ROOT)
+ relative_base_path = TESTS_PROVIDERS_ROOT
+ except ValueError:
+ try:
+ file_path.relative_to(SYSTEM_TESTS_PROVIDERS_ROOT)
+ relative_base_path = SYSTEM_TESTS_PROVIDERS_ROOT
+ except ValueError:
+ try:
+ file_path.relative_to(AIRFLOW_PROVIDERS_ROOT)
+ relative_base_path = AIRFLOW_PROVIDERS_ROOT
+ except ValueError:
+ return None
Review Comment:
Ah yeah. Much nicer. It's a pity by the way that "is_relative_to" is only available in Python 3.9+ :(
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on a diff in pull request #24666: Add more selective provider tests
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #24666:
URL: https://github.com/apache/airflow/pull/24666#discussion_r907093908
##########
dev/breeze/src/airflow_breeze/commands/testing_commands.py:
##########
@@ -272,6 +273,9 @@ def tests(
os.environ["RUN_TESTS"] = "true"
if test_type:
os.environ["TEST_TYPE"] = test_type
+ if "[" and not test_type.startswith("Providers"):
Review Comment:
Yep. It was supposed to be "safety check" so that "[" is only used in providers :)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org