This is an automated email from the ASF dual-hosted git repository.

potiuk pushed a commit to branch v3-4-test
in repository https://gitbox.apache.org/repos/asf/airflow.git


The following commit(s) were added to refs/heads/v3-4-test by this push:
     new fde6f5e0f06 [v3-4-test] Mock the simple auth manager in permission 
tests and guard against the base-class patch (#75138) (#75140)
fde6f5e0f06 is described below

commit fde6f5e0f063350e14f4d06f4d51757c3d8bdab8
Author: Jarek Potiuk <[email protected]>
AuthorDate: Mon Oct 19 22:45:56 2026 +0200

    [v3-4-test] Mock the simple auth manager in permission tests and guard 
against the base-class patch (#75138) (#75140)
    
    Only the check-auth-manager-mock-targets prek hook applies here: the 
dependencies graph permission tests from #73008 are not on v3-4-test, and the 
upstream asset events test from #72924 already patches SimpleAuthManager on 
this branch.
    
    (cherry picked from commit ac0c20c6ae3a18ff75e53d224c77159e33b9a43d)
    Generated-by: Claude Opus 5
---
 .pre-commit-config.yaml                            |  8 ++
 scripts/ci/prek/check_auth_manager_mock_targets.py | 88 ++++++++++++++++++++++
 .../prek/test_check_auth_manager_mock_targets.py   | 88 ++++++++++++++++++++++
 3 files changed, 184 insertions(+)

diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index b32abb06dad..e99e1224edd 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -975,6 +975,14 @@ repos:
         entry: "^\\s*from urllib\\.parse import ((\\|, 
)(urlparse\\|urlunparse))+$"
         pass_filenames: true
         files: \.py$
+      - id: check-auth-manager-mock-targets
+        name: Patch SimpleAuthManager, not BaseAuthManager, for methods it 
overrides
+        description: The API test client uses SimpleAuthManager, so patching 
an overridden BaseAuthManager method has no effect
+        entry: ./scripts/ci/prek/check_auth_manager_mock_targets.py
+        language: python
+        pass_filenames: true
+        files: ^airflow-core/tests/unit/api_fastapi/.*\.py$
+        exclude: ^airflow-core/tests/unit/api_fastapi/auth/managers/simple/
       - id: check-for-inclusive-language
         language: pygrep
         name: Check for language that we do not accept as community
diff --git a/scripts/ci/prek/check_auth_manager_mock_targets.py 
b/scripts/ci/prek/check_auth_manager_mock_targets.py
new file mode 100755
index 00000000000..a0850ba6814
--- /dev/null
+++ b/scripts/ci/prek/check_auth_manager_mock_targets.py
@@ -0,0 +1,88 @@
+#!/usr/bin/env python
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+# /// script
+# requires-python = ">=3.11,<3.12"
+# ///
+"""Check that API tests do not patch ``BaseAuthManager`` methods that 
``SimpleAuthManager`` overrides.
+
+The API test client runs with ``SimpleAuthManager``. Patching a 
``BaseAuthManager`` method that
+``SimpleAuthManager`` overrides leaves the override in place, so the mock 
never takes effect and a
+permission test passes or fails for the wrong reason. Patch 
``SimpleAuthManager`` instead.
+
+The set of overridden methods is read from the two classes, so it follows the 
code.
+"""
+
+from __future__ import annotations
+
+import ast
+import re
+import sys
+from pathlib import Path
+
+AIRFLOW_ROOT = Path(__file__).parents[3]
+BASE_AUTH_MANAGER = AIRFLOW_ROOT / 
"airflow-core/src/airflow/api_fastapi/auth/managers/base_auth_manager.py"
+SIMPLE_AUTH_MANAGER = (
+    AIRFLOW_ROOT / 
"airflow-core/src/airflow/api_fastapi/auth/managers/simple/simple_auth_manager.py"
+)
+
+PATCH_TARGET = re.compile(r"BaseAuthManager\.(\w+)")
+
+
+def get_methods(path: Path, class_name: str) -> set[str]:
+    tree = ast.parse(path.read_text(), filename=str(path))
+    for node in tree.body:
+        if isinstance(node, ast.ClassDef) and node.name == class_name:
+            return {
+                item.name
+                for item in node.body
+                if isinstance(item, (ast.FunctionDef, ast.AsyncFunctionDef)) 
and not item.name.startswith("_")
+            }
+    raise ValueError(f"class {class_name} not found in {path}")
+
+
+def get_overridden_methods() -> set[str]:
+    return get_methods(BASE_AUTH_MANAGER, "BaseAuthManager") & get_methods(
+        SIMPLE_AUTH_MANAGER, "SimpleAuthManager"
+    )
+
+
+def find_bad_patch_targets(text: str, overridden: set[str]) -> list[tuple[int, 
str]]:
+    return [
+        (line_number, match.group(1))
+        for line_number, line in enumerate(text.splitlines(), start=1)
+        for match in PATCH_TARGET.finditer(line)
+        if match.group(1) in overridden
+    ]
+
+
+def main(paths: list[str]) -> int:
+    overridden = get_overridden_methods()
+    errors = []
+    for path in paths:
+        for line_number, method in 
find_bad_patch_targets(Path(path).read_text(), overridden):
+            errors.append(
+                f"{path}:{line_number}: BaseAuthManager.{method} is overridden 
by SimpleAuthManager, "
+                f"so patching it has no effect in API tests; patch 
SimpleAuthManager.{method} instead."
+            )
+    for error in errors:
+        print(error)
+    return 1 if errors else 0
+
+
+if __name__ == "__main__":
+    sys.exit(main(sys.argv[1:]))
diff --git a/scripts/tests/ci/prek/test_check_auth_manager_mock_targets.py 
b/scripts/tests/ci/prek/test_check_auth_manager_mock_targets.py
new file mode 100644
index 00000000000..b73eef606dd
--- /dev/null
+++ b/scripts/tests/ci/prek/test_check_auth_manager_mock_targets.py
@@ -0,0 +1,88 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import pytest
+from check_auth_manager_mock_targets import (
+    find_bad_patch_targets,
+    get_methods,
+    get_overridden_methods,
+    main,
+)
+
+BASE = "airflow.api_fastapi.auth.managers.base_auth_manager.BaseAuthManager"
+SIMPLE = 
"airflow.api_fastapi.auth.managers.simple.simple_auth_manager.SimpleAuthManager"
+
+
+def test_get_methods_skips_private_methods(tmp_path):
+    module = tmp_path / "module.py"
+    module.write_text(
+        "class Manager:\n    def public(self): ...\n    async def run(self): 
...\n    def _private(self): ...\n"
+    )
+
+    assert get_methods(module, "Manager") == {"public", "run"}
+
+
+def test_get_methods_raises_for_missing_class(tmp_path):
+    module = tmp_path / "module.py"
+    module.write_text("class Other: ...\n")
+
+    with pytest.raises(ValueError, match="class Manager not found"):
+        get_methods(module, "Manager")
+
+
+def test_overridden_methods_follow_the_real_classes():
+    overridden = get_overridden_methods()
+
+    assert "get_authorized_assets" in overridden
+    assert "get_authorized_dag_ids" not in overridden
+
+
[email protected](
+    ("text", "expected"),
+    [
+        pytest.param(
+            f'@mock.patch("{BASE}.get_authorized_assets")', [(1, 
"get_authorized_assets")], id="base"
+        ),
+        pytest.param(f'@mock.patch("{SIMPLE}.get_authorized_assets")', [], 
id="simple"),
+        pytest.param(f'@mock.patch("{BASE}.get_authorized_dag_ids")', [], 
id="not-overridden"),
+        pytest.param(
+            f'x = 1\[email protected](\n    "{BASE}.is_authorized_dag",\n)',
+            [(3, "is_authorized_dag")],
+            id="multiline",
+        ),
+    ],
+)
+def test_find_bad_patch_targets(text, expected):
+    overridden = {"get_authorized_assets", "is_authorized_dag"}
+
+    assert find_bad_patch_targets(text, overridden) == expected
+
+
+def test_main_reports_bad_target_and_fails(tmp_path, capsys):
+    test_file = tmp_path / "test_routes.py"
+    test_file.write_text(f'@mock.patch("{BASE}.get_authorized_assets")\n')
+
+    assert main([str(test_file)]) == 1
+    assert "patch SimpleAuthManager.get_authorized_assets instead" in 
capsys.readouterr().out
+
+
+def test_main_passes_good_target(tmp_path):
+    test_file = tmp_path / "test_routes.py"
+    test_file.write_text(f'@mock.patch("{SIMPLE}.get_authorized_assets")\n')
+
+    assert main([str(test_file)]) == 0

Reply via email to