Skip to content

Commit

Permalink
Improve selectiveness of selective checks for "always/tests" case (ap…
Browse files Browse the repository at this point in the history
…ache#35458)

When we decide which tests should be run we err on a safe side and
when we see that some files were modified that we cannot classify
to specific providers, API, CLI, WWW area, we err on a safe side
and assume that we should run all tests (This is called a
"core/other modified" case. The assumption here is that modificiation
of any of the core code or some of the auxiliary utils might affect
everyone else.

However - we can safely assume that if only "tests/always" files
have been modified, then we can remove them from the list - because
those tests will anyhow will be executed and changing those tests
should have no impact on other tests. Those tests are ALWAYS executed.
iThis will - in some cases - avoid running full test suite when
we really only run a small subset of those.

This was the case for apache#35457 where it run full test suite, but really
only "Always Providers[common.io]" were needed.
  • Loading branch information
potiuk authored and romsharon98 committed Nov 10, 2023
1 parent 831fe7d commit 5709b64
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 4 deletions.
14 changes: 12 additions & 2 deletions dev/breeze/src/airflow_breeze/utils/selective_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ class FileGroupForCi(Enum):
ENVIRONMENT_FILES = "environment_files"
PYTHON_PRODUCTION_FILES = "python_scans"
JAVASCRIPT_PRODUCTION_FILES = "javascript_scans"
ALWAYS_TESTS_FILES = "always_test_files"
API_TEST_FILES = "api_test_files"
API_CODEGEN_FILES = "api_codegen_files"
HELM_FILES = "helm_files"
Expand Down Expand Up @@ -176,6 +177,9 @@ def __hash__(self):
FileGroupForCi.SYSTEM_TEST_FILES: [
r"^tests/system/",
],
FileGroupForCi.ALWAYS_TESTS_FILES: [
r"^tests/always/",
],
}
)

Expand Down Expand Up @@ -613,11 +617,17 @@ def _get_test_types_to_run(self) -> list[str]:
kubernetes_files = self._matching_files(FileGroupForCi.KUBERNETES_FILES, CI_FILE_GROUP_MATCHES)
system_test_files = self._matching_files(FileGroupForCi.SYSTEM_TEST_FILES, CI_FILE_GROUP_MATCHES)
all_source_files = self._matching_files(FileGroupForCi.ALL_SOURCE_FILES, CI_FILE_GROUP_MATCHES)

test_always_files = self._matching_files(FileGroupForCi.ALWAYS_TESTS_FILES, CI_FILE_GROUP_MATCHES)
remaining_files = (
set(all_source_files) - set(matched_files) - set(kubernetes_files) - set(system_test_files)
set(all_source_files)
- set(matched_files)
- set(kubernetes_files)
- set(system_test_files)
- set(test_always_files)
)
get_console().print(f"[warning]Remaining non test/always files: {len(remaining_files)}[/]")
count_remaining_files = len(remaining_files)

if count_remaining_files > 0:
get_console().print(
f"[warning]We should run all tests. There are {count_remaining_files} changed "
Expand Down
28 changes: 26 additions & 2 deletions dev/breeze/tests/test_selective_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,30 @@ def assert_outputs_are_printed(expected_outputs: dict[str, str], stderr: str):
},
id="Providers tests run including amazon tests if amazon provider files changed",
),
pytest.param(
(
"tests/always/test_project_structure.py",
"tests/providers/common/io/operators/__init__.py",
"tests/providers/common/io/operators/test_file_transfer.py",
),
{
"affected-providers-list-as-string": "common.io",
"all-python-versions": "['3.8']",
"all-python-versions-list-as-string": "3.8",
"python-versions": "['3.8']",
"python-versions-list-as-string": "3.8",
"ci-image-build": "true",
"prod-image-build": "false",
"needs-helm-tests": "false",
"run-tests": "true",
"run-amazon-tests": "false",
"docs-build": "false",
"run-kubernetes-tests": "false",
"upgrade-to-newer-dependencies": "false",
"parallel-test-types-list-as-string": "Always Providers[common.io]",
},
id="Only Always and Common.IO tests should run when only common.io and tests/always changed",
),
],
)
def test_expected_output_pull_request_main(
Expand Down Expand Up @@ -676,7 +700,7 @@ def test_expected_output_full_tests_needed(
),
],
)
def test_expected_output_pull_request_v2_3(
def test_expected_output_pull_request_v2_7(
files: tuple[str, ...],
expected_outputs: dict[str, str],
):
Expand All @@ -685,7 +709,7 @@ def test_expected_output_pull_request_v2_3(
commit_ref="HEAD",
github_event=GithubEvents.PULL_REQUEST,
pr_labels=(),
default_branch="v2-3-stable",
default_branch="v2-7-stable",
)
assert_outputs_are_printed(expected_outputs, str(stderr))

Expand Down

0 comments on commit 5709b64

Please sign in to comment.