topic/graph strategy

[fscherf]

• Add graph based strategies
• Setup coverage in local test suite
• Some fixes

[codecov-io]

Codecov Report

Merging #231 into master will increase coverage by 0.8%.
The diff coverage is 100%.

@@           Coverage Diff            @@
##           master    #231     +/-   ##
========================================
+ Coverage    54.6%   55.5%   +0.8%     
========================================
  Files          99     100      +1     
  Lines        5678    5786    +108     
========================================
+ Hits         3105    3213    +108     
  Misses       2573    2573

Impacted Files	Coverage Δ
labgrid/strategy/common.py	88.2% <100%> (+2.5%)	⬆️
labgrid/strategy/__init__.py	100% <100%> (ø)	⬆️
labgrid/strategy/graphstrategy.py	100% <100%> (ø)
labgrid/strategy/bareboxstrategy.py	51.3% <100%> (-3.7%)	⬇️
labgrid/strategy/shellstrategy.py	50% <100%> (-3.9%)	⬇️
labgrid/strategy/ubootstrategy.py	51.4% <100%> (-3.9%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1796ae7...ae39eeb. Read the comment docs.

[jluebbe]

Document that exactly one state without dependencies is required.

[fscherf]

Do have the lines 295 and 295 not have enough information?

[jluebbe]

Looks fine.

[jluebbe]

Fix typo in filename (missing 'h').

[jluebbe]

This should be a separate PR.

[jluebbe]

This should be rejected as invalid.

[jluebbe]

Report which state has a missing dependency.

[jluebbe]

Do we want to call invalidate if an exception occurs?

[fscherf]

i think the transition should stop in this case and remain at the previous state

[jluebbe]

If the exception occurs after some code in the state_ function was called (such as power changes), the previous state is no longer correct. So I'd prefer to invalidate().

[jluebbe]

The if is redundant (for on an empty list does nothing).

[jluebbe]

Add a docstring to explain how this works.

[jluebbe]

How much time does this save?

[jluebbe]

Add test which throws an exception.

[fscherf]

@jluebbe

diff --git a/.coveragerc b/.coveragerc
index 2b005afac986..b1a70dbd0eaf 100644
--- a/.coveragerc
+++ b/.coveragerc
@@ -1,6 +1,3 @@
-[run]
-branch = True
-
 [report]
 exclude_lines =
     pragma: no cover
diff --git a/doc/configuration.rst b/doc/configuration.rst
index 910c8c508a15..2bce1bfc74e1 100644
--- a/doc/configuration.rst
+++ b/doc/configuration.rst
@@ -780,6 +780,7 @@ Arguments:
   - prompt (regex): barebox prompt to match
   - autoboot (regex, default="stop autoboot"): autoboot message to match
   - interrupt (str, default="\\n"): string to interrupt autoboot (use "\\x03" for CTRL-C)
+  - startstring (regex, default="[\n]barebox 20\d+"): string that indicates that Barebox is starting
   - bootstring (regex, default="Linux version \d"): succesfully jumped into the kernel 
 
 ExternalConsoleDriver
diff --git a/doc/development.rst b/doc/development.rst
index d62634d11c1a..5e4521d0d781 100644
--- a/doc/development.rst
+++ b/doc/development.rst
@@ -332,9 +332,9 @@ A root state is a state that has no dependencies.
     >>> graph_strategy.graph.render('filename')
     'filename.png'
 
-.. image:: res/grapstrategy-1.png
+.. image:: res/graphstrategy-1.png
 
-.. image:: res/grapstrategy-2.png
+.. image:: res/graphstrategy-2.png
 
 
 .. _contributing:
diff --git a/doc/res/grapstrategy-1.png b/doc/res/graphstrategy-1.png
similarity index 100%
rename from doc/res/grapstrategy-1.png
rename to doc/res/graphstrategy-1.png
diff --git a/doc/res/grapstrategy-2.png b/doc/res/graphstrategy-2.png
similarity index 100%
rename from doc/res/grapstrategy-2.png
rename to doc/res/graphstrategy-2.png
diff --git a/labgrid/driver/bareboxdriver.py b/labgrid/driver/bareboxdriver.py
index 2c5a1ac3db4e..382ad677fe3e 100644
--- a/labgrid/driver/bareboxdriver.py
+++ b/labgrid/driver/bareboxdriver.py
@@ -24,11 +24,14 @@ class BareboxDriver(CommandMixin, Driver, CommandProtocol, LinuxBootProtocol):
 
     Args:
         prompt (str): The default Barebox Prompt
+        startstring (str): string that indicates that Barebox is starting
+        bootstring (str): string that indicates that the Kernel is booting
     """
     bindings = {"console": ConsoleProtocol, }
     prompt = attr.ib(default="", validator=attr.validators.instance_of(str))
     autoboot = attr.ib(default="stop autoboot", validator=attr.validators.instance_of(str))
     interrupt = attr.ib(default="\n", validator=attr.validators.instance_of(str))
+    startstring = attr.ib(default=r"[\n]barebox 20\d+", validator=attr.validators.instance_of(str))
     bootstring = attr.ib(default="Linux version \d", validator=attr.validators.instance_of(str))
 
     def __attrs_post_init__(self):
@@ -121,7 +124,7 @@ class BareboxDriver(CommandMixin, Driver, CommandProtocol, LinuxBootProtocol):
 
     def _await_prompt(self):
         """Await autoboot line and stop it to get to the prompt"""
-        self.console.expect(r"[\n]barebox 20\d+")
+        self.console.expect(self.startstring)
         index, _, _, _ = self.console.expect([self.prompt, self.autoboot])
         if index == 0:
             self._status = 1
diff --git a/labgrid/strategy/graphstrategy.py b/labgrid/strategy/graphstrategy.py
index a384e77fa8f9..99aa76898e19 100644
--- a/labgrid/strategy/graphstrategy.py
+++ b/labgrid/strategy/graphstrategy.py
@@ -40,7 +40,11 @@ class GraphStrategy(Strategy):
             method = getattr(self, state_name)
 
             if not callable(method):
-                continue
+                raise InvalidGraphStrategyError(
+                    "GraphStrategy state '{}' is not callable".format(
+                        state_name,
+                    )
+                )
 
             state_name = '_'.join(state_name.split('_')[1:])
 
@@ -60,18 +64,14 @@ class GraphStrategy(Strategy):
             for dependency in self.states[state_name]['dependencies']:
                 if dependency not in state_names:
                     raise InvalidGraphStrategyError(
-                        "State '{}' is unknown. State names are: {}".format(
-                            dependency,
-                            ', '.join(state_names),
+                        "{}: State '{}' is unknown. State names are: {}".format(
+                            state_name, dependency, ', '.join(state_names),
                         )
                     )
 
         # find root state
-        root_states = list(
-            filter(lambda state: state[0]
-                   if not state[1]['dependencies'] else None,
-                   self.states.items())
-        )
+        root_states = [k for k, v in self.states.items()
+                       if not v['dependencies']]
 
         if not root_states:
             raise InvalidGraphStrategyError(
@@ -85,18 +85,18 @@ class GraphStrategy(Strategy):
                 )
             )
 
-        self.root_state = root_states[0][0]
+        self.root_state = root_states[0]
         self.invalidate()
 
         # setup grahviz cache
         self._graph_cache = {
-            'tempdir': TemporaryDirectory(),
+            'tempdir': None,
             'graph': None,
             'path': self.path,
         }
 
     def invalidate(self):
-        self.path = [self.root_state]
+        self.path = []
 
     @step(args=['state'])
     def transition(self, state, via=[]):
@@ -128,7 +128,13 @@ class GraphStrategy(Strategy):
 
             # run state methods
             for state_name in path:
-                self.states[state_name]['method']()
+                try:
+                    self.states[state_name]['method']()
+
+                except Exception:
+                    self.invalidate()
+
+                    raise
 
             self.path = abs_path
 
@@ -146,11 +152,10 @@ class GraphStrategy(Strategy):
         while current_state['dependencies']:
             next_state = current_state['dependencies'][0]
 
-            if via:
-                for i in via:
-                    if i in current_state['dependencies']:
-                        via.remove(i)
-                        next_state = i
+            for i in via:
+                if i in current_state['dependencies']:
+                    via.remove(i)
+                    next_state = i
 
             path.insert(0, next_state)
             current_state = self.states[next_state]
@@ -171,6 +176,9 @@ class GraphStrategy(Strategy):
            self._graph_cache['path'] == self.path):
             return self._graph_cache['graph']
 
+        if not self._graph_cache['tempdir']:
+            self._graph_cache['tempdir'] = TemporaryDirectory()
+
         dg = Digraph(
             filename=os.path.join(self._graph_cache['tempdir'].name, 'graph'),
             format='png',
diff --git a/tests/test_graphstrategy.py b/tests/test_graphstrategy.py
index 183fece18f0d..3ed38c91e85b 100644
--- a/tests/test_graphstrategy.py
+++ b/tests/test_graphstrategy.py
@@ -43,7 +43,7 @@ def test_fixture(graph_strategy, target):
     assert graph_strategy.root_state == 'Root'
 
     # test path
-    assert graph_strategy.path == ['Root']
+    assert graph_strategy.path == []
 
     # test state discovering
     state_names = sorted(list(graph_strategy.states.keys()))
@@ -120,12 +120,12 @@ def test_unknown_dependencies(target):
     with pytest.raises(InvalidGraphStrategyError) as e:
         TestStrategy(target, 'strategy')
 
-    assert e.value.msg.startswith("State 'C' is unknown.")
+    assert e.value.msg.startswith("B: State 'C' is unknown")
 
 
 @pytest.mark.dependency(depends=['test_fixture'])
 def test_strategy_with_uncallable_states(target):
-    from labgrid.strategy import GraphStrategy
+    from labgrid.strategy import GraphStrategy, InvalidGraphStrategyError
 
     class TestStrategy(GraphStrategy):
         state_foo = 1
@@ -133,9 +133,11 @@ def test_strategy_with_uncallable_states(target):
         def state_Root(self):
             pass
 
-    strategy = TestStrategy(target, 'strategy')
+    with pytest.raises(InvalidGraphStrategyError) as e:
+        TestStrategy(target, 'strategy')
 
-    assert list(strategy.states.keys()) == ['Root']
+    assert e.value.msg.startswith(
+        "GraphStrategy state 'state_foo' is not callable")
 
 
 @pytest.mark.dependency(name='api-works',
@@ -162,7 +164,7 @@ def test_graphviz_graph(graph_strategy):
 
 @pytest.mark.dependency(depends=['api-works'])
 def test_transition(graph_strategy):
-    assert graph_strategy.transition('B') == ['A1', 'B']
+    assert graph_strategy.transition('B') == ['Root', 'A1', 'B']
     assert graph_strategy.path == ['Root', 'A1', 'B']
 
     assert graph_strategy.transition('B') == []
@@ -205,7 +207,7 @@ def test_interleaved_transitions(target):
 
 @pytest.mark.dependency(depends=['api-works', 'test_transition'])
 def test_transition_via(graph_strategy):
-    assert graph_strategy.transition('D', via=['A2']) == ['A2', 'B', 'C1', 'D']
+    assert graph_strategy.transition('D', via=['A2']) == ['Root', 'A2', 'B', 'C1', 'D']
     assert graph_strategy.path == ['Root', 'A2', 'B', 'C1', 'D']
 
 
@@ -213,10 +215,37 @@ def test_transition_via(graph_strategy):
                                  'test_transition',
                                  'test_transition_via'])
 def test_incremental_transition(graph_strategy):
-    assert graph_strategy.transition('B') == ['A1', 'B']
+    assert graph_strategy.transition('B') == ['Root', 'A1', 'B']
     assert graph_strategy.transition('D') == ['C1', 'D']
 
     graph_strategy.invalidate()
 
-    assert graph_strategy.transition('B', via=['A2']) == ['A2', 'B']
+    assert graph_strategy.transition('B', via=['A2']) == ['Root', 'A2', 'B']
     assert graph_strategy.transition('D') == ['Root', 'A1', 'B', 'C1', 'D']
+
+
+@pytest.mark.dependency(depends=['api-works', 'test_transition'])
+def test_transition_error(target):
+    from labgrid.strategy import GraphStrategy
+
+    class TestStrategy(GraphStrategy):
+        def state_Root(self):
+            pass
+
+        @GraphStrategy.depends('Root')
+        def state_A(self):
+            pass
+
+        @GraphStrategy.depends('A')
+        def state_B(self):
+            raise Exception
+
+    strategy = TestStrategy(target, 'strategy')
+
+    strategy.transition('A')
+    assert strategy.path == ['Root', 'A']
+
+    with pytest.raises(Exception):
+        strategy.transition('B')
+
+    assert strategy.path == []