Merge pull request #170 from LedgerHQ/navins/extend

BaseNavInsID in order to allow better integration for custom Na…

.github/workflows/build_and_tests.yml

@@ -27,6 +27,7 @@ jobs:
     needs: [build_boilerplate_application]
     runs-on: ubuntu-latest
     strategy:
+      fail-fast: false
       matrix:
         python_version: ['3.8', '3.9', '3.10', '3.11']
 

CHANGELOG.md

@@ -5,6 +5,13 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+
+## [1.15.0] - 2024-03-??
+
+### Added
+- navigator: `BaseNavInsID` allows to create custom `NavInsID` while using `Navigator` methods
+             without additional type conversion.
+
 ## [1.14.4] - 2024-02-21
 
 ### Fixed
@@ -209,8 +216,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [1.4.0] - 2023-02-09
 
-### Added
-
 ### Changed
 - template: conftest.py: Target nanosp release 1.0.4 which is now supported by speculos
 - backend: speculos.py: Do not pass `--sdk` argument anymore, this enforce using either last speculos supported SDK or app elf file with metadata.
@@ -224,10 +229,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [1.3.1] - 2023-01-24
 
-### Added
-
-### Changed
-
 ### Fixed
 
 - backend/speculos : improved reliability of wait_for_screen_change
@@ -237,8 +238,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [1.3.0] - 2023-01-18
 
-### Added
-
 ### Changed
 - template: conftest.py: Use session scope for firmware and backend fixtures
 - template: conftest.py: Targeted device must be specified with new `--device` required option.
@@ -265,7 +264,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
-
 ## [1.1.0] - 2023-01-02
 
 ### Added

src/ragger/navigator/__init__.py

@@ -13,9 +13,9 @@
    See the License for the specific language governing permissions and
    limitations under the License.
 """
-from .instruction import NavInsID, NavIns
+from .instruction import BaseNavInsID, NavInsID, NavIns
 from .navigator import Navigator
 from .stax_navigator import StaxNavigator
 from .nano_navigator import NanoNavigator
 
-__all__ = ["NavInsID", "NavIns", "Navigator", "StaxNavigator", "NanoNavigator"]
+__all__ = ["BaseNavInsID", "NavInsID", "NavIns", "Navigator", "StaxNavigator", "NanoNavigator"]

src/ragger/navigator/instruction.py

@@ -2,7 +2,15 @@
 from typing import Any, Dict
 
 
-class NavInsID(Enum):
+class BaseNavInsID(Enum):
+    """
+    Base NavInsID class, allowing to define NavInsID specific to one's application
+    while being compatible with all Navigator methods.
+    """
+    pass
+
+
+class NavInsID(BaseNavInsID):
     """
     Pre-defined instruction ID to navigate into a device UI.
     """
@@ -70,7 +78,7 @@ class NavInsID(Enum):
 
 class NavIns:
 
-    def __init__(self, id: NavInsID, args=(), kwargs: Dict[str, Any] = {}):
+    def __init__(self, id: BaseNavInsID, args=(), kwargs: Dict[str, Any] = {}):
         self.id = id
         self.args = args
         self.kwargs = kwargs

src/ragger/navigator/nano_navigator.py

@@ -18,15 +18,15 @@
 
 from ragger.backend import BackendInterface
 from ragger.firmware import Firmware
-from .navigator import NavInsID, Navigator
+from .navigator import BaseNavInsID, Navigator, NavInsID
 
 
 class NanoNavigator(Navigator):
 
     def __init__(self, backend: BackendInterface, firmware: Firmware, golden_run: bool = False):
         if firmware == Firmware.STAX:
             raise ValueError(f"'{self.__class__.__name__}' does not work on Stax")
-        callbacks: Dict[NavInsID, Callable] = {
+        callbacks: Dict[BaseNavInsID, Callable] = {
             NavInsID.WAIT: sleep,
             NavInsID.WAIT_FOR_SCREEN_CHANGE: backend.wait_for_screen_change,
             NavInsID.WAIT_FOR_HOME_SCREEN: backend.wait_for_home_screen,

src/ragger/navigator/navigator.py

@@ -23,9 +23,10 @@
 from ragger.firmware import Firmware
 from ragger.utils import Crop
 
-from .instruction import NavIns, NavInsID
+from .instruction import BaseNavInsID, NavIns, NavInsID
 
 LAST_SCREEN_UPDATE_TIMEOUT = 2
+InstructionType = Union[NavIns, BaseNavInsID]
 
 
 class Navigator(ABC):
@@ -37,7 +38,7 @@ class Navigator(ABC):
     def __init__(self,
                  backend: BackendInterface,
                  firmware: Firmware,
-                 callbacks: Dict[NavInsID, Callable],
+                 callbacks: Dict[BaseNavInsID, Callable],
                  golden_run: bool = False):
         """Initializes the Backend
 
@@ -112,12 +113,12 @@ def _compare_snap(self, snaps_tmp_path: Path, snaps_golden_path: Path, index: in
             golden, tmp_snap_path=tmp,
             golden_run=self._golden_run), f"Screen does not match golden {tmp}."
 
-    def add_callback(self, ins_id: NavInsID, callback: Callable, override: bool = True) -> None:
+    def add_callback(self, ins_id: BaseNavInsID, callback: Callable, override: bool = True) -> None:
         """
         Register a new callback.
 
         :param ins_id: The navigation instruction ID which will trigger the callback
-        :type ins_id: NavInsID
+        :type ins_id: BaseNavInsID
         :param callback: The callback to call
         :type callback: Callable
         :param override: Replace an existing callback if the navigation instruction ID already
@@ -136,13 +137,13 @@ def add_callback(self, ins_id: NavInsID, callback: Callable, override: bool = Tr
         self._callbacks[ins_id] = callback
 
     def _run_instruction(self,
-                         instruction: Union[NavIns, NavInsID],
+                         instruction: InstructionType,
                          timeout: float = 10.0,
                          wait_for_screen_change: bool = True,
                          path: Optional[Path] = None,
                          test_case_name: Optional[Path] = None,
                          snap_idx: int = 0) -> None:
-        if isinstance(instruction, NavInsID):
+        if isinstance(instruction, BaseNavInsID):
             instruction = NavIns(instruction)
         if instruction.id not in self._callbacks:
             raise NotImplementedError(f"No callback registered for instruction ID {instruction.id}")
@@ -208,7 +209,7 @@ def _run_instruction(self,
     def navigate_and_compare(self,
                              path: Optional[Path],
                              test_case_name: Optional[Path],
-                             instructions: List[Union[Union[NavIns, NavInsID]]],
+                             instructions: List[Union[InstructionType]],
                              timeout: float = 10.0,
                              screen_change_before_first_instruction: bool = True,
                              screen_change_after_last_instruction: bool = True,
@@ -223,7 +224,7 @@ def navigate_and_compare(self,
         :type test_case_name: Optional[Path]
         :param instructions: Set of navigation instructions. Navigation instruction IDs are also
                              accepted.
-        :type instructions: List[Union[NavIns, NavInsID]]
+        :type instructions: List[Union[NavIns, BaseNavInsID]]
         :param timeout: Timeout for each navigation step.
         :type timeout: int
         :param screen_change_before_first_instruction: Wait for a screen change before first
@@ -283,7 +284,7 @@ def navigate_and_compare(self,
         self._backend.resume_ticker()
 
     def navigate(self,
-                 instructions: List[Union[NavIns, NavInsID]],
+                 instructions: List[InstructionType],
                  timeout: float = 10.0,
                  screen_change_before_first_instruction: bool = True,
                  screen_change_after_last_instruction: bool = True) -> None:
@@ -293,7 +294,7 @@ def navigate(self,
         :param instructions: Set of navigation instructions. Navigation instruction IDs are also
                              accepted, and will be converted into navigation instruction (without
                              any argument)
-        :type instructions: List[Union[NavIns, NavInsID]]
+        :type instructions: List[Union[NavIns, BaseNavInsID]]
         :param timeout: Timeout for each navigation step.
         :type timeout: int
         :param screen_change_before_first_instruction: Wait for a screen change before first
@@ -319,8 +320,8 @@ def navigate(self,
             screen_change_after_last_instruction=screen_change_after_last_instruction)
 
     def navigate_until_snap(self,
-                            navigate_instruction: Union[NavIns, NavInsID],
-                            validation_instruction: Union[NavIns, NavInsID],
+                            navigate_instruction: InstructionType,
+                            validation_instruction: InstructionType,
                             path: Path,
                             test_case_name: Path,
                             start_img_name: str,
@@ -341,10 +342,10 @@ def navigate_until_snap(self,
 
         :param navigate_instruction: Navigation instruction to be performed until last snapshot is found.
                                      Navigation instruction ID is also accepted.
-        :type navigate_instruction: Union[NavIns, NavInsID]
+        :type navigate_instruction: Union[NavIns, BaseNavInsID]
         :param validation_instruction: Navigation instruction to be performed once last snapshot is found.
                                        Navigation instruction ID is also accepted.
-        :type validation_instruction: Union[NavIns, NavInsID]
+        :type validation_instruction: Union[NavIns, BaseNavInsID]
         :param path: Absolute path to the snapshots directory.
         :type path: Path
         :param test_case_name: Relative path to the test case snapshots directory (from path).
@@ -431,8 +432,8 @@ def navigate_until_snap(self,
         return img_idx
 
     def navigate_until_text_and_compare(self,
-                                        navigate_instruction: Union[NavIns, NavInsID],
-                                        validation_instructions: List[Union[NavIns, NavInsID]],
+                                        navigate_instruction: InstructionType,
+                                        validation_instructions: List[InstructionType],
                                         text: str,
                                         path: Optional[Path] = None,
                                         test_case_name: Optional[Path] = None,
@@ -456,10 +457,10 @@ def navigate_until_text_and_compare(self,
         :type test_case_name: Path
         :param navigate_instruction: Navigation instruction to be performed until the text is found.
                                      Navigation instruction ID is also accepted.
-        :type navigate_instruction: Union[NavIns, NavInsID]
+        :type navigate_instruction: Union[NavIns, BaseNavInsID]
         :param validation_instructions: Navigation instructions to be performed once the text is found.
                                         Navigation instruction IDs are also accepted.
-        :type validation_instructions: List[Union[NavIns, NavInsID]]
+        :type validation_instructions: List[Union[NavIns, BaseNavInsID]]
         :param text: Text string to look for.
         :type text: str
         :param timeout: Timeout for the whole navigation loop.
@@ -532,8 +533,8 @@ def navigate_until_text_and_compare(self,
         self._backend.resume_ticker()
 
     def navigate_until_text(self,
-                            navigate_instruction: Union[NavIns, NavInsID],
-                            validation_instructions: List[Union[NavIns, NavInsID]],
+                            navigate_instruction: InstructionType,
+                            validation_instructions: List[InstructionType],
                             text: str,
                             timeout: int = 300,
                             screen_change_before_first_instruction: bool = True,
@@ -550,10 +551,10 @@ def navigate_until_text(self,
 
         :param navigate_instruction: Navigation instruction to be performed until the text is found.
                                      Navigation instruction ID is also accepted.
-        :type navigate_instruction: Union[NavIns, NavInsID]
+        :type navigate_instruction: Union[NavIns, BaseNavInsID]
         :param validation_instructions: Navigation instructions to be performed once the text is found.
                                         Navigation instruction IDs are also accepted.
-        :type validation_instructions: List[Union[NavIns, NavInsID]]
+        :type validation_instructions: List[Union[NavIns, BaseNavInsID]]
         :param text: Text string to look for.
         :type text: str
         :param timeout: Timeout for the whole navigation loop.

src/ragger/navigator/stax_navigator.py

@@ -20,7 +20,7 @@
 from ragger.firmware import Firmware
 from ragger.firmware.stax import FullScreen
 from .navigator import Navigator
-from .instruction import NavInsID
+from .instruction import BaseNavInsID, NavInsID
 
 
 class StaxNavigator(Navigator):
@@ -29,7 +29,7 @@ def __init__(self, backend: BackendInterface, firmware: Firmware, golden_run: bo
         if firmware != Firmware.STAX:
             raise ValueError(f"'{self.__class__.__name__}' only works on Stax")
         screen = FullScreen(backend, firmware)
-        callbacks: Dict[NavInsID, Callable] = {
+        callbacks: Dict[BaseNavInsID, Callable] = {
             NavInsID.WAIT: sleep,
             NavInsID.WAIT_FOR_SCREEN_CHANGE: backend.wait_for_screen_change,
             NavInsID.WAIT_FOR_HOME_SCREEN: backend.wait_for_home_screen,

tests/unit/navigator/test_navigator.py

@@ -5,7 +5,7 @@
 
 from ragger.backend import SpeculosBackend, LedgerCommBackend
 from ragger.firmware import Firmware
-from ragger.navigator import Navigator, NavIns, NavInsID
+from ragger.navigator import BaseNavInsID, Navigator, NavIns, NavInsID
 
 
 class TestNavigator(TestCase):
@@ -118,6 +118,38 @@ def test_compare_snap_nok_raises(self):
         with self.assertRaises(AssertionError):
             self.navigator._compare_snap(self.pathdir, self.pathdir, 1)
 
+    def test__run_instructions_nok_no_callback(self):
+        instruction = NavIns(NavInsID.WAIT)
+        with self.assertRaises(NotImplementedError):
+            self.navigator._run_instruction(instruction)
+
+    def test__run_instructions_NavIns(self):
+        cb_wait = MagicMock()
+        self.navigator._callbacks = {NavInsID.WAIT: cb_wait}
+        args, kwargs = ("some", "args"), {"1": 2}
+        instruction = NavIns(NavInsID.WAIT, args, kwargs)
+        self.assertIsNone(self.navigator._run_instruction(instruction))
+        self.assertEqual(cb_wait.call_count, 1)
+        self.assertEqual(cb_wait.call_args, (args, kwargs))
+
+    def test__run_instructions_NavInsID(self):
+        cb_wait = MagicMock()
+        self.navigator._callbacks = {NavInsID.WAIT: cb_wait}
+        self.assertIsNone(self.navigator._run_instruction(NavInsID.WAIT))
+        self.assertEqual(cb_wait.call_count, 1)
+        self.assertEqual(cb_wait.call_args, ((), ))
+
+    def test__run_instructions_custom_instruction(self):
+
+        class TestInsID(BaseNavInsID):
+            WAIT = 1
+
+        cb_wait = MagicMock()
+        self.navigator._callbacks = {TestInsID.WAIT: cb_wait}
+        self.assertIsNone(self.navigator._run_instruction(TestInsID.WAIT))
+        self.assertEqual(cb_wait.call_count, 1)
+        self.assertEqual(cb_wait.call_args, ((), ))
+
     def test_navigate_nok_raises(self):
         with self.assertRaises(NotImplementedError):
             self.navigator.navigate([NavIns(2)])