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 <[email protected]> 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)) - - [email protected]( - "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", - ) - ) - ) - ) - - [email protected]( - "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", - ) - ) - ) - ) - - [email protected]( - "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 - - [email protected]( - "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
