Merge pull request #1681 from Bastian-Krause/bst/strategy-errors

Introduce Broken Strategy State, Prevent Further Use and Skip Subsequent Tests

Author: jluebbe

doc/development.rst

@@ -226,7 +226,7 @@ Start by creating a strategy skeleton:
     import attr
 
     from labgrid.step import step
-    from labgrid.strategy import Strategy, StrategyError
+    from labgrid.strategy import Strategy, StrategyError, never_retry
     from labgrid.factory import target_factory
 
     class Status(enum.Enum):
@@ -239,6 +239,7 @@ Start by creating a strategy skeleton:
 
         status = attr.ib(default=Status.unknown)
 
+        @never_retry
         @step()
         def transition(self, status, *, step):
             if not isinstance(status, Status):
@@ -262,9 +263,21 @@ It is possible to reference drivers via their protocol, e.g.
 Note that drivers which implement multiple protocols must not be referenced
 multiple times via different protocols.
 The ``Status`` class needs to be extended to cover the states of your strategy,
-then for each state an ``elif`` entry in the transition function needs to be
+then for each state an ``elif`` entry in the ``transition()`` method needs to be
 added.
 
+.. note::
+  Since infrastructure failures or broken strategies typically cannot recover,
+  it makes little sense to continue operating with such a strategy after an
+  error has occurred.
+  To clearly mark a strategy as unusable after failure (and to avoid cascading
+  errors in subsequent calls) the strategy's ``transition()`` method (and
+  optionally its ``force()`` method) can be decorated with the
+  ``@never_retry`` decorator.
+  This decorator causes the strategy to store the encountered exception in its
+  ``broken`` attribute and raise a ``StrategyError`` for the original and all
+  subsequent calls to the decorated methods.
+
 Lets take a look at the builtin `BareboxStrategy`.
 The Status enum for the BareboxStrategy:
 

doc/usage.rst

@@ -421,6 +421,10 @@ target (session scope)
 
 strategy (session scope)
   Used to access the :any:`Strategy` configured in the 'main' :any:`Target`.
+  If the Strategy enters broken state, all subsequent tests requesting it via
+  this fixture will be skipped.
+  See also :any:`never_retry` for an easy way to mark strategies broken on
+  error.
 
 Command-Line Options
 ~~~~~~~~~~~~~~~~~~~~

examples/qemu-networking/qemunetworkstrategy.py

@@ -17,7 +17,7 @@
 import attr
 
 from labgrid import target_factory, step
-from labgrid.strategy import Strategy, StrategyError
+from labgrid.strategy import Strategy, StrategyError, never_retry
 from labgrid.util import get_free_port
 
 
@@ -77,6 +77,7 @@ def update_network_service(self):
                 networkservice.address = new_address
                 networkservice.port = self.__remote_port
 
+    @never_retry
     @step(args=["state"])
     def transition(self, state, *, step):
         if not isinstance(state, Status):

examples/strategy/bareboxrebootstrategy.py

@@ -5,7 +5,7 @@
 from labgrid import target_factory, step
 from labgrid.driver import BareboxDriver, ShellDriver
 from labgrid.protocol import PowerProtocol
-from labgrid.strategy import Strategy
+from labgrid.strategy import Strategy, never_retry
 
 
 @attr.s(eq=False)
@@ -58,6 +58,7 @@ class BareboxRebootStrategy(Strategy):
     def __attrs_post_init__(self):
         super().__attrs_post_init__()
 
+    @never_retry
     @step(args=["new_status"])
     def transition(self, new_status, *, step):
         if not isinstance(new_status, Status):

examples/strategy/quartusstrategy.py

@@ -5,7 +5,7 @@
 from labgrid import target_factory, step
 from labgrid.driver import QuartusHPSDriver, SerialDriver
 from labgrid.protocol import PowerProtocol
-from labgrid.strategy import Strategy
+from labgrid.strategy import Strategy, never_retry
 
 
 @attr.s(eq=False)
@@ -37,6 +37,7 @@ class QuartusHPSStrategy(Strategy):
     def __attrs_post_init__(self):
         super().__attrs_post_init__()
 
+    @never_retry
     @step(args=["status"])
     def transition(self, status, *, step):
         if not isinstance(status, Status):

examples/usbpower/examplestrategy.py

@@ -5,7 +5,7 @@
 from labgrid.driver import BareboxDriver, ShellDriver, USBSDMuxDriver
 from labgrid import step, target_factory
 from labgrid.protocol import PowerProtocol
-from labgrid.strategy import Strategy
+from labgrid.strategy import Strategy, never_retry
 
 
 @attr.s(eq=False)
@@ -36,6 +36,7 @@ class ExampleStrategy(Strategy):
     def __attrs_post_init__(self):
         super().__attrs_post_init__()
 
+    @never_retry
     @step(args=["status"])
     def transition(self, status, *, step):
         if not isinstance(status, Status):

labgrid/pytestplugin/__init__.py

@@ -1,2 +1,2 @@
 from .fixtures import pytest_addoption, env, target, strategy
-from .hooks import pytest_configure, pytest_collection_modifyitems, pytest_cmdline_main
+from .hooks import pytest_configure, pytest_collection_modifyitems, pytest_cmdline_main, pytest_runtest_setup

labgrid/pytestplugin/hooks.py

@@ -6,6 +6,7 @@
 from ..consoleloggingreporter import ConsoleLoggingReporter
 from ..util.helper import processwrapper
 from ..logging import StepFormatter, StepLogger
+from ..exceptions import NoStrategyFoundError
 
 LABGRID_ENV_KEY = pytest.StashKey[Environment]()
 
@@ -121,3 +122,23 @@ def pytest_collection_modifyitems(config, items):
                     reason=f'Skipping because features "{missing_feature}" are not supported'
                 )
             item.add_marker(skip)
+
+@pytest.hookimpl(tryfirst=True)
+def pytest_runtest_setup(item):
+    """
+    Skip test if one of the targets uses a strategy considered broken.
+    """
+    # Before any fixtures run for the test, check if the session-scoped strategy fixture was
+    # requested (might have been executed already for a prior test). If that's the case and the
+    # strategy is broken, skip the test.
+    if "strategy" in item.fixturenames:
+        env = item.config.stash[LABGRID_ENV_KEY]
+        # skip test even if only one of the targets in the env has a broken strategy
+        for target_name in env.config.get_targets():
+            target = env.get_target(target_name)
+            try:
+                strategy = target.get_strategy()
+                if strategy.broken:
+                    pytest.skip(f"{strategy.__class__.__name__} is in broken state")
+            except NoStrategyFoundError:
+                pass

labgrid/strategy/__init__.py

@@ -1,4 +1,4 @@
-from .common import Strategy, StrategyError
+from .common import Strategy, StrategyError, never_retry
 from .bareboxstrategy import BareboxStrategy
 from .shellstrategy import ShellStrategy
 from .ubootstrategy import UBootStrategy

labgrid/strategy/bareboxstrategy.py

@@ -4,7 +4,7 @@
 
 from ..factory import target_factory
 from ..step import step
-from .common import Strategy, StrategyError
+from .common import Strategy, StrategyError, never_retry
 
 
 class Status(enum.Enum):
@@ -30,6 +30,7 @@ class BareboxStrategy(Strategy):
     def __attrs_post_init__(self):
         super().__attrs_post_init__()
 
+    @never_retry
     @step(args=['status'])
     def transition(self, status, *, step):  # pylint: disable=redefined-outer-name
         if not isinstance(status, Status):
@@ -63,6 +64,7 @@ def transition(self, status, *, step):  # pylint: disable=redefined-outer-name
             )
         self.status = status
 
+    @never_retry
     @step(args=['status'])
     def force(self, status):
         if not isinstance(status, Status):