Merge pull request #573 from kdmukai/flowtest_refactor

[Enhancement] improve FlowTest + Exception handling interactions

tests/base.py

@@ -16,7 +16,7 @@
 from seedsigner.gui.screens.screen import RET_CODE__BACK_BUTTON, RET_CODE__POWER_BUTTON
 from seedsigner.hardware.microsd import MicroSD
 from seedsigner.models.settings import Settings
-from seedsigner.views.view import Destination, MainMenuView, View
+from seedsigner.views.view import Destination, MainMenuView, UnhandledExceptionView, View
 
 import logging
 logger = logging.getLogger(__name__)
@@ -152,11 +152,15 @@ class FlowTestUnexpectedViewException(FlowBasedTestException):
 
 
 
-class FlowTestRunScreenNotExecutedException(FlowBasedTestException):
-    """ The View's run_screen() method was not called but the FlowStep expected it to need user input """
+class FlowTestUnexpectedRedirectException(FlowBasedTestException):
+    """ The Controller's current View triggered a redirect that was not expected by the current FlowStep in the sequence """
     pass
 
 
+class FlowTestMissingRedirectException(FlowBasedTestException):
+    """ The Controller's current View did NOT trigger a redirect when one was expected by the current FlowStep in the sequence """
+    pass
+
 
 class FlowTest(BaseTest):
     """ Base class for any tests that do flow-based testing """
@@ -176,26 +180,28 @@ def run_sequence(self, sequence: list[FlowStep], initial_destination_view_args:
                 def run_view(destination: Destination, *args, **kwargs):
                     """ Replaces Destination._run_view() """
                     if len(sequence) == 0:
+                        # Nothing left to do.
                         self.stop_test()
-                    
+
                     cur_flow_step = sequence[0]
 
                     # Verify that the View class specified in the test sequence matches the
                     # View class that is being run.
                     if destination.View_cls != cur_flow_step.expected_view:
                         raise FlowTestUnexpectedViewException(f"Expected {cur_flow_step.expected_view}, got {destination.View_cls}")
+
+                    if len(sequence) == 1:
+                        # This is the last step in the sequence
+                        if cur_flow_step.screen_return_value is None and cur_flow_step.button_data_selection is None:
+                            # This is the last View in the sequence and it doesn't specify any
+                            # user-mimicking interactions for the Screen. Nothing left to do.
+                            self.stop_test()
+
+                    try:
+                        if cur_flow_step.is_redirect and destination.view.has_redirect:
+                            # Right upon instantiation, the View set its own redirect without
+                            # needing to wait for its run() method to be called.
 
-                    # Run the optional pre-run function to modify the View.
-                    if cur_flow_step.before_run:
-                        cur_flow_step.before_run(destination.view)
-
-                    if cur_flow_step.is_redirect:
-                        # The current View is going to auto-redirect without calling run_screen(),
-                        # so we need to remove the current step from the sequence before the
-                        # View.run() call below.
-                        sequence.pop(0)
-
-                        if destination.view.has_redirect:
                             # TODO: Migrate all View redirects to use `View.set_redirect()`
                             # in their `__init__()` rather than `run()` and then refactor
                             # here to explicitly require `has_redirect` to be True.
@@ -204,44 +210,66 @@ def run_view(destination: Destination, *args, **kwargs):
                             # below.
                             return destination.view.get_redirect()
 
-                    # Some Views reach into their Screen's variables directly (e.g. 
-                    # Screen.buttons to preserve the scroll position), so we need to mock out the
-                    # Screen instance that is created by the View.
-                    destination.view.screen = MagicMock()
+                        # Run the optional pre-run function to modify the View.
+                        if cur_flow_step.before_run:
+                            cur_flow_step.before_run(destination.view)
+
+                        # Some Views reach into their Screen's variables directly (e.g. 
+                        # Screen.buttons to preserve the scroll position), so we need to mock out the
+                        # Screen instance that is created by the View.
+                        destination.view.screen = MagicMock()
 
-                    prev_mock_run_screen_call_count = mock_run_screen.call_count
+                        prev_mock_run_screen_call_count = mock_run_screen.call_count
 
-                    # Run the View (with our mocked run_screen) and get the next Destination that results
-                    destination = destination.view.run()
+                        # Run the View (with our mocked run_screen) and get the next Destination that results
+                        destination = destination.view.run()
 
-                    if (cur_flow_step.button_data_selection or cur_flow_step.screen_return_value is not None) and mock_run_screen.call_count == prev_mock_run_screen_call_count:
-                        # The FlowStep was expecting some kind of user interaction, but the View
-                        # never called run_screen().
-                        raise FlowTestRunScreenNotExecutedException(f"View.run_screen() was not run for {destination.View_cls.__name__}")
+                        if mock_run_screen.call_count == prev_mock_run_screen_call_count and cur_flow_step.is_redirect is not True:
+                            # The current View redirected without calling run_screen()
+                            # but we weren't expecting it.
+                            raise FlowTestUnexpectedRedirectException(f"Unexpected redirect to {destination.View_cls}")
+
+                        elif mock_run_screen.call_count > prev_mock_run_screen_call_count and cur_flow_step.is_redirect:
+                            # The View ran its Screen, but the current FlowStep was expecting it
+                            # to redirect (is_redirect=True) *instead of* running its Screen.
+                            raise FlowTestMissingRedirectException(f"FlowStep expected redirect but {cur_flow_step.expected_view} did not redirect")
+
+                    finally:
+                        # Regardless of the outcome, we always move our FlowTest
+                        # sequence forward.
+                        sequence.pop(0)
 
                     return destination
 
+
                 def run_screen(view: View, *args, **kwargs):
-                    """ Replaces View.run_screen() """
-                    # Return the return value specified in the test sequence and
-                    # remove the completed test step from the sequence.
-                    flow_step = sequence.pop(0)
+                    """
+                    Replaces View.run_screen().
 
-                    if flow_step.button_data_selection:
+                    Just returns the return value specified in the test sequence.
+                    """
+                    cur_flow_step = sequence[0]
+
+                    if cur_flow_step.button_data_selection:
                         # We're mocking out the View.run_screen() method, so we'll get all of the
                         # input args that are normally passed into the Screen.run() method,
                         # including the button_data kwarg.
                         if "button_data" in kwargs:
-                            if flow_step.button_data_selection not in kwargs.get("button_data") and flow_step.button_data_selection not in [RET_CODE__BACK_BUTTON, RET_CODE__POWER_BUTTON]:
-                                raise FlowTestInvalidButtonDataSelectionException(f"'{flow_step.button_data_selection}' not found in button_data: {kwargs.get('button_data')}")
-                            return kwargs.get("button_data").index(flow_step.button_data_selection)
+                            if cur_flow_step.button_data_selection not in kwargs.get("button_data") and cur_flow_step.button_data_selection not in [RET_CODE__BACK_BUTTON, RET_CODE__POWER_BUTTON]:
+                                raise FlowTestInvalidButtonDataSelectionException(f"'{cur_flow_step.button_data_selection}' not found in button_data: {kwargs.get('button_data')}")
+                            return kwargs.get("button_data").index(cur_flow_step.button_data_selection)
                         else:
                             raise Exception(f"Can't specify `FlowStep.button_data_selection` if `button_data` isn't a kwarg in {view.__class__.__name__}'s run_screen()")
 
-                    elif type(flow_step.screen_return_value) in [StopFlowBasedTest, FlowBasedTestException]:
-                        raise flow_step.screen_return_value
+                    elif type(cur_flow_step.screen_return_value) in [StopFlowBasedTest, FlowBasedTestException]:
+                        raise cur_flow_step.screen_return_value
+
+                    elif isinstance(cur_flow_step.screen_return_value, Exception):
+                        # The FlowStep wants to mimic the Screen raising an exception.
+                        raise cur_flow_step.screen_return_value
+
+                    return cur_flow_step.screen_return_value
 
-                    return flow_step.screen_return_value
 
                 # Mock out the Destination._run_view() method so we can verify the View class
                 # that is specified in the test sequence and then run the View.

tests/test_flows.py

@@ -1,13 +1,14 @@
 import pytest
 
 # Must import test base before the Controller
-from base import FlowTest, FlowStep, FlowTestUnexpectedViewException, FlowTestInvalidButtonDataSelectionException, FlowTestRunScreenNotExecutedException
+from base import FlowTest, FlowStep, FlowTestMissingRedirectException, FlowTestUnexpectedRedirectException, FlowTestUnexpectedViewException, FlowTestInvalidButtonDataSelectionException
 
 from seedsigner.controller import Controller
 from seedsigner.gui.screens.screen import RET_CODE__BACK_BUTTON, RET_CODE__POWER_BUTTON
 from seedsigner.models.seed import Seed
-from seedsigner.models.settings_definition import SettingsConstants
-from seedsigner.views.seed_views import SeedBackupView, SeedMnemonicEntryView, SeedOptionsView, SeedWordsWarningView
+from seedsigner.views import scan_views
+from seedsigner.views.psbt_views import PSBTSelectSeedView
+from seedsigner.views.seed_views import SeedBackupView, SeedMnemonicEntryView, SeedOptionsView, SeedsMenuView
 from seedsigner.views.view import MainMenuView, PowerOptionsView, UnhandledExceptionView
 from seedsigner.views.tools_views import ToolsMenuView, ToolsCalcFinalWordNumWordsView
 
@@ -36,8 +37,31 @@ def test_FlowTestUnexpectedViewException(self):
         with pytest.raises(FlowTestUnexpectedViewException):
             self.run_sequence([
                 FlowStep(MainMenuView, button_data_selection=RET_CODE__POWER_BUTTON),
-                FlowStep(ToolsMenuView),  # <-- Wrong target View! Should raise an AssertionError.
+                FlowStep(ToolsMenuView),  # <-- Wrong target View!
             ])
+
+
+    def test_UnhandledExceptionView(self):
+        """
+        This is a regression test to ensure that the FlowTest is aware of exceptions that
+        redirect to the UnhandledExceptionView. If that isn't the expected View, the
+        FlowTest should raise a FlowTestUnexpectedViewException.
+        """
+        # This sequence simulates a FlowTest that is unaware of an exception that will
+        # derail the sequence (i.e. somebody wrote a bad FlowTest or something unexpected
+        # is breaking). The sequence should fail with FlowTestUnexpectedViewException.
+        with pytest.raises(FlowTestUnexpectedViewException):
+            self.run_sequence([
+                FlowStep(PSBTSelectSeedView),  # <-- There is no PSBT loaded. Should raise an exception that routes us to the UnhandledExceptionView.
+                FlowStep(scan_views.ScanSeedQRView),  # <-- This is not the View we'll end up at; FlowTest should raise the FlowTestUnexpectedViewException
+            ])
+
+        # This sequence *expects* an exception to route us to the UnhandledExceptionView
+        # and therefore can complete successfully.
+        self.run_sequence([
+            FlowStep(PSBTSelectSeedView),  # <-- There's no PSBT loaded.
+            FlowStep(UnhandledExceptionView),
+        ])
 
 
     def test_FlowTestInvalidButtonDataSelectionException(self):
@@ -47,29 +71,33 @@ def test_FlowTestInvalidButtonDataSelectionException(self):
         """
         with pytest.raises(FlowTestInvalidButtonDataSelectionException):
             self.run_sequence([
-            FlowStep(MainMenuView, button_data_selection="this is not a real button option!"),
+                FlowStep(MainMenuView, button_data_selection="this is not a real button option!"),
             ])
 
 
-    def test_FlowTestRunScreenNotExecutedException(self):
+    def test_FlowTestUnexpectedRedirectException(self):
         """
-        Ensure that the FlowTest will raise a FlowTestRunScreenNotExecutedException if the next
-        View in the sequence doesn't call its View.run_screen().
+        If the FlowStep doesn't specify is_redirect when the View redirects, raise FlowTestUnexpectedRedirectException
         """
-        # Disable dire warnings so that the SeedWordsWarningView won't execute its run_screen()
-        self.settings.set_value(SettingsConstants.SETTING__DIRE_WARNINGS, SettingsConstants.OPTION__DISABLED)
-        self.controller.storage.set_pending_seed(Seed(mnemonic=["bacon"] * 24))
-        self.controller.storage.finalize_pending_seed()
+        with pytest.raises(FlowTestUnexpectedRedirectException) as e:
+            self.run_sequence([
+                FlowStep(SeedsMenuView, button_data_selection=SeedsMenuView.LOAD),  # <-- No seeds loaded, so it'll redirect elsewhere
+            ])
+
+        # This time we'll show that we know it should redirect
+        self.run_sequence([
+            FlowStep(SeedsMenuView, is_redirect=True),
+        ])
 
-        with pytest.raises(FlowTestRunScreenNotExecutedException):
-            self.run_sequence(
-                initial_destination_view_args=dict(seed_num=0),
-                sequence=[
-                    FlowStep(SeedOptionsView, button_data_selection=SeedOptionsView.BACKUP),
-                    FlowStep(SeedBackupView, button_data_selection=SeedBackupView.VIEW_WORDS),
-                    FlowStep(SeedWordsWarningView, screen_return_value=0),
-                ],
-            )
+
+    def test_FlowTestMissingRedirectException(self):
+        """
+        If the FlowStep specifies is_redirect but the View does NOT redirect, raise FlowTestMissingRedirectException
+        """
+        with pytest.raises(FlowTestMissingRedirectException):
+            self.run_sequence([
+                FlowStep(MainMenuView, button_data_selection=MainMenuView.TOOLS, is_redirect=True),
+            ])
 
 
     def test_before_run_executes(self):

tests/test_flows_seed.py

@@ -8,8 +8,8 @@
 from seedsigner.gui.screens.screen import RET_CODE__BACK_BUTTON
 from seedsigner.models.settings import Settings, SettingsConstants
 from seedsigner.models.seed import ElectrumSeed, Seed
-from seedsigner.views.view import ErrorView, MainMenuView, OptionDisabledView, RemoveMicroSDWarningView, View, NetworkMismatchErrorView, NotYetImplementedView
-from seedsigner.views import seed_views, scan_views, settings_views, tools_views
+from seedsigner.views.view import ErrorView, MainMenuView, OptionDisabledView, View, NetworkMismatchErrorView
+from seedsigner.views import seed_views, scan_views, settings_views
 
 
 def load_seed_into_decoder(view: scan_views.ScanView):
@@ -561,8 +561,6 @@ def expect_network_mismatch_error(load_message: Callable):
         expect_network_mismatch_error(self.load_short_message_into_decoder)
 
 
-
-
     def test_sign_message_option_disabled(self):
         """
         Should redirect to OptionDisabledView if a `signmessage` QR is scanned with
@@ -583,15 +581,15 @@ def test_sign_message_option_disabled(self):
         # First test routing to update the setting
         self.run_sequence(
             sequence + [
-                FlowStep(OptionDisabledView, button_data_selection=OptionDisabledView.UPDATE_SETTING, is_redirect=True),
+                FlowStep(OptionDisabledView, button_data_selection=OptionDisabledView.UPDATE_SETTING),
                 FlowStep(settings_views.SettingsEntryUpdateSelectionView),
             ]
         )
 
         # Now test exiting to Main Menu
         self.run_sequence(
             sequence + [
-                FlowStep(OptionDisabledView, button_data_selection=OptionDisabledView.DONE, is_redirect=True),
+                FlowStep(OptionDisabledView, button_data_selection=OptionDisabledView.DONE),
                 FlowStep(MainMenuView),
             ]
         )

tests/test_flows_view.py

@@ -22,7 +22,7 @@ def test_restart_flow(self):
                 FlowStep(PowerOptionsView, button_data_selection=PowerOptionsView.RESET),
                 FlowStep(RestartView),
             ])
-        
+
 
     def test_power_off_flow(self):
         """
@@ -34,7 +34,7 @@ def test_power_off_flow(self):
                 FlowStep(PowerOptionsView, button_data_selection=PowerOptionsView.POWER_OFF),
                 FlowStep(PowerOffView),
             ])
-        
+
         # And again, but this time as if we were in the SeedSigner OS
         Settings.HOSTNAME = Settings.SEEDSIGNER_OS
         self.run_sequence([
@@ -43,7 +43,7 @@ def test_power_off_flow(self):
             FlowStep(PowerOffView),  # returns BackStackView
             FlowStep(PowerOptionsView),
         ])
-    
+
 
     def test_not_yet_implemented_flow(self):
         """
@@ -59,13 +59,12 @@ def run(self):
             FlowStep(NotYetImplementedView),
             FlowStep(MainMenuView),
         ])
-    
+
 
     def test_unhandled_exception_flow(self):
         """
         Basic flow from any arbitrary View to the UnhandledExceptionView
         """
-        Settings.HOSTNAME = "NOT seedsigner-os"
         self.run_sequence([
             FlowStep(MainMenuView, button_data_selection=MainMenuView.TOOLS),
             FlowStep(ToolsMenuView, button_data_selection=ToolsMenuView.KEYBOARD),