`setup_class` error record is not written to `test_summary.yaml` when `on_fail` raises `TestAbortAll`

[hjfreyer]

Summary

If setup_class raises a regular exception and the test class's on_fail then calls asserts.abort_all(), the setup_class ERROR record is added to the in-memory TestResult but never dumped to the summary file. The resulting test_summary.yaml contains the SKIP records for the class's tests and a final Summary entry with Error: 1, but no RECORD entry accounting for that error.

This was explicitly fixed in #461 (63b8e3f) and regressed in #531 (e8ab13f).

Reproduction

from mobly import asserts, base_test, signals, test_runner

class ReproTest(base_test.BaseTestClass):

  def setup_class(self):
    raise Exception('testbed is unusable')

  def on_fail(self, record):
    asserts.abort_all('aborting: %s' % record.details)

  def test_1(self):
    pass

if __name__ == '__main__':
  test_runner.main()

Expected: test_summary.yaml contains a RECORD with Test Name: setup_class, Result: ERROR, followed by a SKIP record for test_1 and a Summary with Error: 1, Skipped: 1.

Actual: The setup_class RECORD is missing. The SKIP record and Summary (Error: 1, Skipped: 1) are present, so the file is internally inconsistent — one error is counted but no record describes it. Downstream YAML consumers that key on the setup_class record (e.g. to classify a run failure) never see it.

Cause

In BaseTestClass._setup_class, the abort raised from on_fail propagates out of _exec_procedure_func before the record is written:

except Exception as e:
  logging.exception('Error in %s#setup_class.', self.TAG)
  class_record.test_error(e)
  self.results.add_class_error(class_record)               # in-memory only
  self._exec_procedure_func(self._on_fail, class_record)   # TestAbortAll escapes here
  class_record.update_record()                             # not reached
  self.summary_writer.dump(                                # not reached
      class_record.to_dict(), records.TestSummaryEntryType.RECORD
  )
  self._skip_remaining_tests(e)                            # not reached
  return self.results

Control then lands in run()'s except signals.TestAbortAll, which calls _skip_remaining_tests (so the SKIP records are dumped) and re-raises.

The expects.recorder.has_error branch immediately below has the same ordering and the same problem.

History

• Fix behaviors for abort_all related to setup_class. #461 (63b8e3f, 2018-06-12) — commit message: "A record for setup_class should exist if setup_class failed and abort_all is called in on_fail." The diff deliberately moved the summary_writer.dump(...) call before _exec_procedure_func(self._on_fail, ...). Added test_abort_all_in_on_fail_from_setup_class.
• Fix expects for different test stages. #531 (e8ab13f, 2018-10-15) — moved the logic into _setup_class and, to include update_record() after on_fail, placed the dump after the on_fail call again, reintroducing the issue.

The regression test from #461 did not catch this because it only asserts on the in-memory bt_cls.results.error[0], not on what was written via summary_writer.

Suggested fix

Mirror the structure already used in exec_one_test, where update_record() runs first and the dump lives in a finally around the procedure call:

except Exception as e:
  logging.exception('Error in %s#setup_class.', self.TAG)
  class_record.test_error(e)
  self.results.add_class_error(class_record)
  try:
    self._exec_procedure_func(self._on_fail, class_record)
  finally:
    class_record.update_record()
    self.summary_writer.dump(
        class_record.to_dict(), records.TestSummaryEntryType.RECORD
    )
  self._skip_remaining_tests(e)
  return self.results

Apply the same try/finally to the expects.recorder.has_error branch.

Also suggest extending test_abort_all_in_on_fail_from_setup_class to assert that summary_writer.dump was called with a record whose test_name == 'setup_class', so the file-level behavior is covered.

Environment

Reproduced against master (mobly/base_test.py as of the date of filing).