-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Optimize parallel test execution for unit tests #30705
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,6 @@ | |
| from __future__ import annotations | ||
|
|
||
| import os | ||
| import re | ||
| import sys | ||
| from datetime import datetime | ||
|
|
||
|
|
@@ -65,12 +64,11 @@ | |
| from airflow_breeze.utils.parallel import ( | ||
| GenericRegexpProgressMatcher, | ||
| SummarizeAfter, | ||
| bytes2human, | ||
| check_async_run_results, | ||
| run_with_pool, | ||
| ) | ||
| from airflow_breeze.utils.path_utils import FILES_DIR, cleanup_python_generated_files | ||
| from airflow_breeze.utils.run_tests import run_docker_compose_tests | ||
| from airflow_breeze.utils.run_tests import file_name_from_test_type, run_docker_compose_tests | ||
| from airflow_breeze.utils.run_utils import get_filesystem_type, run_command | ||
|
|
||
| LOW_MEMORY_CONDITION = 8 * 1024 * 1024 * 1024 | ||
|
|
@@ -143,7 +141,7 @@ def _run_test( | |
| "[error]Only 'Providers' test type can specify actual tests with \\[\\][/]" | ||
| ) | ||
| sys.exit(1) | ||
| project_name = _file_name_from_test_type(exec_shell_params.test_type) | ||
| project_name = file_name_from_test_type(exec_shell_params.test_type) | ||
| down_cmd = [ | ||
| *DOCKER_COMPOSE_COMMAND, | ||
| "--project-name", | ||
|
|
@@ -209,11 +207,6 @@ def _run_test( | |
| return result.returncode, f"Test: {exec_shell_params.test_type}" | ||
|
|
||
|
|
||
| def _file_name_from_test_type(test_type: str): | ||
| test_type_no_brackets = test_type.lower().replace("[", "_").replace("]", "") | ||
| return re.sub("[,\.]", "_", test_type_no_brackets)[:30] | ||
|
|
||
|
|
||
| def _run_tests_in_pool( | ||
| tests_to_run: list[str], | ||
| parallelism: int, | ||
|
|
@@ -268,43 +261,12 @@ def run_tests_in_parallel( | |
| parallel_test_types_list: list[str], | ||
| extra_pytest_args: tuple, | ||
| db_reset: bool, | ||
| full_tests_needed: bool, | ||
| test_timeout: int, | ||
| include_success_outputs: bool, | ||
| debug_resources: bool, | ||
| parallelism: int, | ||
| skip_cleanup: bool, | ||
| ) -> None: | ||
| import psutil | ||
|
|
||
| memory_available = psutil.virtual_memory() | ||
| if memory_available.available < LOW_MEMORY_CONDITION and exec_shell_params.backend in ["mssql", "mysql"]: | ||
| # Run heavy tests sequentially | ||
| heavy_test_types_to_run = {"Core", "Providers"} & set(parallel_test_types_list) | ||
| if heavy_test_types_to_run: | ||
| # some of those are requested | ||
| get_console().print( | ||
| f"[warning]Running {heavy_test_types_to_run} tests sequentially" | ||
| f"for {exec_shell_params.backend}" | ||
| f" backend due to low memory available: {bytes2human(memory_available.available)}" | ||
| ) | ||
| tests_to_run_sequentially = [] | ||
| for heavy_test_type in heavy_test_types_to_run: | ||
| for test_type in parallel_test_types_list: | ||
| if test_type.startswith(heavy_test_type): | ||
| parallel_test_types_list.remove(test_type) | ||
| tests_to_run_sequentially.append(test_type) | ||
| _run_tests_in_pool( | ||
| tests_to_run=tests_to_run_sequentially, | ||
| parallelism=1, | ||
| exec_shell_params=exec_shell_params, | ||
| extra_pytest_args=extra_pytest_args, | ||
| test_timeout=test_timeout, | ||
| db_reset=db_reset, | ||
| include_success_outputs=include_success_outputs, | ||
| debug_resources=debug_resources, | ||
| skip_cleanup=skip_cleanup, | ||
| ) | ||
| _run_tests_in_pool( | ||
| tests_to_run=parallel_test_types_list, | ||
| parallelism=parallelism, | ||
|
|
@@ -336,8 +298,9 @@ def run_tests_in_parallel( | |
| @option_mount_sources | ||
| @click.option( | ||
| "--test-type", | ||
| help="Type of test to run. Note that with Providers, you can also specify which provider " | ||
| 'tests should be run - for example --test-type "Providers[airbyte,http]"', | ||
| help="Type of test to run. With Providers, you can specify tests of which providers " | ||
| "should be run: `Providers[airbyte,http]` or " | ||
| "excluded from the full test suite: `Providers[-amazon,google]`", | ||
| default="All", | ||
| type=NotVerifiedBetterChoice(ALLOWED_TEST_TYPE_CHOICES), | ||
| ) | ||
|
|
@@ -361,12 +324,6 @@ def run_tests_in_parallel( | |
| show_default=True, | ||
| envvar="PARALLEL_TEST_TYPES", | ||
| ) | ||
| @click.option( | ||
| "--full-tests-needed", | ||
| help="Whether full set of tests is run.", | ||
| is_flag=True, | ||
| envvar="FULL_TESTS_NEEDED", | ||
| ) | ||
| @click.option( | ||
| "--upgrade-boto", | ||
| help="Remove aiobotocore and upgrade botocore and boto to the latest version.", | ||
|
|
@@ -405,7 +362,6 @@ def command_for_tests( | |
| debug_resources: bool, | ||
| include_success_outputs: bool, | ||
| parallel_test_types: str, | ||
| full_tests_needed: bool, | ||
| mount_sources: str, | ||
| extra_pytest_args: tuple, | ||
| upgrade_boto: bool, | ||
|
|
@@ -434,16 +390,11 @@ def command_for_tests( | |
| perform_environment_checks() | ||
| if run_in_parallel: | ||
| test_list = parallel_test_types.split(" ") | ||
| test_list.sort(key=lambda x: x in ["Providers", "WWW"], reverse=True) | ||
| run_tests_in_parallel( | ||
| exec_shell_params=exec_shell_params, | ||
| parallel_test_types_list=test_list, | ||
| extra_pytest_args=extra_pytest_args, | ||
| db_reset=db_reset, | ||
| # Allow to pass information on whether to use full tests in the parallel execution mode | ||
|
||
| # or not - this will allow to skip some heavy tests on more resource-heavy configurations | ||
| # in case full tests are not required, some of those will be skipped | ||
| full_tests_needed=full_tests_needed, | ||
| test_timeout=test_timeout, | ||
| include_success_outputs=include_success_outputs, | ||
| parallelism=parallelism, | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,7 +46,6 @@ | |
| "--skip-cleanup", | ||
| "--debug-resources", | ||
| "--include-success-outputs", | ||
| "--full-tests-needed", | ||
| ], | ||
| }, | ||
| { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,6 +47,7 @@ | |
| MSSQL_TMP_DIR_NAME, | ||
| SCRIPTS_CI_DIR, | ||
| ) | ||
| from airflow_breeze.utils.run_tests import file_name_from_test_type | ||
| from airflow_breeze.utils.run_utils import get_filesystem_type, run_command | ||
| from airflow_breeze.utils.shared_options import get_verbose | ||
|
|
||
|
|
@@ -267,9 +268,13 @@ def command_passed(self): | |
| @property | ||
| def mssql_data_volume(self) -> str: | ||
| docker_filesystem = get_filesystem_type("/var/lib/docker") | ||
| # in case of Providers[....], only leave Providers | ||
| base_test_type = self.test_type.split("[")[0] if self.test_type else None | ||
| volume_name = f"tmp-mssql-volume-{base_test_type}" if base_test_type else "tmp-mssql-volume" | ||
| # Make sure the test type is not too long to be used as a volume name in docker-compose | ||
|
||
| # The tmp directory in our self-hosted runners can be quite long, so we should limit the volume name | ||
| volume_name = ( | ||
| "tmp-mssql-volume-" + file_name_from_test_type(self.test_type)[:20] | ||
| if self.test_type | ||
| else "tmp-mssql-volume" | ||
| ) | ||
| if docker_filesystem == "tmpfs": | ||
| return os.fspath(Path.home() / MSSQL_TMP_DIR_NAME / f"{volume_name}-{self.mssql_version}") | ||
| else: | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can now remote this whole part - each test is "smaller" so the memory issues we experienced in the past should be far less frequent.