Code review

project-chip · Jan 3, 2025 · 7cc1017 · 7cc1017
1 parent d909809
commit 7cc1017
Show file tree

Hide file tree

Showing 6 changed files with 21 additions and 42 deletions.
diff --git a/app/container_manager/container_manager.py b/app/container_manager/container_manager.py
@@ -129,18 +129,18 @@ def copy_file_from_container(
         container: Container,
         container_file_path: Path,
         destination_path: Path,
-        container_file_name: str,
+        destination_file_name: str,
     ) -> None:
         try:
             logger.info(
                 "### File Copy: CONTAINER->HOST"
                 f" From Container Path: {str(container_file_path)}"
-                f" To Host Path: {str(destination_path)}/{container_file_name}"
+                f" To Host Path: {str(destination_path)}/{destination_file_name}"
                 f" Container Name: {str(container.name)}"
             )
             stream, _ = container.get_archive(str(container_file_path))
             with open(
-                f"{str(destination_path)}/{container_file_name}",
+                f"{str(destination_path)}/{destination_file_name}",
                 "wb",
             ) as f:
                 for d in stream:

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
@@ -42,10 +42,9 @@
     SDKPythonTestResultBase,
     SDKPythonTestRunnerHooks,
 )
+from .utils import EXECUTABLE, RUNNER_CLASS_PATH, DUTCommissioningError
+from .utils import PromptOption as PromptOptionUtils
 from .utils import (
-    EXECUTABLE,
-    RUNNER_CLASS_PATH,
-    DUTCommissioningError,
     commission_device,
     generate_command_arguments,
     should_perform_new_commissioning,
@@ -348,7 +347,7 @@ async def setup(self) -> None:
         user_response = await prompt_for_commissioning_mode(
             self, logger, None, self.cancel
         )
-        if user_response == PromptOption.NO:
+        if user_response == PromptOptionUtils.FAIL:
             raise DUTCommissioningError(
                 "User chose prompt option FAILED for DUT is in Commissioning Mode"
             )
@@ -361,7 +360,7 @@ async def setup(self) -> None:
         user_response = await prompt_for_commissioning_mode(
             self, logger, None, self.cancel
         )
-        if user_response == PromptOption.NO:
+        if user_response == PromptOptionUtils.FAIL:
             raise DUTCommissioningError(
                 "User chose prompt option FAILED for DUT is in Commissioning Mode"
             )

diff --git a/test_collections/matter/sdk_tests/support/python_testing/models/utils.py b/test_collections/matter/sdk_tests/support/python_testing/models/utils.py
@@ -138,7 +138,7 @@ def __copy_admin_storage_file(
     sdk_container.copy_file_from_container(
         container_file_path=Path(storage_path),
         destination_path=ADMIN_STORAGE_FILE_HOST_PATH,
-        container_file_name=ADMIN_STORAGE_FILE_DEFAULT_NAME,
+        destination_file_name=ADMIN_STORAGE_FILE_DEFAULT_NAME,
     )
 
 

diff --git a/test_collections/matter/sdk_tests/support/sdk_container.py b/test_collections/matter/sdk_tests/support/sdk_container.py
@@ -296,13 +296,13 @@ def copy_file_from_container(
         self,
         container_file_path: Path,
         destination_path: Path,
-        container_file_name: str,
+        destination_file_name: str,
     ) -> None:
         container_manager.copy_file_from_container(
             container=self.__container,
             container_file_path=container_file_path,
             destination_path=destination_path,
-            container_file_name=container_file_name,
+            destination_file_name=destination_file_name,
         )
 
     def copy_file_to_container(

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
@@ -32,10 +32,9 @@
 from ...models.matter_test_models import MatterTestStep, MatterTestType
 from ...python_testing.models import PythonTestCase
 from ...python_testing.models.python_test_models import PythonTest, PythonTestType
-from ...python_testing.models.test_case import LegacyPythonTestCase, PromptOption
+from ...python_testing.models.test_case import LegacyPythonTestCase
 from ...python_testing.models.utils import DUTCommissioningError
-
-# from ...utils import PromptOption
+from ...utils import PromptOption
 
 
 def python_test_instance(
@@ -227,7 +226,7 @@ async def test_should_raise_DUTCommissioningError_prompt_commissioning_failed()
     with mock.patch(
         "test_collections.matter.sdk_tests.support.python_testing.models.test_case"
         ".prompt_for_commissioning_mode",
-        return_value=PromptOption.NO,
+        return_value=PromptOption.FAIL,
     ), pytest.raises(DUTCommissioningError):
         await test_case.setup()
 
@@ -295,15 +294,15 @@ async def test_legacy_python_test_case_no_new_commissioning() -> None:
     )(test_case_execution)
 
     mock_prompt_response_no = mock.Mock()
-    mock_prompt_response_no.response = PromptOption.NO
+    mock_prompt_response_no.response = PromptOption.FAIL
 
     mock_config = mock.Mock()
     mock_config.__dict__ = project.config
 
     with mock.patch(
         "test_collections.matter.sdk_tests.support.python_testing.models.test_case"
         ".prompt_for_commissioning_mode",
-        return_value=PromptOption.YES,
+        return_value=PromptOption.PASS,
     ) as mock_prompt_commissioning, mock.patch(
         "test_collections.matter.sdk_tests.support.python_testing.models.utils"
         ".commission_device"
@@ -386,10 +385,10 @@ async def test_legacy_python_test_case_new_commissioning() -> None:
     )(test_case_execution)
 
     mock_prompt_response_yes = mock.Mock()
-    mock_prompt_response_yes.response = PromptOption.YES
+    mock_prompt_response_yes.response = PromptOption.PASS
 
     mock_prompt_response_no = mock.Mock()
-    mock_prompt_response_no.response = PromptOption.NO
+    mock_prompt_response_no.response = PromptOption.FAIL
 
     mock_config = mock.Mock()
     mock_config.__dict__ = project.config
@@ -400,7 +399,7 @@ async def test_legacy_python_test_case_new_commissioning() -> None:
     with mock.patch(
         "test_collections.matter.sdk_tests.support.python_testing.models.test_case"
         ".prompt_for_commissioning_mode",
-        return_value=PromptOption.YES,
+        return_value=PromptOption.PASS,
     ) as mock_prompt_commissioning, mock.patch(
         "test_collections.matter.sdk_tests.support.python_testing.models.test_case"
         ".commission_device",

diff --git a/test_collections/matter/sdk_tests/support/tests/python_tests/test_python_test_suite.py b/test_collections/matter/sdk_tests/support/tests/python_tests/test_python_test_suite.py
@@ -25,6 +25,9 @@
 from app.schemas import PICS
 from app.test_engine.logger import test_engine_logger
 from app.tests.utils.test_pics_data import create_random_pics
+from test_collections.matter.sdk_tests.support.python_testing.models.utils import (
+    DUTCommissioningError,
+)
 
 from ...python_testing.models.test_suite import (
     CommissioningPythonTestSuite,
@@ -338,9 +341,6 @@ async def test_commissioning_suite_setup_fail() -> None:
     """Test that when prompt_for_commissioning_mode returns FAIL, the setup process
     should raise DUTCommissioningError.
     """
-    from test_collections.matter.sdk_tests.support.python_testing.models.utils import (
-        DUTCommissioningError,
-    )
 
     suite_class: Type[PythonTestSuite] = PythonTestSuite.class_factory(
         suite_type=SuiteType.COMMISSIONING,
@@ -400,10 +400,6 @@ async def test_should_perform_new_commissioning_yes() -> None:
 
     suite_instance = suite_class(TestSuiteExecution())
 
-    # Mock prompt response para "NO" - No commissioning
-    mock_prompt_response = mock.Mock()
-    mock_prompt_response.response = PromptOption.FAIL
-
     with mock.patch(
         "test_collections.matter.sdk_tests.support.python_testing.models.test_suite"
         ".PythonTestSuite.setup"
@@ -419,9 +415,6 @@ async def test_should_perform_new_commissioning_yes() -> None:
         ".PythonTestSuite.config",
         new_callable=mock.PropertyMock,
         return_value=default_environment_config.__dict__,
-    ), mock.patch(
-        "app.user_prompt_support.user_prompt_support.UserPromptSupport.send_prompt_request",
-        return_value=mock_prompt_response,
     ), mock.patch(
         "test_collections.matter.sdk_tests.support.python_testing.models.test_suite"
         ".should_perform_new_commissioning",
@@ -430,14 +423,9 @@ async def test_should_perform_new_commissioning_yes() -> None:
         await suite_instance.setup()
 
         mock_should_perform_new_commissioning.assert_called_once()
-
         python_suite_setup.assert_called_once()
-
         mock_prompt_commissioning.assert_called_once()
-        assert mock_prompt_commissioning.return_value == PromptOption.PASS
-
         mock_commission_device.assert_called_once()
-        assert mock_commission_device.call_count == 1
 
 
 @pytest.mark.asyncio
@@ -454,10 +442,6 @@ async def test_should_perform_new_commissioning_no() -> None:
 
     suite_instance = suite_class(TestSuiteExecution())
 
-    # Mock prompt response para "YES" - Perform the commissioning
-    mock_prompt_response = mock.Mock()
-    mock_prompt_response.response = PromptOption.PASS
-
     with mock.patch(
         "test_collections.matter.sdk_tests.support.python_testing.models.test_suite"
         ".PythonTestSuite.setup"
@@ -473,9 +457,6 @@ async def test_should_perform_new_commissioning_no() -> None:
         ".PythonTestSuite.config",
         new_callable=mock.PropertyMock,
         return_value=default_environment_config.__dict__,
-    ), mock.patch(
-        "app.user_prompt_support.user_prompt_support.UserPromptSupport.send_prompt_request",
-        return_value=mock_prompt_response,
     ), mock.patch(
         "test_collections.matter.sdk_tests.support.python_testing.models.test_suite"
         ".should_perform_new_commissioning",