From 0a896696f48a9fddab2f92723bde8db5a553ac57 Mon Sep 17 00:00:00 2001 From: Romulo Quidute Filho Date: Mon, 2 Mar 2026 17:43:14 +0000 Subject: [PATCH 01/11] Improved logic to load python test scripts --- .../test_list_python_tests_classes.py | 562 ++++++++++++++++++ .../list_python_tests_classes.py | 173 ++++-- 2 files changed, 676 insertions(+), 59 deletions(-) create mode 100644 app/tests/test_engine/test_list_python_tests_classes.py diff --git a/app/tests/test_engine/test_list_python_tests_classes.py b/app/tests/test_engine/test_list_python_tests_classes.py new file mode 100644 index 00000000..93fd4b39 --- /dev/null +++ b/app/tests/test_engine/test_list_python_tests_classes.py @@ -0,0 +1,562 @@ +# +# Copyright (c) 2024 Project CHIP Authors +# +# Licensed 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. +# +""" +Unit tests for list_python_tests_classes module. + +Covers: +- load_ignore_list / load_include_list : file parsing with comments and blanks +- _is_matter_base_test_class : recursive AST inheritance resolution +- base_test_classes : module-level class filtering +- get_command_list : full discovery pipeline +""" +import ast +import textwrap +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest + +from test_collections.matter.sdk_tests.support.python_testing.list_python_tests_classes import ( + MATTER_BASE_TEST_CLASS_NAME, + _is_matter_base_test_class, + base_test_classes, + get_command_list, + load_ignore_list, + load_include_list, +) + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _parse(source: str) -> ast.Module: + """Parse a source string into an AST module.""" + return ast.parse(textwrap.dedent(source)) + + +def _make_sdk_folder(tmp_path: Path, files: dict[str, str]): + """Write *files* into *tmp_path* and return a mock SDKTestFolder.""" + for name, content in files.items(): + (tmp_path / name).write_text(textwrap.dedent(content)) + + folder = MagicMock() + folder.file_paths.return_value = sorted( + [tmp_path / name for name in files], + key=lambda p: p.name, + ) + return folder + + +# --------------------------------------------------------------------------- +# load_ignore_list / load_include_list +# --------------------------------------------------------------------------- + + +class TestLoadFileList: + def test_returns_empty_set_when_file_missing(self, tmp_path: Path) -> None: + with patch( + "test_collections.matter.sdk_tests.support.python_testing" + ".list_python_tests_classes.PYTHON_TESTS_IGNORE_FILE", + tmp_path / "nonexistent.txt", + ): + result = load_ignore_list() + assert result == set() + + def test_loads_filenames_from_file(self, tmp_path: Path) -> None: + f = tmp_path / "ignore.txt" + f.write_text("TC_FOO.py\nTC_BAR.py\n") + with patch( + "test_collections.matter.sdk_tests.support.python_testing" + ".list_python_tests_classes.PYTHON_TESTS_IGNORE_FILE", + f, + ): + result = load_ignore_list() + assert result == {"TC_FOO.py", "TC_BAR.py"} + + def test_skips_comment_lines(self, tmp_path: Path) -> None: + f = tmp_path / "ignore.txt" + f.write_text("# this is a comment\nTC_FOO.py\n") + with patch( + "test_collections.matter.sdk_tests.support.python_testing" + ".list_python_tests_classes.PYTHON_TESTS_IGNORE_FILE", + f, + ): + result = load_ignore_list() + assert result == {"TC_FOO.py"} + assert "# this is a comment" not in result + + def test_skips_blank_lines(self, tmp_path: Path) -> None: + f = tmp_path / "ignore.txt" + f.write_text("\n\nTC_FOO.py\n\n") + with patch( + "test_collections.matter.sdk_tests.support.python_testing" + ".list_python_tests_classes.PYTHON_TESTS_IGNORE_FILE", + f, + ): + result = load_ignore_list() + assert result == {"TC_FOO.py"} + + def test_include_list_uses_separate_file(self, tmp_path: Path) -> None: + f = tmp_path / "include.txt" + f.write_text("TCP_Tests.py\n") + with patch( + "test_collections.matter.sdk_tests.support.python_testing" + ".list_python_tests_classes.PYTHON_TESTS_INCLUDE_FILE", + f, + ): + result = load_include_list() + assert result == {"TCP_Tests.py"} + + +# --------------------------------------------------------------------------- +# _is_matter_base_test_class +# --------------------------------------------------------------------------- + + +class TestIsMatterBaseTestClass: + # --- positive: direct inheritance --- + + def test_direct_inheritance_from_matter_base_test(self) -> None: + module = _parse( + f""" + class MyTest({MATTER_BASE_TEST_CLASS_NAME}): + pass + """ + ) + assert _is_matter_base_test_class("MyTest", module, search_dir=None) is True + + # --- positive: same-file intermediate base --- + + def test_same_file_intermediate_base_class(self) -> None: + module = _parse( + f""" + class IntermediateBase({MATTER_BASE_TEST_CLASS_NAME}): + pass + + class MyTest(IntermediateBase): + pass + """ + ) + assert _is_matter_base_test_class("MyTest", module, search_dir=None) is True + + # --- positive: local file resolution (dot-to-slash) --- + + def test_resolves_flat_local_import(self, tmp_path: Path) -> None: + base_file = tmp_path / "MyBase.py" + base_file.write_text( + textwrap.dedent( + f""" + class MyBase({MATTER_BASE_TEST_CLASS_NAME}): + pass + """ + ) + ) + module = _parse( + """ + from MyBase import MyBase + + class MyTest(MyBase): + pass + """ + ) + assert _is_matter_base_test_class("MyTest", module, search_dir=tmp_path) is True + + def test_resolves_package_local_import_with_dot_to_slash( + self, tmp_path: Path + ) -> None: + pkg = tmp_path / "support_modules" + pkg.mkdir() + (pkg / "idm_support.py").write_text( + textwrap.dedent( + f""" + class IDMBaseTest({MATTER_BASE_TEST_CLASS_NAME}): + pass + """ + ) + ) + module = _parse( + """ + from support_modules.idm_support import IDMBaseTest + + class TC_IDM_1_2(IDMBaseTest): + pass + """ + ) + assert ( + _is_matter_base_test_class("TC_IDM_1_2", module, search_dir=tmp_path) + is True + ) + + def test_resolves_aliased_import(self, tmp_path: Path) -> None: + (tmp_path / "SomeBase.py").write_text( + textwrap.dedent( + f""" + class RealBase({MATTER_BASE_TEST_CLASS_NAME}): + pass + """ + ) + ) + module = _parse( + """ + from SomeBase import RealBase as AliasBase + + class MyTest(AliasBase): + pass + """ + ) + assert _is_matter_base_test_class("MyTest", module, search_dir=tmp_path) is True + + # --- positive: matter.testing fallback --- + + def test_matter_testing_fallback_when_no_local_file(self) -> None: + module = _parse( + """ + from matter.testing.basic_composition import BasicCompositionTests + + class TC_DA_1_2(BasicCompositionTests): + pass + """ + ) + assert _is_matter_base_test_class("TC_DA_1_2", module, search_dir=None) is True + + # --- negative --- + + def test_returns_false_for_unrelated_class(self) -> None: + module = _parse( + """ + class NotATest: + pass + """ + ) + assert _is_matter_base_test_class("NotATest", module, search_dir=None) is False + + def test_returns_false_when_class_not_in_module(self) -> None: + module = _parse( + f""" + class OtherTest({MATTER_BASE_TEST_CLASS_NAME}): + pass + """ + ) + assert _is_matter_base_test_class("Missing", module, search_dir=None) is False + + def test_returns_false_when_local_file_exists_but_no_inheritance( + self, tmp_path: Path + ) -> None: + (tmp_path / "SomeHelper.py").write_text( + textwrap.dedent( + """ + class SomeHelper: + pass + """ + ) + ) + module = _parse( + """ + from SomeHelper import SomeHelper + + class MyTest(SomeHelper): + pass + """ + ) + assert ( + _is_matter_base_test_class("MyTest", module, search_dir=tmp_path) is False + ) + + def test_returns_false_for_non_matter_testing_package(self) -> None: + module = _parse( + """ + from some.other.package import SomeBase + + class MyTest(SomeBase): + pass + """ + ) + assert _is_matter_base_test_class("MyTest", module, search_dir=None) is False + + # --- edge cases --- + + def test_cycle_guard_prevents_infinite_recursion(self) -> None: + # A -> B -> A (circular reference in same module) + module = _parse( + """ + class A(B): + pass + + class B(A): + pass + """ + ) + assert _is_matter_base_test_class("A", module, search_dir=None) is False + + def test_skips_local_file_with_syntax_error(self, tmp_path: Path) -> None: + (tmp_path / "BrokenBase.py").write_text("def broken(:\n") + module = _parse( + """ + from BrokenBase import BrokenBase + + class MyTest(BrokenBase): + pass + """ + ) + # Should not raise; returns False because the file can't be parsed + assert ( + _is_matter_base_test_class("MyTest", module, search_dir=tmp_path) is False + ) + + def test_ignores_non_name_bases(self) -> None: + # Attribute access (e.g. module.Base) is an ast.Attribute, not ast.Name + module = _parse( + """ + import some_module + + class MyTest(some_module.MatterBaseTest): + pass + """ + ) + assert _is_matter_base_test_class("MyTest", module, search_dir=None) is False + + +# --------------------------------------------------------------------------- +# base_test_classes +# --------------------------------------------------------------------------- + + +class TestBaseTestClasses: + def test_returns_only_matter_base_test_subclasses(self) -> None: + module = _parse( + f""" + class GoodTest({MATTER_BASE_TEST_CLASS_NAME}): + pass + + class NotATest: + pass + + class AnotherGoodTest({MATTER_BASE_TEST_CLASS_NAME}): + pass + """ + ) + result = base_test_classes(module) + names = [c.name for c in result] + assert names == ["GoodTest", "AnotherGoodTest"] + + def test_returns_empty_list_when_no_subclasses(self) -> None: + module = _parse( + """ + class Util: + pass + """ + ) + assert base_test_classes(module) == [] + + def test_passes_search_dir_for_local_resolution(self, tmp_path: Path) -> None: + (tmp_path / "TC_MyBase.py").write_text( + textwrap.dedent( + f""" + class TC_MyBase({MATTER_BASE_TEST_CLASS_NAME}): + pass + """ + ) + ) + module = _parse( + """ + from TC_MyBase import TC_MyBase + + class TC_CERT_1_1(TC_MyBase): + pass + """ + ) + result = base_test_classes(module, search_dir=tmp_path) + assert len(result) == 1 + assert result[0].name == "TC_CERT_1_1" + + +# --------------------------------------------------------------------------- +# get_command_list +# --------------------------------------------------------------------------- + + +class TestGetCommandList: + def test_includes_tc_files_with_matter_base_test_subclass( + self, tmp_path: Path + ) -> None: + folder = _make_sdk_folder( + tmp_path, + { + "TC_FOO_1_1.py": f""" + class TC_FOO_1_1({MATTER_BASE_TEST_CLASS_NAME}): + pass + """, + }, + ) + result = get_command_list(folder) + assert len(result) == 1 + assert result[0][1] == "TC_FOO_1_1" + + def test_excludes_non_tc_filenames(self, tmp_path: Path) -> None: + folder = _make_sdk_folder( + tmp_path, + { + "helper.py": f""" + class Helper({MATTER_BASE_TEST_CLASS_NAME}): + pass + """, + }, + ) + result = get_command_list(folder) + assert result == [] + + def test_excludes_files_in_ignore_list(self, tmp_path: Path) -> None: + ignore_file = tmp_path / "ignore.txt" + ignore_file.write_text("TC_IGNORED_1_1.py\n") + folder = _make_sdk_folder( + tmp_path, + { + "TC_IGNORED_1_1.py": f""" + class TC_IGNORED_1_1({MATTER_BASE_TEST_CLASS_NAME}): + pass + """, + }, + ) + with patch( + "test_collections.matter.sdk_tests.support.python_testing" + ".list_python_tests_classes.PYTHON_TESTS_IGNORE_FILE", + ignore_file, + ): + result = get_command_list(folder) + assert result == [] + + def test_includes_non_tc_file_from_include_list(self, tmp_path: Path) -> None: + include_file = tmp_path / "include.txt" + include_file.write_text("TCP_Tests.py\n") + folder = _make_sdk_folder( + tmp_path, + { + "TCP_Tests.py": f""" + class TCP_Tests({MATTER_BASE_TEST_CLASS_NAME}): + pass + """, + }, + ) + with patch( + "test_collections.matter.sdk_tests.support.python_testing" + ".list_python_tests_classes.PYTHON_TESTS_INCLUDE_FILE", + include_file, + ): + result = get_command_list(folder) + assert len(result) == 1 + assert result[0][1] == "TCP_Tests" + + def test_skips_files_with_syntax_errors(self, tmp_path: Path) -> None: + folder = _make_sdk_folder( + tmp_path, + { + "TC_BROKEN_1_1.py": "def broken(:\n", + }, + ) + result = get_command_list(folder) + assert result == [] + + def test_skips_tc_file_with_no_base_test_subclass(self, tmp_path: Path) -> None: + folder = _make_sdk_folder( + tmp_path, + { + "TC_UTIL_1_1.py": """ + class TC_UTIL_1_1: + pass + """, + }, + ) + result = get_command_list(folder) + assert result == [] + + def test_command_format_contains_path_and_class_name(self, tmp_path: Path) -> None: + folder = _make_sdk_folder( + tmp_path, + { + "TC_FOO_1_1.py": f""" + class TC_FOO_1_1({MATTER_BASE_TEST_CLASS_NAME}): + pass + """, + }, + ) + result = get_command_list(folder) + assert len(result) == 1 + path_part, class_part = result[0] + assert class_part == "TC_FOO_1_1" + assert "TC_FOO_1_1" in path_part + + def test_resolves_local_base_class_file(self, tmp_path: Path) -> None: + folder = _make_sdk_folder( + tmp_path, + { + "TC_TLSCERT_Base.py": f""" + class TC_TLSCERT_Base({MATTER_BASE_TEST_CLASS_NAME}): + pass + """, + "TC_TLSCERT_2_1.py": """ + from TC_TLSCERT_Base import TC_TLSCERT_Base + + class TC_TLSCERT_2_1(TC_TLSCERT_Base): + pass + """, + }, + ) + result = get_command_list(folder) + class_names = [cmd[1] for cmd in result] + # TC_TLSCERT_2_1 must be discovered via local base file resolution + assert "TC_TLSCERT_2_1" in class_names + # TC_TLSCERT_Base.py matches TC_*.py and directly inherits MatterBaseTest, + # so it is also a valid command entry + assert "TC_TLSCERT_Base" in class_names + + def test_multiple_classes_in_one_file_each_become_command( + self, tmp_path: Path + ) -> None: + folder = _make_sdk_folder( + tmp_path, + { + "TC_MULTI_1_1.py": f""" + class TC_MULTI_1_1({MATTER_BASE_TEST_CLASS_NAME}): + pass + + class TC_MULTI_1_2({MATTER_BASE_TEST_CLASS_NAME}): + pass + """, + }, + ) + result = get_command_list(folder) + class_names = [cmd[1] for cmd in result] + assert "TC_MULTI_1_1" in class_names + assert "TC_MULTI_1_2" in class_names + + def test_results_are_sorted_by_filename(self, tmp_path: Path) -> None: + folder = _make_sdk_folder( + tmp_path, + { + "TC_B_1_1.py": f""" + class TC_B_1_1({MATTER_BASE_TEST_CLASS_NAME}): + pass + """, + "TC_A_1_1.py": f""" + class TC_A_1_1({MATTER_BASE_TEST_CLASS_NAME}): + pass + """, + }, + ) + result = get_command_list(folder) + class_names = [cmd[1] for cmd in result] + assert class_names == ["TC_A_1_1", "TC_B_1_1"] diff --git a/test_collections/matter/sdk_tests/support/python_testing/list_python_tests_classes.py b/test_collections/matter/sdk_tests/support/python_testing/list_python_tests_classes.py index fd5553f2..8f30d10c 100644 --- a/test_collections/matter/sdk_tests/support/python_testing/list_python_tests_classes.py +++ b/test_collections/matter/sdk_tests/support/python_testing/list_python_tests_classes.py @@ -35,6 +35,7 @@ # Make these constants synced with "test_harness_client.py" GET_TEST_INFO_ARGUMENT = "--get-test-info" TEST_INFO_JSON_FILENAME = "test_info.json" +MATTER_BASE_TEST_CLASS_NAME = "MatterBaseTest" # Pattern to match TC_*.py format # TC_ followed by at least one character/digit, then .py @@ -131,76 +132,128 @@ def load_include_list() -> set[str]: return __load_file_list(PYTHON_TESTS_INCLUDE_FILE) -def base_test_classes(module: ast.Module) -> list[ast.ClassDef]: - """Find classes that inherit from MatterBaseTest. +def _is_matter_base_test_class( + class_name: str, + module: ast.Module, + search_dir: Optional[Path], + _visiting: Optional[set] = None, +) -> bool: + """Recursively check if a class name in a parsed module ultimately inherits + from MatterBaseTest, following local file imports as needed. Args: - module (ast.Module): Python module. + class_name: Name of the class to check. + module: Parsed AST of the file where class_name is defined. + search_dir: Directory to search for locally-imported modules. + _visiting: Set of (file, class) pairs already being resolved (cycle guard). Returns: - list[ast.ClassDef]: List of classes from the given module that inherit from - MatterBaseTest. + bool: True if the class transitively inherits from MatterBaseTest. """ + if _visiting is None: + _visiting = set() - # Get all imported classes that could be base test classes - imported_base_classes = set() - for node in module.body: - if isinstance(node, ast.ImportFrom): - # Include imports from support_modules, matter_testing, - # or any module ending with Base/Test - if node.module and ( - any( - s in node.module - for s in [ - "support_modules", - "matter_testing", - "matter.testing.basic_composition", - "test_testing", - ] - ) - or node.module.endswith("TestBase") - ): - for alias in node.names: - imported_base_classes.add(alias.name) - - def inherits_from_matter_base_test( - class_def: ast.ClassDef, visited: Optional[set] = None - ) -> bool: - if visited is None: - visited = set() - - if class_def.name in visited: - return False - visited.add(class_def.name) - - # Check direct inheritance from MatterBaseTest or imported base classes - for base in class_def.bases: - if isinstance(base, ast.Name): - if base.id == "MatterBaseTest" or base.id in imported_base_classes: - return True - - # Check inheritance from parent classes in the same module - for base in class_def.bases: - if isinstance(base, ast.Name): - parent_class = next( - ( - c - for c in module.body - if isinstance(c, ast.ClassDef) and c.name == base.id - ), - None, - ) - if parent_class and inherits_from_matter_base_test( - parent_class, visited.copy() - ): - return True + # Find the class definition in this module + class_def = next( + ( + node + for node in module.body + if isinstance(node, ast.ClassDef) and node.name == class_name + ), + None, + ) + if class_def is None: + return False + key = (id(module), class_name) + if key in _visiting: return False + _visiting = _visiting | {key} + + for base in class_def.bases: + if not isinstance(base, ast.Name): + continue + base_name = base.id + + # Direct hit + if base_name == MATTER_BASE_TEST_CLASS_NAME: + return True + + # Check if base_name is defined in the same module (local parent class) + if _is_matter_base_test_class(base_name, module, search_dir, _visiting): + return True + + # Try to resolve base_name via imports in this module + for node in module.body: + if not isinstance(node, ast.ImportFrom): + continue + imported_names = [alias.asname or alias.name for alias in node.names] + if base_name not in imported_names: + continue + + if not node.module: + continue + + # Try to load a local file. Module dots are converted to path separators + # so both flat imports (TC_TLSCERT_Base) and package imports + # (support_modules.idm_support) resolve to the correct sibling file. + if search_dir: + candidate = search_dir / f"{node.module.replace('.', '/')}.py" + if candidate.exists(): + try: + with open(candidate, "r") as f: + imported_module = ast.parse(f.read()) + # The actual class name in the imported file may differ + # (e.g. `from TC_TLSCERT_Base import TC_TLSCERT_Base`) + actual_name = next( + ( + alias.name + for alias in node.names + if (alias.asname or alias.name) == base_name + ), + base_name, + ) + if _is_matter_base_test_class( + actual_name, imported_module, search_dir, _visiting + ): + return True + continue + + except SyntaxError: + pass + + # Fallback for installed packages whose source is not available as a + # local file (e.g. matter.testing.* installed as a wheel). + # BasicCompositionTests and similar classes from matter.testing.* are + # known to inherit from MatterBaseTest, so we trust those imports. + if "matter.testing" in node.module: + return True + + return False + +def base_test_classes( + module: ast.Module, search_dir: Optional[Path] = None +) -> list[ast.ClassDef]: + """Find classes that inherit from MatterBaseTest, following local imports + transitively so that intermediate base classes (e.g. TC_TLSCERT_Base) are + resolved without needing hardcoded module-name patterns. + + Args: + module (ast.Module): Parsed Python module. + search_dir (Optional[Path]): Directory containing the file's siblings, + used to resolve local imports. When None only same-file inheritance + and the legacy pattern-based fallback are used. + + Returns: + list[ast.ClassDef]: Classes in the module that ultimately inherit from + MatterBaseTest. + """ return [ c for c in module.body - if isinstance(c, ast.ClassDef) and inherits_from_matter_base_test(c) + if isinstance(c, ast.ClassDef) + and _is_matter_base_test_class(c.name, module, search_dir) ] @@ -240,7 +293,9 @@ def get_command_list(test_folder: SDKTestFolder) -> list: ) continue - test_classes = base_test_classes(parsed_python_file) + test_classes = base_test_classes( + parsed_python_file, search_dir=python_test_file.parent + ) for test_class in test_classes: # Add file path and class name script_command = [ From a1f65db5eaba667c80ae01bc59c764a0e0fe9ed9 Mon Sep 17 00:00:00 2001 From: Romulo Quidute Filho Date: Mon, 2 Mar 2026 20:24:53 +0000 Subject: [PATCH 02/11] Added test_python_test_initialization.py unit test file --- .../test_python_test_initialization.py | 376 ++++++++++++++++++ 1 file changed, 376 insertions(+) create mode 100644 app/tests/test_engine/test_python_test_initialization.py diff --git a/app/tests/test_engine/test_python_test_initialization.py b/app/tests/test_engine/test_python_test_initialization.py new file mode 100644 index 00000000..a451d5d4 --- /dev/null +++ b/app/tests/test_engine/test_python_test_initialization.py @@ -0,0 +1,376 @@ +# +# Copyright (c) 2026 Project CHIP Authors +# +# Licensed 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. +# +""" +Unit tests for Python test initialization optimizations. + +These tests verify: +1. Phase 1: TestScriptManager constructor doesn't initialize Python tests +2. Phase 2: Python test generation uses single container session +""" +from unittest.mock import AsyncMock, MagicMock, Mock, call, patch + +import pytest + +from app.test_engine.test_script_manager import TestScriptManager +from test_collections.matter.sdk_tests.support.python_testing.list_python_tests_classes import ( + generate_python_test_json_file, + process_test_commands_with_container, +) +from test_collections.matter.sdk_tests.support.python_testing.test_manager import ( + _generate_all_test_files, +) + + +@pytest.fixture(autouse=True) +def restore_singleton_state(): + """Save and restore TestScriptManager singleton state around each test. + + TestScriptManager is a singleton shared across the test session. + Some tests call initialize_python_tests() which mutates test_collections + and _python_tests_initialized on the singleton. This fixture ensures + those mutations are rolled back after each test, and that the flag starts + at False before each test (matching the constructor's initial state). + """ + manager = TestScriptManager() + saved_collections = manager.test_collections + saved_flag = manager._python_tests_initialized + # Reset to the initial post-constructor state before each test + manager._python_tests_initialized = False + yield + manager.test_collections = saved_collections + manager._python_tests_initialized = saved_flag + + +class TestPhase1ConstructorOptimization: + """Test Phase 1: Constructor should not initialize Python tests.""" + + def test_constructor_does_not_call_ensure_initialization(self) -> None: + """Verify constructor doesn't call _ensure_python_tests_initialized.""" + # Create a new TestScriptManager instance + # Note: We use the existing singleton, so we verify behavior indirectly + manager = TestScriptManager() + + # Verify the method doesn't exist (it was removed) + assert not hasattr( + manager, "_ensure_python_tests_initialized" + ), "Constructor should not have _ensure_python_tests_initialized method" + + def test_constructor_sets_initialization_flag_to_false(self) -> None: + """Verify constructor sets _python_tests_initialized to False. + + Since TestScriptManager is a singleton, this test verifies that the + flag is False when no initialization has occurred in this test session, + relying on the restore_singleton_state fixture to reset it. + """ + manager = TestScriptManager() + + # The flag should be False (reset by restore_singleton_state fixture) + assert ( + manager._python_tests_initialized is False + ), "Constructor should set _python_tests_initialized to False" + + def test_constructor_discovers_test_collections(self) -> None: + """Verify constructor calls _discover_test_collections.""" + manager = TestScriptManager() + + # Verify test_collections is populated + assert hasattr( + manager, "test_collections" + ), "Constructor should set test_collections" + assert isinstance( + manager.test_collections, dict + ), "test_collections should be a dictionary" + + +class TestPhase1AsyncInitialization: + """Test Phase 1: Async initialization should work correctly.""" + + @pytest.mark.asyncio + async def test_initialize_python_tests_sets_flag(self) -> None: + """Verify initialize_python_tests sets _python_tests_initialized flag.""" + manager = TestScriptManager() + + # Mock the initialization function + with patch( + "app.test_engine.test_script_manager.discover_test_collections" + ) as mock_discover: + mock_discover.return_value = {} + + with patch( + "test_collections.matter.sdk_tests.support.python_testing.initialize_python_tests", + new_callable=AsyncMock, + ) as mock_init: + # Setup mock to return test collections + mock_init.return_value = ( + MagicMock(), # sdk_collection + MagicMock(), # mandatory_collection + None, # custom_collection + ) + + try: + await manager.initialize_python_tests() + except Exception: + # May fail due to import issues in test environment, but we verify the attempt + pass + + # The flag should be set to True after initialization attempt + # Note: In test environment this may not complete due to mocking + + @pytest.mark.asyncio + async def test_initialize_python_tests_updates_collections(self) -> None: + """Verify initialize_python_tests updates test_collections.""" + manager = TestScriptManager() + + with patch( + "app.test_engine.test_script_manager.discover_test_collections" + ) as mock_discover: + expected_collections = {"test_collection": MagicMock()} + mock_discover.return_value = expected_collections + + with patch( + "test_collections.matter.sdk_tests.support.python_testing.initialize_python_tests", + new_callable=AsyncMock, + ) as mock_init: + mock_init.return_value = (MagicMock(), MagicMock(), None) + + try: + await manager.initialize_python_tests() + + # Verify discover was called after initialization + assert mock_discover.called, "Should call discover_test_collections" + except Exception: + # May fail in test environment, but we verify the pattern + pass + + +class TestPhase2SingleContainerSession: + """Test Phase 2: Single container session for all test generation.""" + + @pytest.mark.asyncio + async def test_generate_all_test_files_uses_single_container(self) -> None: + """Verify _generate_all_test_files uses a single SDK container session.""" + mock_container = MagicMock() + mock_container.start = AsyncMock() + mock_container.destroy = MagicMock() + + with patch( + "test_collections.matter.sdk_tests.support.python_testing.test_manager.SDKContainer" + ) as mock_container_class: + mock_container_class.return_value = mock_container + + with patch( + "test_collections.matter.sdk_tests.support.python_testing.test_manager.get_command_list" + ) as mock_get_commands: + mock_get_commands.return_value = [] + + with patch( + "test_collections.matter.sdk_tests.support.python_testing.test_manager.process_test_commands_with_container", + new_callable=AsyncMock, + ) as mock_process: + with patch( + "test_collections.matter.sdk_tests.support.python_testing.test_manager._has_custom_tests" + ) as mock_has_custom: + mock_has_custom.return_value = True + + # Execute the function + await _generate_all_test_files() + + # Verify container was created once + assert ( + mock_container_class.call_count == 1 + ), "Should create container once" + + # Verify container was started once + assert ( + mock_container.start.call_count == 1 + ), "Should start container once" + + # Verify container was destroyed once + assert ( + mock_container.destroy.call_count == 1 + ), "Should destroy container once" + + # Verify process_test_commands_with_container was called twice + # (once for SDK tests, once for custom tests) + assert ( + mock_process.call_count == 2 + ), "Should process tests twice with same container" + + @pytest.mark.asyncio + async def test_generate_all_test_files_destroys_container_on_error(self) -> None: + """Verify container is destroyed even if test generation fails.""" + mock_container = MagicMock() + mock_container.start = AsyncMock() + mock_container.destroy = MagicMock() + + with patch( + "test_collections.matter.sdk_tests.support.python_testing.test_manager.SDKContainer" + ) as mock_container_class: + mock_container_class.return_value = mock_container + + with patch( + "test_collections.matter.sdk_tests.support.python_testing.test_manager.get_command_list" + ) as mock_get_commands: + # Simulate an error during command list generation + mock_get_commands.side_effect = Exception("Test error") + + # Execute the function and expect it to raise + with pytest.raises(Exception, match="Test error"): + await _generate_all_test_files() + + # Verify container was still destroyed despite the error + assert ( + mock_container.destroy.call_count == 1 + ), "Should destroy container even on error" + + @pytest.mark.asyncio + async def test_generate_all_test_files_skips_custom_when_none_exist(self) -> None: + """Verify custom test generation is skipped when no custom tests exist.""" + mock_container = MagicMock() + mock_container.start = AsyncMock() + mock_container.destroy = MagicMock() + + with patch( + "test_collections.matter.sdk_tests.support.python_testing.test_manager.SDKContainer" + ) as mock_container_class: + mock_container_class.return_value = mock_container + + with patch( + "test_collections.matter.sdk_tests.support.python_testing.test_manager.get_command_list" + ) as mock_get_commands: + mock_get_commands.return_value = [] + + with patch( + "test_collections.matter.sdk_tests.support.python_testing.test_manager.process_test_commands_with_container", + new_callable=AsyncMock, + ) as mock_process: + with patch( + "test_collections.matter.sdk_tests.support.python_testing.test_manager._has_custom_tests" + ) as mock_has_custom: + mock_has_custom.return_value = False + + with patch( + "test_collections.matter.sdk_tests.support.python_testing.test_manager.CUSTOM_PYTHON_TESTS_PARSED_FILE" + ) as mock_file: + mock_file.write_text = MagicMock() + + # Execute the function + await _generate_all_test_files() + + # Verify process was called only once (for SDK tests) + assert ( + mock_process.call_count == 1 + ), "Should process SDK tests only" + + # Verify empty JSON was written for custom tests + mock_file.write_text.assert_called_once_with( + '{"tests": []}' + ) + + +class TestPhase2ProcessTestCommandsWithContainer: + """Test the new process_test_commands_with_container function.""" + + @pytest.mark.asyncio + async def test_process_test_commands_does_not_start_container(self) -> None: + """Verify process_test_commands_with_container doesn't start/stop container.""" + mock_container = MagicMock() + mock_container.start = AsyncMock() + mock_container.destroy = MagicMock() + + with patch( + "test_collections.matter.sdk_tests.support.python_testing.list_python_tests_classes.__process_grouped_commands", + new_callable=AsyncMock, + ) as mock_process: + mock_process.return_value = (0, 0) # test_count, invalid_count + + with patch( + "test_collections.matter.sdk_tests.support.python_testing.list_python_tests_classes.Path" + ): + with patch("builtins.open", MagicMock()): + # Execute the function + await process_test_commands_with_container( + sdk_container=mock_container, + commands=[], + json_output_file=Mock(), + grouped_commands=True, + ) + + # Verify container start/destroy were NOT called + assert ( + mock_container.start.call_count == 0 + ), "Should not start container" + assert ( + mock_container.destroy.call_count == 0 + ), "Should not destroy container" + + @pytest.mark.asyncio + async def test_process_test_commands_uses_provided_container(self) -> None: + """Verify process_test_commands_with_container uses the provided container.""" + mock_container = MagicMock() + + with patch( + "test_collections.matter.sdk_tests.support.python_testing.list_python_tests_classes.__process_grouped_commands", + new_callable=AsyncMock, + ) as mock_process: + mock_process.return_value = (0, 0) + + with patch( + "test_collections.matter.sdk_tests.support.python_testing.list_python_tests_classes.Path" + ): + with patch("builtins.open", MagicMock()): + await process_test_commands_with_container( + sdk_container=mock_container, + commands=[], + json_output_file=Mock(), + grouped_commands=True, + ) + + # Verify the container was passed to process function + assert mock_process.called, "Should call process function" + call_args = mock_process.call_args + assert ( + call_args[0][0] == mock_container + ), "Should pass container to process function" + + +class TestBackwardCompatibility: + """Test that backward compatibility is maintained.""" + + @pytest.mark.asyncio + async def test_generate_python_test_json_file_still_works(self) -> None: + """Verify generate_python_test_json_file still works for backward compatibility.""" + # This function should still exist and be callable + assert callable( + generate_python_test_json_file + ), "generate_python_test_json_file should still exist" + + # Verify it calls the old process_commands_sdk_container function + with patch( + "test_collections.matter.sdk_tests.support.python_testing.list_python_tests_classes.process_commands_sdk_container", + new_callable=AsyncMock, + ) as mock_process: + with patch( + "test_collections.matter.sdk_tests.support.python_testing.list_python_tests_classes.get_command_list" + ) as mock_get_commands: + mock_get_commands.return_value = [] + + await generate_python_test_json_file() + + # Verify the old function was called + assert ( + mock_process.called + ), "Should call process_commands_sdk_container for backward compatibility" From 322ecff182dab5d6199c16080914c31a633d2783 Mon Sep 17 00:00:00 2001 From: Romulo Quidute Filho Date: Mon, 2 Mar 2026 21:12:08 +0000 Subject: [PATCH 03/11] Update copyright year --- app/tests/test_engine/test_list_python_tests_classes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/tests/test_engine/test_list_python_tests_classes.py b/app/tests/test_engine/test_list_python_tests_classes.py index 93fd4b39..332a1107 100644 --- a/app/tests/test_engine/test_list_python_tests_classes.py +++ b/app/tests/test_engine/test_list_python_tests_classes.py @@ -1,5 +1,5 @@ # -# Copyright (c) 2024 Project CHIP Authors +# Copyright (c) 2026 Project CHIP Authors # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. From 14d30835458e0f1f158e51799f05dc860a20352a Mon Sep 17 00:00:00 2001 From: Romulo Quidute Filho Date: Mon, 2 Mar 2026 21:20:48 +0000 Subject: [PATCH 04/11] Code review - mypy/flake8 --- .../test_list_python_tests_classes.py | 4 +- .../test_python_test_initialization.py | 61 ++++++++++++------- 2 files changed, 41 insertions(+), 24 deletions(-) diff --git a/app/tests/test_engine/test_list_python_tests_classes.py b/app/tests/test_engine/test_list_python_tests_classes.py index 332a1107..f5e9b443 100644 --- a/app/tests/test_engine/test_list_python_tests_classes.py +++ b/app/tests/test_engine/test_list_python_tests_classes.py @@ -27,9 +27,7 @@ from pathlib import Path from unittest.mock import MagicMock, patch -import pytest - -from test_collections.matter.sdk_tests.support.python_testing.list_python_tests_classes import ( +from test_collections.matter.sdk_tests.support.python_testing.list_python_tests_classes import ( # noqa MATTER_BASE_TEST_CLASS_NAME, _is_matter_base_test_class, base_test_classes, diff --git a/app/tests/test_engine/test_python_test_initialization.py b/app/tests/test_engine/test_python_test_initialization.py index a451d5d4..3a9077d8 100644 --- a/app/tests/test_engine/test_python_test_initialization.py +++ b/app/tests/test_engine/test_python_test_initialization.py @@ -20,12 +20,12 @@ 1. Phase 1: TestScriptManager constructor doesn't initialize Python tests 2. Phase 2: Python test generation uses single container session """ -from unittest.mock import AsyncMock, MagicMock, Mock, call, patch +from unittest.mock import AsyncMock, MagicMock, Mock, patch import pytest from app.test_engine.test_script_manager import TestScriptManager -from test_collections.matter.sdk_tests.support.python_testing.list_python_tests_classes import ( +from test_collections.matter.sdk_tests.support.python_testing.list_python_tests_classes import ( # noqa generate_python_test_json_file, process_test_commands_with_container, ) @@ -110,7 +110,8 @@ async def test_initialize_python_tests_sets_flag(self) -> None: mock_discover.return_value = {} with patch( - "test_collections.matter.sdk_tests.support.python_testing.initialize_python_tests", + "test_collections.matter.sdk_tests.support.python_testing." + "initialize_python_tests", new_callable=AsyncMock, ) as mock_init: # Setup mock to return test collections @@ -123,7 +124,8 @@ async def test_initialize_python_tests_sets_flag(self) -> None: try: await manager.initialize_python_tests() except Exception: - # May fail due to import issues in test environment, but we verify the attempt + # May fail due to import issues in test environment, + # but we verify the attempt pass # The flag should be set to True after initialization attempt @@ -141,7 +143,8 @@ async def test_initialize_python_tests_updates_collections(self) -> None: mock_discover.return_value = expected_collections with patch( - "test_collections.matter.sdk_tests.support.python_testing.initialize_python_tests", + "test_collections.matter.sdk_tests.support.python_testing." + "initialize_python_tests", new_callable=AsyncMock, ) as mock_init: mock_init.return_value = (MagicMock(), MagicMock(), None) @@ -167,21 +170,25 @@ async def test_generate_all_test_files_uses_single_container(self) -> None: mock_container.destroy = MagicMock() with patch( - "test_collections.matter.sdk_tests.support.python_testing.test_manager.SDKContainer" + "test_collections.matter.sdk_tests.support.python_testing.test_manager." + "SDKContainer" ) as mock_container_class: mock_container_class.return_value = mock_container with patch( - "test_collections.matter.sdk_tests.support.python_testing.test_manager.get_command_list" + "test_collections.matter.sdk_tests.support.python_testing.test_manager." + "get_command_list" ) as mock_get_commands: mock_get_commands.return_value = [] with patch( - "test_collections.matter.sdk_tests.support.python_testing.test_manager.process_test_commands_with_container", + "test_collections.matter.sdk_tests.support.python_testing." + "test_manager.process_test_commands_with_container", new_callable=AsyncMock, ) as mock_process: with patch( - "test_collections.matter.sdk_tests.support.python_testing.test_manager._has_custom_tests" + "test_collections.matter.sdk_tests.support.python_testing." + "test_manager._has_custom_tests" ) as mock_has_custom: mock_has_custom.return_value = True @@ -217,12 +224,14 @@ async def test_generate_all_test_files_destroys_container_on_error(self) -> None mock_container.destroy = MagicMock() with patch( - "test_collections.matter.sdk_tests.support.python_testing.test_manager.SDKContainer" + "test_collections.matter.sdk_tests.support.python_testing.test_manager." + "SDKContainer" ) as mock_container_class: mock_container_class.return_value = mock_container with patch( - "test_collections.matter.sdk_tests.support.python_testing.test_manager.get_command_list" + "test_collections.matter.sdk_tests.support.python_testing.test_manager." + "get_command_list" ) as mock_get_commands: # Simulate an error during command list generation mock_get_commands.side_effect = Exception("Test error") @@ -244,26 +253,31 @@ async def test_generate_all_test_files_skips_custom_when_none_exist(self) -> Non mock_container.destroy = MagicMock() with patch( - "test_collections.matter.sdk_tests.support.python_testing.test_manager.SDKContainer" + "test_collections.matter.sdk_tests.support.python_testing.test_manager." + "SDKContainer" ) as mock_container_class: mock_container_class.return_value = mock_container with patch( - "test_collections.matter.sdk_tests.support.python_testing.test_manager.get_command_list" + "test_collections.matter.sdk_tests.support.python_testing.test_manager." + "get_command_list" ) as mock_get_commands: mock_get_commands.return_value = [] with patch( - "test_collections.matter.sdk_tests.support.python_testing.test_manager.process_test_commands_with_container", + "test_collections.matter.sdk_tests.support.python_testing." + "test_manager.process_test_commands_with_container", new_callable=AsyncMock, ) as mock_process: with patch( - "test_collections.matter.sdk_tests.support.python_testing.test_manager._has_custom_tests" + "test_collections.matter.sdk_tests.support.python_testing." + "test_manager._has_custom_tests" ) as mock_has_custom: mock_has_custom.return_value = False with patch( - "test_collections.matter.sdk_tests.support.python_testing.test_manager.CUSTOM_PYTHON_TESTS_PARSED_FILE" + "test_collections.matter.sdk_tests.support.python_testing." + "test_manager.CUSTOM_PYTHON_TESTS_PARSED_FILE" ) as mock_file: mock_file.write_text = MagicMock() @@ -292,13 +306,15 @@ async def test_process_test_commands_does_not_start_container(self) -> None: mock_container.destroy = MagicMock() with patch( - "test_collections.matter.sdk_tests.support.python_testing.list_python_tests_classes.__process_grouped_commands", + "test_collections.matter.sdk_tests.support.python_testing." + "list_python_tests_classes.__process_grouped_commands", new_callable=AsyncMock, ) as mock_process: mock_process.return_value = (0, 0) # test_count, invalid_count with patch( - "test_collections.matter.sdk_tests.support.python_testing.list_python_tests_classes.Path" + "test_collections.matter.sdk_tests.support.python_testing." + "list_python_tests_classes.Path" ): with patch("builtins.open", MagicMock()): # Execute the function @@ -323,13 +339,15 @@ async def test_process_test_commands_uses_provided_container(self) -> None: mock_container = MagicMock() with patch( - "test_collections.matter.sdk_tests.support.python_testing.list_python_tests_classes.__process_grouped_commands", + "test_collections.matter.sdk_tests.support.python_testing." + "list_python_tests_classes.__process_grouped_commands", new_callable=AsyncMock, ) as mock_process: mock_process.return_value = (0, 0) with patch( - "test_collections.matter.sdk_tests.support.python_testing.list_python_tests_classes.Path" + "test_collections.matter.sdk_tests.support.python_testing." + "list_python_tests_classes.Path" ): with patch("builtins.open", MagicMock()): await process_test_commands_with_container( @@ -352,7 +370,8 @@ class TestBackwardCompatibility: @pytest.mark.asyncio async def test_generate_python_test_json_file_still_works(self) -> None: - """Verify generate_python_test_json_file still works for backward compatibility.""" + """Verify generate_python_test_json_file still works for backward + compatibility.""" # This function should still exist and be callable assert callable( generate_python_test_json_file From 1b1d2752dbf5454f8ae7b540ae53d4d8894cca2c Mon Sep 17 00:00:00 2001 From: Romulo Quidute Filho Date: Mon, 2 Mar 2026 21:23:56 +0000 Subject: [PATCH 05/11] Code review - mypy/flake8 --- .../test_engine/test_list_python_tests_classes.py | 2 +- .../test_python_test_initialization.py | 15 +++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/app/tests/test_engine/test_list_python_tests_classes.py b/app/tests/test_engine/test_list_python_tests_classes.py index f5e9b443..8d3ffe05 100644 --- a/app/tests/test_engine/test_list_python_tests_classes.py +++ b/app/tests/test_engine/test_list_python_tests_classes.py @@ -46,7 +46,7 @@ def _parse(source: str) -> ast.Module: return ast.parse(textwrap.dedent(source)) -def _make_sdk_folder(tmp_path: Path, files: dict[str, str]): +def _make_sdk_folder(tmp_path: Path, files: dict[str, str]) -> None: """Write *files* into *tmp_path* and return a mock SDKTestFolder.""" for name, content in files.items(): (tmp_path / name).write_text(textwrap.dedent(content)) diff --git a/app/tests/test_engine/test_python_test_initialization.py b/app/tests/test_engine/test_python_test_initialization.py index 3a9077d8..10df2ffc 100644 --- a/app/tests/test_engine/test_python_test_initialization.py +++ b/app/tests/test_engine/test_python_test_initialization.py @@ -25,17 +25,17 @@ import pytest from app.test_engine.test_script_manager import TestScriptManager -from test_collections.matter.sdk_tests.support.python_testing.list_python_tests_classes import ( # noqa +from test_collections.matter.sdk_tests.support.python_testing.list_python_tests_classes import ( generate_python_test_json_file, process_test_commands_with_container, -) +) # noqa from test_collections.matter.sdk_tests.support.python_testing.test_manager import ( _generate_all_test_files, ) @pytest.fixture(autouse=True) -def restore_singleton_state(): +def restore_singleton_state() -> None: """Save and restore TestScriptManager singleton state around each test. TestScriptManager is a singleton shared across the test session. @@ -379,11 +379,13 @@ async def test_generate_python_test_json_file_still_works(self) -> None: # Verify it calls the old process_commands_sdk_container function with patch( - "test_collections.matter.sdk_tests.support.python_testing.list_python_tests_classes.process_commands_sdk_container", + "test_collections.matter.sdk_tests.support.python_testing." + "list_python_tests_classes.process_commands_sdk_container", new_callable=AsyncMock, ) as mock_process: with patch( - "test_collections.matter.sdk_tests.support.python_testing.list_python_tests_classes.get_command_list" + "test_collections.matter.sdk_tests.support.python_testing." + "list_python_tests_classes.get_command_list" ) as mock_get_commands: mock_get_commands.return_value = [] @@ -392,4 +394,5 @@ async def test_generate_python_test_json_file_still_works(self) -> None: # Verify the old function was called assert ( mock_process.called - ), "Should call process_commands_sdk_container for backward compatibility" + ), "Should call process_commands_sdk_container for backward " + "compatibility" From 6eb1ff9b1ca834b9a39d8b1fd12aea9620fd3497 Mon Sep 17 00:00:00 2001 From: Romulo Quidute Filho Date: Mon, 2 Mar 2026 21:40:19 +0000 Subject: [PATCH 06/11] Code review - mypy/flake8 --- app/tests/test_engine/test_list_python_tests_classes.py | 2 +- app/tests/test_engine/test_python_test_initialization.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/tests/test_engine/test_list_python_tests_classes.py b/app/tests/test_engine/test_list_python_tests_classes.py index 8d3ffe05..33e91ac0 100644 --- a/app/tests/test_engine/test_list_python_tests_classes.py +++ b/app/tests/test_engine/test_list_python_tests_classes.py @@ -46,7 +46,7 @@ def _parse(source: str) -> ast.Module: return ast.parse(textwrap.dedent(source)) -def _make_sdk_folder(tmp_path: Path, files: dict[str, str]) -> None: +def _make_sdk_folder(tmp_path: Path, files: dict[str, str]) -> MagicMock: """Write *files* into *tmp_path* and return a mock SDKTestFolder.""" for name, content in files.items(): (tmp_path / name).write_text(textwrap.dedent(content)) diff --git a/app/tests/test_engine/test_python_test_initialization.py b/app/tests/test_engine/test_python_test_initialization.py index 10df2ffc..313bb485 100644 --- a/app/tests/test_engine/test_python_test_initialization.py +++ b/app/tests/test_engine/test_python_test_initialization.py @@ -20,6 +20,7 @@ 1. Phase 1: TestScriptManager constructor doesn't initialize Python tests 2. Phase 2: Python test generation uses single container session """ +from typing import Generator from unittest.mock import AsyncMock, MagicMock, Mock, patch import pytest @@ -28,14 +29,14 @@ from test_collections.matter.sdk_tests.support.python_testing.list_python_tests_classes import ( generate_python_test_json_file, process_test_commands_with_container, -) # noqa +) from test_collections.matter.sdk_tests.support.python_testing.test_manager import ( _generate_all_test_files, ) @pytest.fixture(autouse=True) -def restore_singleton_state() -> None: +def restore_singleton_state() -> Generator: """Save and restore TestScriptManager singleton state around each test. TestScriptManager is a singleton shared across the test session. From dc04c5dafb42bbcfdd277e1f772542be7b9e52fc Mon Sep 17 00:00:00 2001 From: Romulo Quidute Filho Date: Mon, 2 Mar 2026 21:46:18 +0000 Subject: [PATCH 07/11] Code review - flake8 --- app/tests/test_engine/test_python_test_initialization.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/tests/test_engine/test_python_test_initialization.py b/app/tests/test_engine/test_python_test_initialization.py index 313bb485..dfd86015 100644 --- a/app/tests/test_engine/test_python_test_initialization.py +++ b/app/tests/test_engine/test_python_test_initialization.py @@ -26,7 +26,7 @@ import pytest from app.test_engine.test_script_manager import TestScriptManager -from test_collections.matter.sdk_tests.support.python_testing.list_python_tests_classes import ( +from test_collections.matter.sdk_tests.support.python_testing.list_python_tests_classes import ( # noqa: E501 generate_python_test_json_file, process_test_commands_with_container, ) From 5cf308b76c3e28fcb4c07c5806f09764ba9ada23 Mon Sep 17 00:00:00 2001 From: Romulo Quidute Filho <116586593+rquidute@users.noreply.github.com> Date: Tue, 3 Mar 2026 09:29:30 -0300 Subject: [PATCH 08/11] Update test_collections/matter/sdk_tests/support/python_testing/list_python_tests_classes.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- .../support/python_testing/list_python_tests_classes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_collections/matter/sdk_tests/support/python_testing/list_python_tests_classes.py b/test_collections/matter/sdk_tests/support/python_testing/list_python_tests_classes.py index 8f30d10c..7b93c2a2 100644 --- a/test_collections/matter/sdk_tests/support/python_testing/list_python_tests_classes.py +++ b/test_collections/matter/sdk_tests/support/python_testing/list_python_tests_classes.py @@ -136,7 +136,7 @@ def _is_matter_base_test_class( class_name: str, module: ast.Module, search_dir: Optional[Path], - _visiting: Optional[set] = None, +_visiting: Optional[set[tuple[int, str]]] = None, ) -> bool: """Recursively check if a class name in a parsed module ultimately inherits from MatterBaseTest, following local file imports as needed. From 6162e8f6dce69cdd2f90c7cf700eb0dc89bf6da0 Mon Sep 17 00:00:00 2001 From: Romulo Quidute Filho <116586593+rquidute@users.noreply.github.com> Date: Tue, 3 Mar 2026 09:29:43 -0300 Subject: [PATCH 09/11] Update test_collections/matter/sdk_tests/support/python_testing/list_python_tests_classes.py Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- .../support/python_testing/list_python_tests_classes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_collections/matter/sdk_tests/support/python_testing/list_python_tests_classes.py b/test_collections/matter/sdk_tests/support/python_testing/list_python_tests_classes.py index 7b93c2a2..a0fc2cca 100644 --- a/test_collections/matter/sdk_tests/support/python_testing/list_python_tests_classes.py +++ b/test_collections/matter/sdk_tests/support/python_testing/list_python_tests_classes.py @@ -219,7 +219,7 @@ def _is_matter_base_test_class( return True continue - except SyntaxError: + logger.warning(f"Warning: Skipping {candidate} due to syntax error") pass # Fallback for installed packages whose source is not available as a From 73ec4a1ca882351b838dae6c0091352c374f5cce Mon Sep 17 00:00:00 2001 From: Romulo Quidute Filho Date: Tue, 3 Mar 2026 12:40:06 +0000 Subject: [PATCH 10/11] Code review - gemini code assist --- .../test_list_python_tests_classes.py | 1 - .../test_python_test_initialization.py | 23 +++------ .../list_python_tests_classes.py | 50 +++++++++++++++---- 3 files changed, 48 insertions(+), 26 deletions(-) diff --git a/app/tests/test_engine/test_list_python_tests_classes.py b/app/tests/test_engine/test_list_python_tests_classes.py index 33e91ac0..951cdcab 100644 --- a/app/tests/test_engine/test_list_python_tests_classes.py +++ b/app/tests/test_engine/test_list_python_tests_classes.py @@ -95,7 +95,6 @@ def test_skips_comment_lines(self, tmp_path: Path) -> None: ): result = load_ignore_list() assert result == {"TC_FOO.py"} - assert "# this is a comment" not in result def test_skips_blank_lines(self, tmp_path: Path) -> None: f = tmp_path / "ignore.txt" diff --git a/app/tests/test_engine/test_python_test_initialization.py b/app/tests/test_engine/test_python_test_initialization.py index dfd86015..c32fb600 100644 --- a/app/tests/test_engine/test_python_test_initialization.py +++ b/app/tests/test_engine/test_python_test_initialization.py @@ -122,15 +122,12 @@ async def test_initialize_python_tests_sets_flag(self) -> None: None, # custom_collection ) - try: - await manager.initialize_python_tests() - except Exception: - # May fail due to import issues in test environment, - # but we verify the attempt - pass + await manager.initialize_python_tests() - # The flag should be set to True after initialization attempt - # Note: In test environment this may not complete due to mocking + # Verify the flag is set to True after successful initialization + assert ( + manager._python_tests_initialized is True + ), "initialize_python_tests should set _python_tests_initialized to True" @pytest.mark.asyncio async def test_initialize_python_tests_updates_collections(self) -> None: @@ -150,14 +147,10 @@ async def test_initialize_python_tests_updates_collections(self) -> None: ) as mock_init: mock_init.return_value = (MagicMock(), MagicMock(), None) - try: - await manager.initialize_python_tests() + await manager.initialize_python_tests() - # Verify discover was called after initialization - assert mock_discover.called, "Should call discover_test_collections" - except Exception: - # May fail in test environment, but we verify the pattern - pass + # Verify discover was called after initialization + assert mock_discover.called, "Should call discover_test_collections" class TestPhase2SingleContainerSession: diff --git a/test_collections/matter/sdk_tests/support/python_testing/list_python_tests_classes.py b/test_collections/matter/sdk_tests/support/python_testing/list_python_tests_classes.py index a0fc2cca..760ab9bd 100644 --- a/test_collections/matter/sdk_tests/support/python_testing/list_python_tests_classes.py +++ b/test_collections/matter/sdk_tests/support/python_testing/list_python_tests_classes.py @@ -136,7 +136,8 @@ def _is_matter_base_test_class( class_name: str, module: ast.Module, search_dir: Optional[Path], -_visiting: Optional[set[tuple[int, str]]] = None, + _visiting: Optional[set[tuple[int, str]]] = None, + _module_cache: Optional[dict] = None, ) -> bool: """Recursively check if a class name in a parsed module ultimately inherits from MatterBaseTest, following local file imports as needed. @@ -145,13 +146,20 @@ def _is_matter_base_test_class( class_name: Name of the class to check. module: Parsed AST of the file where class_name is defined. search_dir: Directory to search for locally-imported modules. - _visiting: Set of (file, class) pairs already being resolved (cycle guard). + _visiting: Set of `(id(module), class_name)` or `(abs_path, class_name)` + tuples already being resolved (cycle guard). Same-module checks use + the object id; cross-file checks use the absolute file path so that + separately parsed copies of the same file are correctly identified. + _module_cache: Cache of already-parsed AST modules keyed by absolute + file path, to avoid redundant disk reads. Returns: bool: True if the class transitively inherits from MatterBaseTest. """ if _visiting is None: _visiting = set() + if _module_cache is None: + _module_cache = {} # Find the class definition in this module class_def = next( @@ -168,7 +176,7 @@ def _is_matter_base_test_class( key = (id(module), class_name) if key in _visiting: return False - _visiting = _visiting | {key} + _visiting.add(key) for base in class_def.bases: if not isinstance(base, ast.Name): @@ -180,7 +188,9 @@ def _is_matter_base_test_class( return True # Check if base_name is defined in the same module (local parent class) - if _is_matter_base_test_class(base_name, module, search_dir, _visiting): + if _is_matter_base_test_class( + base_name, module, search_dir, _visiting, _module_cache + ): return True # Try to resolve base_name via imports in this module @@ -201,8 +211,21 @@ def _is_matter_base_test_class( candidate = search_dir / f"{node.module.replace('.', '/')}.py" if candidate.exists(): try: - with open(candidate, "r") as f: - imported_module = ast.parse(f.read()) + abs_path = str(candidate.resolve()) + if abs_path not in _module_cache: + with open(candidate, "r") as f: + _module_cache[abs_path] = ast.parse(f.read()) + imported_module = _module_cache[abs_path] + + # Use the absolute path as the cycle-guard key so that + # re-parsed copies of the same file are treated as + # identical, preventing infinite recursion on cross-file + # circular imports. + file_key = (abs_path, class_name) + if file_key in _visiting: + continue + _visiting.add(file_key) + # The actual class name in the imported file may differ # (e.g. `from TC_TLSCERT_Base import TC_TLSCERT_Base`) actual_name = next( @@ -214,13 +237,17 @@ def _is_matter_base_test_class( base_name, ) if _is_matter_base_test_class( - actual_name, imported_module, search_dir, _visiting + actual_name, + imported_module, + search_dir, + _visiting, + _module_cache, ): return True continue - logger.warning(f"Warning: Skipping {candidate} due to syntax error") - pass + except SyntaxError: + logger.warning(f"Skipping {candidate} due to syntax error") # Fallback for installed packages whose source is not available as a # local file (e.g. matter.testing.* installed as a wheel). @@ -249,11 +276,14 @@ def base_test_classes( list[ast.ClassDef]: Classes in the module that ultimately inherit from MatterBaseTest. """ + module_cache: dict = {} return [ c for c in module.body if isinstance(c, ast.ClassDef) - and _is_matter_base_test_class(c.name, module, search_dir) + and _is_matter_base_test_class( + c.name, module, search_dir, _module_cache=module_cache + ) ] From 5bbe3066b51edb2d074be1b56985aef11bc4f249 Mon Sep 17 00:00:00 2001 From: Romulo Quidute Filho Date: Tue, 3 Mar 2026 12:52:16 +0000 Subject: [PATCH 11/11] Code review --- app/tests/test_engine/test_python_test_initialization.py | 3 ++- .../support/python_testing/list_python_tests_classes.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/tests/test_engine/test_python_test_initialization.py b/app/tests/test_engine/test_python_test_initialization.py index c32fb600..fe2ae5bb 100644 --- a/app/tests/test_engine/test_python_test_initialization.py +++ b/app/tests/test_engine/test_python_test_initialization.py @@ -127,7 +127,8 @@ async def test_initialize_python_tests_sets_flag(self) -> None: # Verify the flag is set to True after successful initialization assert ( manager._python_tests_initialized is True - ), "initialize_python_tests should set _python_tests_initialized to True" + ), "initialize_python_tests should set _python_tests_initialized " + "to True" @pytest.mark.asyncio async def test_initialize_python_tests_updates_collections(self) -> None: diff --git a/test_collections/matter/sdk_tests/support/python_testing/list_python_tests_classes.py b/test_collections/matter/sdk_tests/support/python_testing/list_python_tests_classes.py index 760ab9bd..d896df5c 100644 --- a/test_collections/matter/sdk_tests/support/python_testing/list_python_tests_classes.py +++ b/test_collections/matter/sdk_tests/support/python_testing/list_python_tests_classes.py @@ -136,7 +136,7 @@ def _is_matter_base_test_class( class_name: str, module: ast.Module, search_dir: Optional[Path], - _visiting: Optional[set[tuple[int, str]]] = None, + _visiting: Optional[set[tuple]] = None, _module_cache: Optional[dict] = None, ) -> bool: """Recursively check if a class name in a parsed module ultimately inherits