From 62fae75418aa1b4e6c3f1565e8512f3dcc0fc2f1 Mon Sep 17 00:00:00 2001 From: aamj Date: Thu, 20 Aug 2026 13:44:54 -0300 Subject: [PATCH] Adding THconfig schema as a new TH config field --- app/schemas/test_environment_config.py | 10 ++ .../user_prompts/test_prompt_exchange.py | 25 +++- .../user_prompts/test_user_prompt_support.py | 108 ++++++++++++++++++ app/user_prompt_support/prompt_request.py | 5 +- .../user_prompt_manager.py | 7 +- .../user_prompt_support.py | 22 +++- .../matter/default_project.config | 6 +- test_collections/matter/python_tests | 2 +- .../python_testing/models/test_case.py | 20 +++- .../matter/test_test_environment_config.py | 25 ++++ .../python_tests/test_python_test_case.py | 56 +++++++++ .../sdk_tests/support/tests/utils/utils.py | 8 ++ .../support/yaml_tests/models/chip_suite.py | 8 +- .../support/yaml_tests/models/chip_test.py | 3 +- 14 files changed, 287 insertions(+), 18 deletions(-) diff --git a/app/schemas/test_environment_config.py b/app/schemas/test_environment_config.py index e1845422..b2f5b1b5 100644 --- a/app/schemas/test_environment_config.py +++ b/app/schemas/test_environment_config.py @@ -48,11 +48,21 @@ class TestEnvironmentConfigError(Exception): """ +class THConfig(BaseModel): + """Test-Harness-specific settings, independent of any particular DUT/program.""" + + prompt_timeout_seconds: int = 60 + # None means "not set at project level", deferring to the + # ENABLE_REALTIME_PYTHON_TEST_LOGS environment variable. + enable_realtime_python_test_logs: Optional[bool] = None + + class TestEnvironmentConfig(BaseModel): __test__ = False # Needed to indicate to PyTest that this is not a "test" # TODO(#490): Need to be refactored to support real PIXIT format test_parameters: Optional[dict[str, Any]] + th_config: Optional[THConfig] = None def __init__(self, **kwargs: Any): try: diff --git a/app/tests/user_prompts/test_prompt_exchange.py b/app/tests/user_prompts/test_prompt_exchange.py index f666e95f..b4d621da 100644 --- a/app/tests/user_prompts/test_prompt_exchange.py +++ b/app/tests/user_prompts/test_prompt_exchange.py @@ -21,11 +21,34 @@ from app.user_prompt_support import user_prompt_manager from app.user_prompt_support.constants import UserResponseStatusEnum -from app.user_prompt_support.prompt_request import PromptRequest +from app.user_prompt_support.prompt_request import PromptRequest, default_timeout_s from app.user_prompt_support.prompt_response import PromptResponse from app.user_prompt_support.user_prompt_manager import PromptExchange +def test_prompt_request_default_timeout_is_unresolved() -> None: + """ + PromptRequest.timeout defaults to None (unresolved), not a concrete value. + + UserPromptSupport.send_prompt_request() is responsible for resolving it from + th_config before dispatch; PromptExchange applies a defensive fallback to + default_timeout_s if a request ever reaches it unresolved (see below). + """ + assert PromptRequest(prompt="Test string").timeout is None + + +def test_prompt_exchange_resolves_unset_timeout_to_default() -> None: + """ + PromptExchange defensively resolves an unset (None) timeout to + default_timeout_s, so a request bypassing UserPromptSupport's funnel never + reaches wait_for() with an unresolved timeout. + """ + request: PromptRequest = PromptRequest(prompt="Test string") + exchange: PromptExchange = PromptExchange(prompt=request, message_id=0) + + assert exchange.prompt.timeout == default_timeout_s + + def test_prompt_exchange_handle_empty_response() -> None: """ This tests the handle_response() by passing an empty message dictionary. diff --git a/app/tests/user_prompts/test_user_prompt_support.py b/app/tests/user_prompts/test_user_prompt_support.py index 9354c595..3989b479 100644 --- a/app/tests/user_prompts/test_user_prompt_support.py +++ b/app/tests/user_prompts/test_user_prompt_support.py @@ -18,6 +18,9 @@ import pytest from app.user_prompt_support import user_prompt_manager +from app.user_prompt_support.constants import UserResponseStatusEnum +from app.user_prompt_support.prompt_request import PromptRequest, default_timeout_s +from app.user_prompt_support.prompt_response import PromptResponse from app.user_prompt_support.user_prompt_support import ( UserPromptError, UserPromptSupport, @@ -39,3 +42,108 @@ async def test_send_prompt_request_no_response() -> None: with pytest.raises(UserPromptError): await prompt_support.send_prompt_request(prompt_request=mock.MagicMock()) send_prompt_request.assert_called_once() + + +async def _send_and_capture_timeout( + prompt_support: UserPromptSupport, prompt_request: PromptRequest +) -> int: + """Send prompt_request and return the timeout that reached user_prompt_manager.""" + captured: dict = {} + + async def _capture(prompt_request: PromptRequest) -> PromptResponse: + captured["timeout"] = prompt_request.timeout + return PromptResponse(status_code=UserResponseStatusEnum.OKAY, response="ok") + + with mock.patch.object( + user_prompt_manager.user_prompt_manager, + "send_prompt_request", + side_effect=_capture, + ): + await prompt_support.send_prompt_request(prompt_request=prompt_request) + + return captured["timeout"] + + +@pytest.mark.asyncio +async def test_send_prompt_request_resolves_timeout_from_config() -> None: + """An unspecified timeout is resolved from th_config.prompt_timeout_seconds.""" + prompt_support = UserPromptSupport() + prompt_support.config = {"th_config": {"prompt_timeout_seconds": 300}} + + timeout = await _send_and_capture_timeout( + prompt_support, PromptRequest(prompt="hi") + ) + + assert timeout == 300 + + +@pytest.mark.asyncio +async def test_send_prompt_request_falls_back_without_config_attribute() -> None: + """No `.config` attribute at all (e.g. TestStep-based mixins) falls back safely.""" + prompt_support = UserPromptSupport() + + timeout = await _send_and_capture_timeout( + prompt_support, PromptRequest(prompt="hi") + ) + + assert timeout == default_timeout_s + + +@pytest.mark.asyncio +async def test_send_prompt_request_falls_back_with_empty_config() -> None: + """An empty/absent th_config key falls back to the default timeout.""" + prompt_support = UserPromptSupport() + prompt_support.config = {} + + timeout = await _send_and_capture_timeout( + prompt_support, PromptRequest(prompt="hi") + ) + + assert timeout == default_timeout_s + + +@pytest.mark.asyncio +async def test_send_prompt_request_falls_back_with_null_th_config() -> None: + """A hand-edited "th_config": null must not raise AttributeError.""" + prompt_support = UserPromptSupport() + prompt_support.config = {"th_config": None} + + timeout = await _send_and_capture_timeout( + prompt_support, PromptRequest(prompt="hi") + ) + + assert timeout == default_timeout_s + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "prompt_timeout_seconds", + ["not-a-number", 0, -5, True], +) +async def test_send_prompt_request_falls_back_with_malformed_value( + prompt_timeout_seconds: object, +) -> None: + """A malformed/invalid configured value falls back safely instead of crashing.""" + prompt_support = UserPromptSupport() + prompt_support.config = { + "th_config": {"prompt_timeout_seconds": prompt_timeout_seconds} + } + + timeout = await _send_and_capture_timeout( + prompt_support, PromptRequest(prompt="hi") + ) + + assert timeout == default_timeout_s + + +@pytest.mark.asyncio +async def test_send_prompt_request_explicit_timeout_wins_over_config() -> None: + """A caller-supplied timeout is never overridden by th_config.""" + prompt_support = UserPromptSupport() + prompt_support.config = {"th_config": {"prompt_timeout_seconds": 300}} + + timeout = await _send_and_capture_timeout( + prompt_support, PromptRequest(prompt="hi", timeout=5) + ) + + assert timeout == 5 diff --git a/app/user_prompt_support/prompt_request.py b/app/user_prompt_support/prompt_request.py index 757a6682..c2d83bd1 100644 --- a/app/user_prompt_support/prompt_request.py +++ b/app/user_prompt_support/prompt_request.py @@ -24,7 +24,10 @@ class PromptRequest(BaseModel): prompt: Optional[str] - timeout: int = default_timeout_s + # None means "caller did not specify a timeout"; UserPromptSupport.send_prompt_request + # resolves it (from the project's th_config, falling back to default_timeout_s) before + # the request is dispatched. + timeout: Optional[int] = None @property def messageType(self) -> MessageTypeEnum: diff --git a/app/user_prompt_support/user_prompt_manager.py b/app/user_prompt_support/user_prompt_manager.py index 58a18b4d..382af4b5 100644 --- a/app/user_prompt_support/user_prompt_manager.py +++ b/app/user_prompt_support/user_prompt_manager.py @@ -30,7 +30,7 @@ UserResponseStatusEnum, ) -from .prompt_request import PromptRequest +from .prompt_request import PromptRequest, default_timeout_s from .prompt_response import PromptResponse @@ -40,6 +40,11 @@ class PromptExchange(object): def __init__(self, prompt: PromptRequest, message_id: int) -> None: self.message_id = message_id + # Callers are expected to resolve a concrete timeout before dispatch (see + # UserPromptSupport.send_prompt_request); this is a defensive fallback so an + # unresolved timeout never reaches wait_for() or gets sent to the frontend. + if prompt.timeout is None: + prompt = prompt.copy(update={"timeout": default_timeout_s}) self.prompt = prompt self.message_event = Event() self.received_response: Optional[PromptResponse] diff --git a/app/user_prompt_support/user_prompt_support.py b/app/user_prompt_support/user_prompt_support.py index 44edae40..42984a87 100644 --- a/app/user_prompt_support/user_prompt_support.py +++ b/app/user_prompt_support/user_prompt_support.py @@ -14,7 +14,7 @@ # limitations under the License. # from .constants import UserResponseStatusEnum -from .prompt_request import PromptRequest +from .prompt_request import PromptRequest, default_timeout_s from .prompt_response import PromptResponse from .user_prompt_manager import user_prompt_manager @@ -31,12 +31,32 @@ class UserPromptSupport(object): async def send_prompt_request( self, prompt_request: PromptRequest ) -> PromptResponse: + if prompt_request.timeout is None: + prompt_request = prompt_request.copy( + update={"timeout": self.__resolve_prompt_timeout()} + ) + response = await user_prompt_manager.send_prompt_request(prompt_request) if response is None: raise UserPromptError("No prompt response returned") return response + def __resolve_prompt_timeout(self) -> int: + """Resolve the default prompt timeout from the project's th_config. + + Falls back to default_timeout_s when no config is available (e.g. this + mixin is used from a class with no `.config` property) or the configured + value is missing/invalid. + """ + config = getattr(self, "config", None) + if config: + th_config = config.get("th_config") or {} + value = th_config.get("prompt_timeout_seconds") + if isinstance(value, int) and not isinstance(value, bool) and value > 0: + return value + return default_timeout_s + async def invoke_prompt_and_get_str_response( self, prompt_request: PromptRequest ) -> str: diff --git a/test_collections/matter/default_project.config b/test_collections/matter/default_project.config index 5a4a103f..7245f605 100644 --- a/test_collections/matter/default_project.config +++ b/test_collections/matter/default_project.config @@ -28,5 +28,9 @@ "chip_use_paa_certs":false, "trace_log":true }, - "test_parameters": null + "test_parameters": null, + "th_config": { + "prompt_timeout_seconds": 60, + "enable_realtime_python_test_logs": null + } } diff --git a/test_collections/matter/python_tests b/test_collections/matter/python_tests index f3122943..70e22f5d 160000 --- a/test_collections/matter/python_tests +++ b/test_collections/matter/python_tests @@ -1 +1 @@ -Subproject commit f3122943854679cdc7208f260ef544bb70a72e0d +Subproject commit 70e22f5dfd626ed49963abffc9c027a5b728edeb diff --git a/test_collections/matter/sdk_tests/support/python_testing/models/test_case.py b/test_collections/matter/sdk_tests/support/python_testing/models/test_case.py index d9a40508..d9c7e594 100644 --- a/test_collections/matter/sdk_tests/support/python_testing/models/test_case.py +++ b/test_collections/matter/sdk_tests/support/python_testing/models/test_case.py @@ -145,9 +145,21 @@ async def step_success( self, logger: Any, logs: str, duration: int, request: Any ) -> None: # Display logs captured during this step only if real-time logging is enabled - if settings.ENABLE_REALTIME_PYTHON_TEST_LOGS: + if self._realtime_logs_enabled(): await self._display_step_logs() + def _realtime_logs_enabled(self) -> bool: + """Whether Python test logs should be displayed incrementally per step. + + The project's th_config.enable_realtime_python_test_logs, when explicitly + set, overrides the instance-wide ENABLE_REALTIME_PYTHON_TEST_LOGS env var. + """ + th_config = (self.config or {}).get("th_config") or {} + override = th_config.get("enable_realtime_python_test_logs") + if override is not None: + return bool(override) + return settings.ENABLE_REALTIME_PYTHON_TEST_LOGS + async def _display_step_logs(self) -> None: """Display logs that were captured during the current step.""" # Validate file path is set and file exists @@ -265,7 +277,7 @@ async def step_failure( ) -> None: # Display logs captured during this step before marking as failure # only if real-time logging is enabled - if settings.ENABLE_REALTIME_PYTHON_TEST_LOGS: + if self._realtime_logs_enabled(): await self._display_step_logs() failure_msg = "Python test step failure" @@ -462,7 +474,7 @@ async def cleanup(self) -> None: logger.info("Test Cleanup") # Log any remaining content that wasn't captured by steps # only if real-time logging is enabled - if settings.ENABLE_REALTIME_PYTHON_TEST_LOGS: + if self._realtime_logs_enabled(): await self._log_remaining_content() else: # Use batch logging when real-time logging is disabled @@ -644,7 +656,7 @@ async def execute(self) -> None: # Check for any remaining logs that weren't captured by steps # or show all logs if real-time logging is disabled - if settings.ENABLE_REALTIME_PYTHON_TEST_LOGS: + if self._realtime_logs_enabled(): await self._log_remaining_content() else: # Use batch logging when real-time logging is disabled diff --git a/test_collections/matter/sdk_tests/support/tests/matter/test_test_environment_config.py b/test_collections/matter/sdk_tests/support/tests/matter/test_test_environment_config.py index 52371237..fe0b00a2 100644 --- a/test_collections/matter/sdk_tests/support/tests/matter/test_test_environment_config.py +++ b/test_collections/matter/sdk_tests/support/tests/matter/test_test_environment_config.py @@ -28,6 +28,7 @@ default_config_thread_no_ba_port, default_config_thread_valid, default_matter_config, + default_matter_config_with_th_config, ) from test_collections.matter.test_environment_config import TestEnvironmentConfigMatter @@ -38,6 +39,30 @@ def test_create_config_matter_with_valid_config_success() -> None: assert config_matter is not None +def test_create_config_matter_with_no_th_config_defaults_to_none() -> None: + config_matter = TestEnvironmentConfigMatter(**default_matter_config) + + assert config_matter.th_config is None + + +def test_create_config_matter_with_th_config_round_trips() -> None: + config_matter = TestEnvironmentConfigMatter(**default_matter_config_with_th_config) + + assert config_matter.th_config is not None + assert config_matter.th_config.prompt_timeout_seconds == 120 + assert config_matter.th_config.enable_realtime_python_test_logs is True + + +def test_create_config_matter_with_invalid_th_config_type_fails() -> None: + config = { + **default_matter_config, + "th_config": {"prompt_timeout_seconds": "not-a-number"}, + } + + with pytest.raises(TestEnvironmentConfigError): + TestEnvironmentConfigMatter(**config) + + def test_create_config_matter_with_no_config_fails() -> None: with pytest.raises(TestEnvironmentConfigError) as e: TestEnvironmentConfigMatter() diff --git a/test_collections/matter/sdk_tests/support/tests/python_tests/test_python_test_case.py b/test_collections/matter/sdk_tests/support/tests/python_tests/test_python_test_case.py index 0f97df64..bd6e835d 100644 --- a/test_collections/matter/sdk_tests/support/tests/python_tests/test_python_test_case.py +++ b/test_collections/matter/sdk_tests/support/tests/python_tests/test_python_test_case.py @@ -764,3 +764,59 @@ async def _capture_config( assert len(captured_configs) == 1 assert "qr-code" in (captured_configs[0].test_parameters or {}) + + +def _instance_with_project_config(project_config: dict) -> PythonTestCase: + """Build a PythonTestCase instance whose `.config` resolves to project_config.""" + project = Project(name="test_project") + project.config = project_config + + test_run_execution = TestRunExecution() + test_run_execution.project = project + + test_suite_execution = TestSuiteExecution() + test_suite_execution.test_run_execution = test_run_execution + + test_case_execution = TestCaseExecution() + test_case_execution.test_suite_execution = test_suite_execution + + test = python_test_instance() + case_class: Type[PythonTestCase] = PythonTestCase.class_factory( + test=test, python_test_version="version", mandatory=False + ) + return case_class(test_case_execution) + + +@pytest.mark.parametrize( + "th_config_value, env_value, expected", + [ + (True, False, True), # config override True beats env False + (False, True, False), # config override False beats env True + (None, True, True), # unset config defers to env True + (None, False, False), # unset config defers to env False + ], +) +def test_realtime_logs_enabled_matrix( + th_config_value: Optional[bool], env_value: bool, expected: bool +) -> None: + instance = _instance_with_project_config( + {"th_config": {"enable_realtime_python_test_logs": th_config_value}} + ) + + with mock.patch( + "test_collections.matter.sdk_tests.support.python_testing.models.test_case" + ".settings" + ) as mock_settings: + mock_settings.ENABLE_REALTIME_PYTHON_TEST_LOGS = env_value + assert instance._realtime_logs_enabled() is expected + + +def test_realtime_logs_enabled_defers_to_env_when_th_config_absent() -> None: + instance = _instance_with_project_config({}) + + with mock.patch( + "test_collections.matter.sdk_tests.support.python_testing.models.test_case" + ".settings" + ) as mock_settings: + mock_settings.ENABLE_REALTIME_PYTHON_TEST_LOGS = True + assert instance._realtime_logs_enabled() is True diff --git a/test_collections/matter/sdk_tests/support/tests/utils/utils.py b/test_collections/matter/sdk_tests/support/tests/utils/utils.py index 8fb2d8ee..2cc367ac 100644 --- a/test_collections/matter/sdk_tests/support/tests/utils/utils.py +++ b/test_collections/matter/sdk_tests/support/tests/utils/utils.py @@ -42,6 +42,14 @@ "test_parameters": None, } +default_matter_config_with_th_config = { + **default_matter_config, + "th_config": { + "prompt_timeout_seconds": 120, + "enable_realtime_python_test_logs": True, + }, +} + default_config_no_dut = { "network": { "fabric_id": "0", diff --git a/test_collections/matter/sdk_tests/support/yaml_tests/models/chip_suite.py b/test_collections/matter/sdk_tests/support/yaml_tests/models/chip_suite.py index 5936c193..d193ac05 100644 --- a/test_collections/matter/sdk_tests/support/yaml_tests/models/chip_suite.py +++ b/test_collections/matter/sdk_tests/support/yaml_tests/models/chip_suite.py @@ -353,9 +353,7 @@ async def __prompt_user_to_perform_commission(self) -> None: Example: pairing code """ - prompt_request = OptionsSelectPromptRequest( - prompt=prompt, options=options, timeout=60 - ) + prompt_request = OptionsSelectPromptRequest(prompt=prompt, options=options) prompt_response = await self.send_prompt_request(prompt_request) match prompt_response.response: @@ -383,9 +381,7 @@ async def __prompt_user_to_perform_decommission(self) -> None: Example: pairing unpair """ - prompt_request = OptionsSelectPromptRequest( - prompt=prompt, options=options, timeout=60 - ) + prompt_request = OptionsSelectPromptRequest(prompt=prompt, options=options) prompt_response = await self.send_prompt_request(prompt_request) match prompt_response.response: diff --git a/test_collections/matter/sdk_tests/support/yaml_tests/models/chip_test.py b/test_collections/matter/sdk_tests/support/yaml_tests/models/chip_test.py index c2326340..7522404a 100644 --- a/test_collections/matter/sdk_tests/support/yaml_tests/models/chip_test.py +++ b/test_collections/matter/sdk_tests/support/yaml_tests/models/chip_test.py @@ -41,7 +41,6 @@ from ...sdk_container import SDKContainer from ...yaml_tests.matter_yaml_runner import MatterYAMLRunner -CHIP_TOOL_DEFAULT_PROMPT_TIMEOUT_S = 60 # seconds OUTCOME_TIMEOUT_S = 60 * 10 # Seconds EXTENDED_PROMPT_TIMEOUT_S = 300 @@ -305,7 +304,7 @@ async def __prompt_user_for_controller_action(self, action: str) -> None: """ prompt = f"Please do the following action on the Controller: {action}" - prompt_request = MessagePromptRequest(prompt=prompt, timeout=60) + prompt_request = MessagePromptRequest(prompt=prompt) await self.send_prompt_request(prompt_request) def __handle_logs(self, logs: Any) -> None: