Merge a28ec75 into c31f895

openhtf/core/phase_descriptor.py

@@ -103,6 +103,8 @@ class PhaseOptions(object):
       needs to wrap another phase for some reason, as PhaseDescriptors can only
       be invoked with a TestState instance.
     force_repeat: If True, force the phase to repeat up to repeat_limit times.
+    repeat_on_measurement_fail: If true, force phase with failed
+      measurements to repeat up to repeat_limit times.
     repeat_on_timeout:  If consider repeat on phase timeout, default is No.
     repeat_limit:  Maximum number of repeats.  None indicates a phase will be
       repeated infinitely as long as PhaseResult.REPEAT is returned.
@@ -121,6 +123,7 @@ def PhaseFunc(test, port, other_info): pass
   run_if = attr.ib(type=Optional[Callable[[], bool]], default=None)
   requires_state = attr.ib(type=bool, default=False)
   force_repeat = attr.ib(type=bool, default=False)
+  repeat_on_measurement_fail = attr.ib(type=bool, default=False)
   repeat_on_timeout = attr.ib(type=bool, default=False)
   repeat_limit = attr.ib(type=Optional[int], default=None)
   run_under_pdb = attr.ib(type=bool, default=False)
@@ -147,6 +150,10 @@ def __call__(self, phase_func: PhaseT) -> 'PhaseDescriptor':
       phase.options.requires_state = self.requires_state
     if self.repeat_on_timeout:
       phase.options.repeat_on_timeout = self.repeat_on_timeout
+    if self.force_repeat:
+      phase.options.force_repeat = self.force_repeat
+    if self.repeat_on_measurement_fail:
+      phase.options.repeat_on_measurement_fail = self.repeat_on_measurement_fail
     if self.repeat_limit is not None:
       phase.options.repeat_limit = self.repeat_limit
     if self.run_under_pdb:

openhtf/core/phase_executor.py

@@ -242,6 +242,20 @@ def __init__(self, test_state: 'htf_test_state.TestState'):
     self._current_phase_thread = None  # type: Optional[PhaseExecutorThread]
     self._stopping = threading.Event()
 
+  def _should_repeat(self, phase: phase_descriptor.PhaseDescriptor,
+                     phase_execution_outcome: PhaseExecutionOutcome) -> bool:
+    """Returns whether a phase should be repeated."""
+    if phase_execution_outcome.is_timeout and phase.options.repeat_on_timeout:
+      return True
+    elif phase_execution_outcome.is_repeat:
+      return True
+    elif phase.options.force_repeat:
+      return True
+    elif phase.options.repeat_on_measurement_fail:
+      last_phase_outcome = self.test_state.test_record.phases[-1].outcome
+      return last_phase_outcome == test_record.PhaseOutcome.FAIL
+    return False
+
   def execute_phase(
       self,
       phase: phase_descriptor.PhaseDescriptor,
@@ -270,12 +284,8 @@ def execute_phase(
       is_last_repeat = repeat_count >= repeat_limit
       phase_execution_outcome, profile_stats = self._execute_phase_once(
           phase, is_last_repeat, run_with_profiling, subtest_rec)
-
-      # Give 3 default retries for timeout phase.
-      # Force repeat up to the repeat limit if force_repeat is set.
-      if ((phase_execution_outcome.is_timeout and
-           phase.options.repeat_on_timeout) or phase_execution_outcome.is_repeat
-          or phase.options.force_repeat) and not is_last_repeat:
+      if (self._should_repeat(phase, phase_execution_outcome) and
+          not is_last_repeat):
         repeat_count += 1
         continue
 
@@ -362,7 +372,8 @@ def evaluate_checkpoint(
       subtest_name = None
     evaluated_millis = util.time_millis()
     try:
-      outcome = PhaseExecutionOutcome(checkpoint.get_result(self.test_state, subtest_rec))
+      outcome = PhaseExecutionOutcome(checkpoint.get_result(self.test_state,
+                                                            subtest_rec))
       self.logger.debug('Checkpoint %s result: %s', checkpoint.name,
                         outcome.phase_result)
       if outcome.is_fail_subtest and not subtest_rec:

test/core/exe_test.py

@@ -71,6 +71,18 @@ class MoreRepeatsUnittestPlug(UnittestPlug):
   return_continue_count = 100
 
 
+class RepeatTracker():
+
+  def __init__(self):
+    self.count = 0
+
+  def increment(self):
+    self.count += 1
+
+  def get_num_repeats(self) -> int:
+    return self.count
+
+
 class FailedPlugError(Exception):
   """Exception for the failed plug."""
 
@@ -115,6 +127,29 @@ def phase_repeat(test, test_plug):
   return openhtf.PhaseResult.CONTINUE if ret else openhtf.PhaseResult.REPEAT
 
 
+@openhtf.PhaseOptions(repeat_on_measurement_fail=True, repeat_limit=5)
+@openhtf.measures(
+    openhtf.Measurement('example_dimension').with_dimensions(
+        'dim').dimension_pivot_validate(
+            util.validators.InRange(
+                minimum=-5,
+                maximum=5,
+            )))
+def phase_repeat_on_multidim_measurement_fail(test, meas_value: int,
+                                              tracker: RepeatTracker):
+  test.measurements['example_dimension'][0] = meas_value
+  tracker.increment()
+
+
+@openhtf.PhaseOptions(repeat_on_measurement_fail=True, repeat_limit=5)
+@openhtf.measures(
+    openhtf.Measurement('meas_val').in_range(minimum=-5, maximum=5,))
+def phase_repeat_on_measurement_fail(test, meas_value: int,
+                                     tracker: RepeatTracker):
+  test.measurements['meas_val'] = meas_value
+  tracker.increment()
+
+
 @openhtf.PhaseOptions(run_if=lambda: False)
 def phase_skip_from_run_if(test):
   del test  # Unused.
@@ -1129,15 +1164,16 @@ def test_branch_with_log(self):
         'branch:{}'.format(diag_cond.message))
 
 
-class PhaseExecutorTest(unittest.TestCase):
+class PhaseExecutorTest(parameterized.TestCase):
 
   def setUp(self):
     super(PhaseExecutorTest, self).setUp()
     self.test_state = mock.MagicMock(
         spec=test_state.TestState,
         plug_manager=plugs.PlugManager(),
         execution_uid='01234567890',
-        state_logger=mock.MagicMock())
+        state_logger=mock.MagicMock(),
+        test_record=mock.MagicMock(spec=test_record.TestRecord))
     self.test_state.plug_manager.initialize_plugs(
         [UnittestPlug, MoreRepeatsUnittestPlug])
     self.phase_executor = phase_executor.PhaseExecutor(self.test_state)
@@ -1148,14 +1184,54 @@ def test_execute_continue_phase(self):
 
   def test_execute_repeat_okay_phase(self):
     result, _ = self.phase_executor.execute_phase(
-        phase_repeat.with_plugs(test_plug=UnittestPlug))
+        phase_repeat.with_plugs(test_plug=UnittestPlug)
+    )
     self.assertEqual(openhtf.PhaseResult.CONTINUE, result.phase_result)
 
   def test_execute_repeat_limited_phase(self):
     result, _ = self.phase_executor.execute_phase(
-        phase_repeat.with_plugs(test_plug=MoreRepeatsUnittestPlug))
+        phase_repeat.with_plugs(test_plug=MoreRepeatsUnittestPlug)
+    )
     self.assertEqual(openhtf.PhaseResult.STOP, result.phase_result)
 
+  @parameterized.named_parameters(
+      # NAME, PHASE, MEASUREMENT_VALUE, OUTCOME, EXPECTED_NUMBER_OF_RUNS.
+      # Not failing phase with a simple measurement value in range [-5, +5].
+      ('measurement_phase_not_failing', phase_repeat_on_measurement_fail, 4,
+       test_record.PhaseOutcome.PASS, 1),
+      # Failing phase with simple measurement value out of range.
+      ('measurement_phase_failing', phase_repeat_on_measurement_fail, 10,
+       test_record.PhaseOutcome.FAIL, 5),
+      # Not failing phase with a multidim measurement value in range [-5, +5].
+      ('multidim_measurement_phase_not_failing',
+       phase_repeat_on_multidim_measurement_fail, 4,
+       test_record.PhaseOutcome.PASS, 1),
+      # Failing phase with multidim measurement value out of range.
+      ('multidim_measurement_phase_failing',
+       phase_repeat_on_multidim_measurement_fail, 10,
+       test_record.PhaseOutcome.FAIL, 5),
+  )
+  def test_execute_repeat_on_measurement_fail_phase(self, phase, meas_value,
+                                                    outcome, num_runs):
+    mock_test_state = mock.MagicMock(
+        spec=test_state.TestState,
+        plug_manager=plugs.PlugManager(),
+        execution_uid='01234567890',
+        state_logger=mock.MagicMock(),
+        test_record=test_record.TestRecord('mock-dut-id', 'mock-station-id'))
+    mock_test_state.plug_manager.initialize_plugs(
+        [UnittestPlug, MoreRepeatsUnittestPlug])
+    my_phase_record = test_record.PhaseRecord.from_descriptor(phase)
+    my_phase_record.outcome = outcome
+    mock_test_state.test_record.add_phase_record(my_phase_record)
+    my_phase_executor = phase_executor.PhaseExecutor(mock_test_state)
+    tracker = RepeatTracker()
+    result, _ = my_phase_executor.execute_phase(
+        phase.with_args(tracker=tracker, meas_value=meas_value)
+    )
+    self.assertEqual(openhtf.PhaseResult.CONTINUE, result.phase_result)
+    self.assertEqual(tracker.get_num_repeats(), num_runs)
+
   def test_execute_run_if_false(self):
     result, _ = self.phase_executor.execute_phase(phase_skip_from_run_if)
     self.assertEqual(openhtf.PhaseResult.SKIP, result.phase_result)