Skip to content

Consolidate class-level error record finalization in BaseTestClass - #1032

Merged
xpconanfan merged 1 commit into
masterfrom
consolidate-class-record-finalization
Sep 18, 2026
Merged

xpconanfan merged 1 commit into
masterfrom
consolidate-class-record-finalization

Conversation

@xpconanfan

Copy link
Copy Markdown
Collaborator

Summary

Fixes #1029.

Consolidates all 6 class-level error record finalization sites (_pre_run, _setup_class exception & expects branches, _teardown_class exception & expects branches, and _clean_up) into a single helper method BaseTestClass._record_class_error().

Problem

  1. Lost class error records on abort_all() in on_fail (Issue setup_class error record is not written to test_summary.yaml when on_fail raises TestAbortAll #1029):
    When setup_class fails (via an uncaught exception or expects) and on_fail raises an abort signal (asserts.abort_all() / asserts.abort_class()), summary_writer.dump() (and results.add_class_error() in the expects path) were skipped because they appeared after self._exec_procedure_func(self._on_fail, class_record) without a try...finally guard.
  2. Missing end_time and details inside on_fail for setup_class expects failures:
    In _setup_class, the if expects.recorder.has_error: block previously invoked self._exec_procedure_func(self._on_fail, class_record) before class_record.test_error() and class_record.update_record(). As a result, record.end_time and record.details were None inside on_fail (unlike normal test failures in exec_one_test or uncaught exceptions in _setup_class).

Solution

Introduce _record_class_error(self, record, e=None, exec_on_fail=False) which enforces a canonical lifecycle across all class stages:

  1. Call record.test_error(e) and record.update_record() before running on_fail so record.end_time, record.termination_signal, and record.details are always populated when on_fail executes.
  2. Execute _exec_procedure_func(self._on_fail, record) inside a try...finally block so that self.results.add_class_error(record) and self.summary_writer.dump(...) always execute even if on_fail raises an abort signal.

- Introduce BaseTestClass._record_class_error() to unify the 6 class-level
  error record finalization sites across _pre_run, _setup_class,
  _teardown_class, and _clean_up.
- Ensure record.test_error() and record.update_record() are always invoked
  prior to _exec_procedure_func(self._on_fail, record) so that on_fail
  consistently receives a record with populated end_time and details (including
  when setup_class fails via expects).
- Wrap _on_fail execution in try/finally so that results.add_class_error() and
  summary_writer.dump() are guaranteed to execute even if on_fail raises an
  abort signal such as asserts.abort_all().

Fixes #1029
@xpconanfan xpconanfan added this to the Mobly Release 1.14 milestone Sep 16, 2026
@xpconanfan xpconanfan self-assigned this Sep 16, 2026
@hjfreyer

Copy link
Copy Markdown

I've confirmed that this resolves my issue! Thanks!

@xpconanfan
xpconanfan merged commit 0d925c6 into master Sep 18, 2026
35 checks passed
@xpconanfan
xpconanfan deleted the consolidate-class-record-finalization branch September 18, 2026 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

3 participants