You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by as...@apache.org on 2025/09/01 13:59:47 UTC
(airflow) 02/03: Remove check for caplog in tests
This is an automated email from the ASF dual-hosted git repository.
ash pushed a commit to branch structlog-in-logging-mixin
in repository https://gitbox.apache.org/repos/asf/airflow.git
commit dddc15357930ebbc8e89f22ec5747cd0d7dffe4a
Author: Ash Berlin-Taylor <as...@apache.org>
AuthorDate: Tue Jul 1 17:08:06 2025 +0100
Remove check for caplog in tests
The reason for not using caplog has gone away with the switch to structlog,
and we already override the builtin `caplog` fixture to use our structlog
version
---
.../src/airflow_breeze/utils/selective_checks.py | 75 -------
dev/breeze/tests/test_selective_checks.py | 229 +--------------------
2 files changed, 4 insertions(+), 300 deletions(-)
diff --git a/dev/breeze/src/airflow_breeze/utils/selective_checks.py b/dev/breeze/src/airflow_breeze/utils/selective_checks.py
index fdfa6bbf384..265d2d4c7da 100644
--- a/dev/breeze/src/airflow_breeze/utils/selective_checks.py
+++ b/dev/breeze/src/airflow_breeze/utils/selective_checks.py
@@ -81,7 +81,6 @@ FORCE_PIP_LABEL = "force pip"
FULL_TESTS_NEEDED_LABEL = "full tests needed"
INCLUDE_SUCCESS_OUTPUTS_LABEL = "include success outputs"
LATEST_VERSIONS_ONLY_LABEL = "latest versions only"
-LOG_WITHOUT_MOCK_IN_TESTS_EXCEPTION_LABEL = "log exception"
NON_COMMITTER_BUILD_LABEL = "non committer build"
UPGRADE_TO_NEWER_DEPENDENCIES_LABEL = "upgrade to newer dependencies"
USE_PUBLIC_RUNNERS_LABEL = "use public runners"
@@ -1437,80 +1436,6 @@ class SelectiveChecks:
and self._github_repository == APACHE_AIRFLOW_GITHUB_REPOSITORY
) or CANARY_LABEL in self._pr_labels
- @classmethod
- def _find_caplog_in_def(cls, added_lines):
- """
- Find caplog in def
-
- :param added_lines: lines added in the file
- :return: True if caplog is found in def else False
- """
- line_counter = 0
- while line_counter < len(added_lines):
- if all(keyword in added_lines[line_counter] for keyword in ["def", "caplog", "(", ")"]):
- return True
- if "def" in added_lines[line_counter] and ")" not in added_lines[line_counter]:
- while line_counter < len(added_lines) and ")" not in added_lines[line_counter]:
- if "caplog" in added_lines[line_counter]:
- return True
- line_counter += 1
- line_counter += 1
- return None
-
- def _caplog_exists_in_added_lines(self) -> bool:
- """
- Check if caplog is used in added lines
-
- :return: True if caplog is used in added lines else False
- """
- lines = run_command(
- ["git", "diff", f"{self._commit_ref}^"],
- capture_output=True,
- text=True,
- cwd=AIRFLOW_ROOT_PATH,
- check=False,
- )
-
- if "caplog" not in lines.stdout or lines.stdout == "":
- return False
-
- added_caplog_lines = [
- line.lstrip().lstrip("+") for line in lines.stdout.split("\n") if line.lstrip().startswith("+")
- ]
- return self._find_caplog_in_def(added_lines=added_caplog_lines)
-
- @cached_property
- def is_log_mocked_in_the_tests(self) -> bool:
- """
- Check if log is used without mock in the tests
- """
- if self._is_canary_run() or self._github_event not in (
- GithubEvents.PULL_REQUEST,
- GithubEvents.PULL_REQUEST_TARGET,
- ):
- return False
- # Check if changed files are unit tests
- if (
- self._matching_files(FileGroupForCi.UNIT_TEST_FILES, CI_FILE_GROUP_MATCHES)
- and LOG_WITHOUT_MOCK_IN_TESTS_EXCEPTION_LABEL not in self._pr_labels
- ):
- if self._caplog_exists_in_added_lines():
- get_console().print(
- f"[error]Caplog is used in the test. "
- f"Please be sure you are mocking the log. "
- "For example, use `patch.object` to mock `log`."
- "Using `caplog` directly in your test files can cause side effects "
- "and not recommended. If you think, `caplog` is the only way to test "
- "the functionality or there is and exceptional case, "
- "please ask maintainer to include as an exception using "
- f"'{LOG_WITHOUT_MOCK_IN_TESTS_EXCEPTION_LABEL}' label. "
- "It is up to maintainer decision to allow this exception.",
- )
- sys.exit(1)
- else:
- return True
- return True
-
@cached_property
def force_pip(self):
return FORCE_PIP_LABEL in self._pr_labels
diff --git a/dev/breeze/tests/test_selective_checks.py b/dev/breeze/tests/test_selective_checks.py
index b9542f72d9d..3d2671a9711 100644
--- a/dev/breeze/tests/test_selective_checks.py
+++ b/dev/breeze/tests/test_selective_checks.py
@@ -19,7 +19,6 @@ from __future__ import annotations
import json
import re
from typing import Any
-from unittest.mock import MagicMock, patch
import pytest
from rich.console import Console
@@ -171,9 +170,6 @@ All_SKIPPED_COMMITS_IF_NON_MAIN_BRANCH = (
# commit that is neutral - allows to keep pyproject.toml-changing PRS neutral for unit tests
NEUTRAL_COMMIT = "938f0c1f3cc4cbe867123ee8aa9f290f9f18100a"
-# Use me if you are adding test for the changed files that includes caplog
-LOG_WITHOUT_MOCK_IN_TESTS_EXCEPTION_LABEL = "log exception"
-
def escape_ansi_colors(line):
return ANSI_COLORS_MATCHER.sub("", line)
@@ -1170,7 +1166,7 @@ def test_expected_output_pull_request_main(
files=files,
commit_ref=NEUTRAL_COMMIT,
github_event=GithubEvents.PULL_REQUEST,
- pr_labels=(LOG_WITHOUT_MOCK_IN_TESTS_EXCEPTION_LABEL,),
+ pr_labels=tuple(),
default_branch="main",
)
assert_outputs_are_printed(expected_outputs, str(stderr))
@@ -1548,10 +1544,7 @@ def test_full_test_needed_when_scripts_changes(files: tuple[str, ...], expected_
(
pytest.param(
("INTHEWILD.md", "providers/asana/tests/asana.py"),
- (
- "full tests needed",
- LOG_WITHOUT_MOCK_IN_TESTS_EXCEPTION_LABEL,
- ),
+ ("full tests needed",),
"v2-7-stable",
{
"all-python-versions": f"['{DEFAULT_PYTHON_MAJOR_MINOR_VERSION}']",
@@ -1706,7 +1699,7 @@ def test_expected_output_pull_request_v2_7(
files=files,
commit_ref=NEUTRAL_COMMIT,
github_event=GithubEvents.PULL_REQUEST,
- pr_labels=(LOG_WITHOUT_MOCK_IN_TESTS_EXCEPTION_LABEL,),
+ pr_labels=(),
default_branch="v2-7-stable",
)
assert_outputs_are_printed(expected_outputs, str(stderr))
@@ -1984,7 +1977,7 @@ def test_expected_output_pull_request_target(
files=files,
commit_ref=NEUTRAL_COMMIT,
github_event=GithubEvents.PULL_REQUEST_TARGET,
- pr_labels=(LOG_WITHOUT_MOCK_IN_TESTS_EXCEPTION_LABEL,),
+ pr_labels=(),
default_branch="main",
)
assert_outputs_are_printed(expected_outputs, str(stderr))
@@ -2445,217 +2438,3 @@ def test_mypy_matches(
pr_labels=pr_labels,
)
assert_outputs_are_printed(expected_outputs, str(stderr))
-
-
-@pytest.mark.parametrize(
- "files, pr_labels, github_event",
- [
- pytest.param(
- ("airflow-core/tests/unit/test.py",),
- (),
- GithubEvents.PULL_REQUEST,
- id="Caplog is in the the git diff Tests",
- ),
- pytest.param(
- ("providers/common/sql/tests/unit/common/sql/operators/test_sql.py",),
- (),
- GithubEvents.PULL_REQUEST,
- id="Caplog is in the git diff Providers",
- ),
- pytest.param(
- ("task-sdk/tests/definitions/test_dag.py",),
- (),
- GithubEvents.PULL_REQUEST,
- id="Caplog is in the git diff TaskSDK",
- ),
- ],
-)
-# Patch run_command
-@patch("airflow_breeze.utils.selective_checks.run_command")
-def test_is_log_mocked_in_the_tests_fail(
- mock_run_command,
- files: tuple[str, ...],
- pr_labels: tuple[str, ...],
- github_event: GithubEvents,
-):
- mock_run_command_result = MagicMock()
- mock_run_command_result.stdout = """
- + #Test Change
- + def test_selective_checks_caplop(self, caplog)
- + caplog.set_level(logging.INFO)
- + "test log" in caplog.text
- """
- mock_run_command.return_value = mock_run_command_result
- with pytest.raises(SystemExit):
- assert (
- "[error]please ask maintainer to include as an exception using "
- f"'{LOG_WITHOUT_MOCK_IN_TESTS_EXCEPTION_LABEL}' label."
- in escape_ansi_colors(
- str(
- SelectiveChecks(
- files=files,
- commit_ref=NEUTRAL_COMMIT,
- pr_labels=pr_labels,
- github_event=GithubEvents.PULL_REQUEST,
- default_branch="main",
- )
- )
- )
- )
-
-
-@pytest.mark.parametrize(
- "files, pr_labels, github_event",
- [
- pytest.param(
- ("airflow-core/tests/unit/test.py",),
- (),
- GithubEvents.PULL_REQUEST,
- id="Caplog is in the the git diff Tests",
- ),
- pytest.param(
- ("providers/common/sql/tests/unit/common/sql/operators/test_sql.py",),
- (),
- GithubEvents.PULL_REQUEST,
- id="Caplog is in the git diff Providers",
- ),
- pytest.param(
- ("task-sdk/tests/definitions/test_dag.py",),
- (),
- GithubEvents.PULL_REQUEST,
- id="Caplog is in the git diff TaskSDK",
- ),
- ],
-)
-# Patch run_command
-@patch("airflow_breeze.utils.selective_checks.run_command")
-def test_is_log_mocked_in_the_tests_fail_formatted(
- mock_run_command,
- files: tuple[str, ...],
- pr_labels: tuple[str, ...],
- github_event: GithubEvents,
-):
- mock_run_command_result = MagicMock()
- mock_run_command_result.stdout = """
- + #Test Change
- + def test_selective_checks(
- + self,
- + caplog
- + )
- + caplog.set_level(logging.INFO)
- + "test log" in caplog.text
- """
- mock_run_command.return_value = mock_run_command_result
- with pytest.raises(SystemExit):
- assert (
- "[error]please ask maintainer to include as an exception using "
- f"'{LOG_WITHOUT_MOCK_IN_TESTS_EXCEPTION_LABEL}' label."
- in escape_ansi_colors(
- str(
- SelectiveChecks(
- files=files,
- commit_ref=NEUTRAL_COMMIT,
- pr_labels=pr_labels,
- github_event=GithubEvents.PULL_REQUEST,
- default_branch="main",
- )
- )
- )
- )
-
-
-@pytest.mark.parametrize(
- "files, pr_labels, github_event",
- [
- pytest.param(
- ("airflow-core/tests/unit/test.py",),
- (),
- GithubEvents.PULL_REQUEST,
- id="Caplog is in the the git diff Tests",
- ),
- pytest.param(
- ("providers/common/sql/tests/unit/common/sql/operators/test_sql.py",),
- (),
- GithubEvents.PULL_REQUEST,
- id="Caplog is in the git diff Providers",
- ),
- pytest.param(
- ("task-sdk/tests/definitions/test_dag.py",),
- (),
- GithubEvents.PULL_REQUEST,
- id="Caplog is in the git diff TaskSDK",
- ),
- ],
-)
-# Patch run_command
-@patch("airflow_breeze.utils.selective_checks.run_command")
-def test_is_log_mocked_in_the_tests_not_fail(
- mock_run_command,
- files: tuple[str, ...],
- pr_labels: tuple[str, ...],
- github_event: GithubEvents,
-):
- mock_run_command_result = MagicMock()
- mock_run_command_result.stdout = """
- + #Test Change
- + def test_selective_checks(self)
- + assert "I am just a test" == "I am just a test"
- """
- mock_run_command.return_value = mock_run_command_result
- selective_checks = SelectiveChecks(
- files=files,
- commit_ref=NEUTRAL_COMMIT,
- pr_labels=pr_labels,
- github_event=GithubEvents.PULL_REQUEST,
- default_branch="main",
- )
- assert selective_checks.is_log_mocked_in_the_tests
-
-
-@pytest.mark.parametrize(
- "files, pr_labels, github_event",
- [
- pytest.param(
- ("airflow-core/tests/unit/test.py",),
- (LOG_WITHOUT_MOCK_IN_TESTS_EXCEPTION_LABEL,),
- GithubEvents.PULL_REQUEST,
- id="Caplog is in the the git diff Tests",
- ),
- pytest.param(
- ("providers/common/sql/tests/unit/common/sql/operators/test_sql.py",),
- (LOG_WITHOUT_MOCK_IN_TESTS_EXCEPTION_LABEL,),
- GithubEvents.PULL_REQUEST,
- id="Caplog is in the git diff Providers",
- ),
- pytest.param(
- ("task-sdk/tests/definitions/test_dag.py",),
- (LOG_WITHOUT_MOCK_IN_TESTS_EXCEPTION_LABEL,),
- GithubEvents.PULL_REQUEST,
- id="Caplog is in the git diff TaskSDK",
- ),
- ],
-)
-# Patch run_command
-@patch("airflow_breeze.utils.selective_checks.run_command")
-def test_is_log_mocked_in_the_tests_not_fail_with_label(
- mock_run_command,
- files: tuple[str, ...],
- pr_labels: tuple[str, ...],
- github_event: GithubEvents,
-):
- mock_run_command_result = MagicMock()
- mock_run_command_result.stdout = """
- + #Test Change
- + def test_selective_checks_caplop(self, caplog)
- + caplog.set_level(logging.INFO)
- + "test log" in caplog.text
- """
- mock_run_command.return_value = mock_run_command_result
- selective_checks = SelectiveChecks(
- files=files,
- commit_ref=NEUTRAL_COMMIT,
- pr_labels=pr_labels,
- github_event=GithubEvents.PULL_REQUEST,
- default_branch="main",
- )
- assert selective_checks.is_log_mocked_in_the_tests