From 9fcad7986c81b4876666d3e1ba81bcd6965ecc92 Mon Sep 17 00:00:00 2001 From: Ang Li Date: Wed, 14 Oct 2026 08:28:59 +0000 Subject: [PATCH 1/8] Enrich TestClassInfoRecord with per-class counts, result, and termination signal --- mobly/base_test.py | 100 +++++++++++++++--------- mobly/records.py | 131 +++++++++++++++++++++++++++++--- mobly/suite_runner.py | 18 +++-- tests/mobly/base_test_test.py | 84 +++++++++++++++++++- tests/mobly/records_test.py | 101 ++++++++++++++++++++++++ tests/mobly/test_runner_test.py | 10 +++ 6 files changed, 388 insertions(+), 56 deletions(-) diff --git a/mobly/base_test.py b/mobly/base_test.py index 78224ac5..ab5b8587 100644 --- a/mobly/base_test.py +++ b/mobly/base_test.py @@ -359,9 +359,10 @@ def _record_controller_info(self): # Collect controller information and write to test result. for record in self._controller_manager.get_controller_info_records(): self.results.add_controller_info_record(record) - self.summary_writer.dump( - record.to_dict(), records.TestSummaryEntryType.CONTROLLER_INFO - ) + if self.summary_writer: + self.summary_writer.dump( + record.to_dict(), records.TestSummaryEntryType.CONTROLLER_INFO + ) def _pre_run(self): """Proxy function to guarantee the base implementation of `pre_run` is @@ -376,9 +377,13 @@ def _pre_run(self): self.current_test_info = runtime_test_info.RuntimeTestInfo( stage_name, self.log_path, record ) + expects.recorder.reset_internal_states(record) try: with self._log_test_stage(stage_name): self.pre_run() + if expects.recorder.has_error: + self._record_class_error(record) + return False return True except _UNCATCHABLE_EXCEPTIONS: raise @@ -402,9 +407,10 @@ def _record_class_error(self, record, e=None, exec_on_fail=False): self._exec_procedure_func(self._on_fail, record) finally: self.results.add_class_error(record) - self.summary_writer.dump( - record.to_dict(), records.TestSummaryEntryType.RECORD - ) + if self.summary_writer: + self.summary_writer.dump( + record.to_dict(), records.TestSummaryEntryType.RECORD + ) def pre_run(self): """Preprocesses that need to be done before setup_class. @@ -435,8 +441,8 @@ def _setup_class(self): try: with self._log_test_stage(STAGE_NAME_SETUP_CLASS): self.setup_class() - except signals.TestAbortSignal: - # Throw abort signals to outer try block for handling. + except (signals.TestAbortSignal, signals.TestSkip): + # Throw abort/skip signals to outer try block for handling. raise except _UNCATCHABLE_EXCEPTIONS: raise @@ -1158,29 +1164,28 @@ def run(self, test_names=None): logging.log_path = self.log_path class_info = records.TestClassInfoRecord(self.TAG) class_info.class_begin() - # Executes pre-setup procedures, like generating test methods. - if not self._pre_run(): - class_info.class_end() + setup_class_reached = False + class_termination_error = None + try: + # Executes pre-setup procedures, like generating test methods. + if not self._pre_run(): + return self.results + logging.info('==========> %s <==========', self.TAG) + # Devise the actual test methods to run in the test class. + if not test_names: + if self.tests: + # Specified by run list in class. + test_names = list(self.tests) + else: + # No test method specified by user, execute all in test class. + test_names = self.get_existing_test_names() + self.results.requested = test_names self.summary_writer.dump( - class_info.to_dict(), records.TestSummaryEntryType.TEST_CLASS_INFO + self.results.requested_test_names_dict(), + records.TestSummaryEntryType.TEST_NAME_LIST, ) - return self.results - logging.info('==========> %s <==========', self.TAG) - # Devise the actual test methods to run in the test class. - if not test_names: - if self.tests: - # Specified by run list in class. - test_names = list(self.tests) - else: - # No test method specified by user, execute all in test class. - test_names = self.get_existing_test_names() - self.results.requested = test_names - self.summary_writer.dump( - self.results.requested_test_names_dict(), - records.TestSummaryEntryType.TEST_NAME_LIST, - ) - tests = self._get_test_methods(test_names) - try: + tests = self._get_test_methods(test_names) + setup_class_reached = True setup_class_result = self._setup_class() if setup_class_result: return setup_class_result @@ -1237,26 +1242,47 @@ def run(self, test_names=None): # Execute test as a regular single test self.exec_one_test(test_name, test_method) return self.results + except signals.TestSkip as e: + class_termination_error = e + self._skip_remaining_tests(e) + return self.results except signals.TestAbortClass as e: e.details = f'Test class aborted due to: {e.details}' + class_termination_error = e self._skip_remaining_tests(e) return self.results except signals.TestAbortAll as e: e.details = f'All remaining tests aborted due to: {e.details}' + class_termination_error = e self._skip_remaining_tests(e) # Piggy-back test results on this exception object so we don't lose # results from this test class. setattr(e, 'results', self.results) raise e + except BaseException as e: + class_termination_error = e + raise finally: - self._teardown_class() - class_info.class_end() - self.summary_writer.dump( - class_info.to_dict(), records.TestSummaryEntryType.TEST_CLASS_INFO - ) - logging.info( - 'Summary for test class %s: %s', self.TAG, self.results.summary_str() - ) + try: + if setup_class_reached: + self._teardown_class() + else: + self._clean_up() + except BaseException as e: + if class_termination_error is None: + class_termination_error = e + raise + finally: + class_info.class_end(results=self.results, e=class_termination_error) + if self.summary_writer: + self.summary_writer.dump( + class_info.to_dict(), records.TestSummaryEntryType.TEST_CLASS_INFO + ) + logging.info( + 'Summary for test class %s: %s', + self.TAG, + self.results.summary_str(), + ) def _clean_up(self): """The final stage of a test class execution.""" diff --git a/mobly/records.py b/mobly/records.py index 8e526dec..43dd3561 100644 --- a/mobly/records.py +++ b/mobly/records.py @@ -99,6 +99,8 @@ class TestSummaryEntryType(enum.Enum): # and ended (including setup_class and teardown_class). Dumped once per test # class after the class finishes. TEST_CLASS_INFO = 'TestClassInfo' + # Information on the execution of a test suite. + SUITE_INFO = 'SuiteInfo' class TestSummaryWriter: @@ -227,31 +229,136 @@ class TestClassInfoRecord: Captures the wall-clock span of the whole class, including `setup_class` and `teardown_class`, which is not derivable from the individual test - records. Times are epoch milliseconds, consistent with `TestResultRecord`. + records, as well as the per-class summary counts and any class-level + termination signal. Times are epoch milliseconds, consistent with + `TestResultRecord`. """ KEY_TEST_CLASS = TestResultEnums.RECORD_CLASS KEY_BEGIN_TIME = TestResultEnums.RECORD_BEGIN_TIME KEY_END_TIME = TestResultEnums.RECORD_END_TIME + KEY_RESULT = TestResultEnums.RECORD_RESULT + KEY_REQUESTED = 'Requested' + KEY_EXECUTED = 'Executed' + KEY_PASSED = 'Passed' + KEY_FAILED = 'Failed' + KEY_SKIPPED = 'Skipped' + KEY_ERROR = 'Error' + KEY_DETAILS = TestResultEnums.RECORD_DETAILS + KEY_TERMINATION_SIGNAL_TYPE = TestResultEnums.RECORD_TERMINATION_SIGNAL_TYPE + KEY_STACKTRACE = TestResultEnums.RECORD_STACKTRACE + KEY_EXTRAS = TestResultEnums.RECORD_EXTRAS def __init__(self, test_class): self.test_class = test_class self.begin_time = None self.end_time = None + self.result = None + self.requested = 0 + self.executed = 0 + self.passed = 0 + self.failed = 0 + self.skipped = 0 + self.error = 0 + self.termination_signal = None + + @property + def details(self): + if self.termination_signal: + return self.termination_signal.details + + @property + def termination_signal_type(self): + if self.termination_signal: + return self.termination_signal.type + + @property + def stacktrace(self): + if self.termination_signal: + return self.termination_signal.stacktrace + + @property + def extras(self): + if self.termination_signal: + return self.termination_signal.extras def class_begin(self): """Marks the beginning of the test class execution.""" self.begin_time = utils.get_current_epoch_time() - def class_end(self): - """Marks the end of the test class execution.""" + def class_end(self, results=None, e=None): + """Marks the end of the test class execution. + + Args: + results: Optional `TestResult` object for this test class. + e: Optional exception or `TestSignal` that terminated the class. + """ self.end_time = utils.get_current_epoch_time() + if e is not None: + self.termination_signal = ExceptionRecord(e) + if results is not None: + summary = results.summary_dict() + self.requested = summary[self.KEY_REQUESTED] + self.executed = summary[self.KEY_EXECUTED] + self.passed = summary[self.KEY_PASSED] + self.failed = summary[self.KEY_FAILED] + self.skipped = summary[self.KEY_SKIPPED] + self.error = summary[self.KEY_ERROR] + if self.termination_signal is None and results.error: + executed_ids = {id(r) for r in results.executed} + for err_record in results.error: + if ( + id(err_record) not in executed_ids + and err_record.termination_signal is not None + ): + self.termination_signal = copy.deepcopy( + err_record.termination_signal + ) + break + if e is not None and not isinstance( + e, (signals.TestAbortSignal, signals.TestSkip, signals.TestPass) + ): + if isinstance(e, (signals.TestFailure, AssertionError)): + self.result = TestResultEnums.TEST_RESULT_FAIL + else: + self.result = TestResultEnums.TEST_RESULT_ERROR + elif results is not None: + if not results.is_all_pass: + retried_ids = results._get_eventually_passing_retry_record_ids() + has_error = any(id(r) not in retried_ids for r in results.error) + self.result = ( + TestResultEnums.TEST_RESULT_ERROR + if has_error + else TestResultEnums.TEST_RESULT_FAIL + ) + elif len(results.passed) == 0 and ( + isinstance(e, (signals.TestAbortSignal, signals.TestSkip)) + or len(results.skipped) > 0 + ): + self.result = TestResultEnums.TEST_RESULT_SKIP + else: + self.result = TestResultEnums.TEST_RESULT_PASS + elif isinstance(e, (signals.TestAbortSignal, signals.TestSkip)): + self.result = TestResultEnums.TEST_RESULT_SKIP + else: + self.result = TestResultEnums.TEST_RESULT_PASS def to_dict(self): result = {} result[self.KEY_TEST_CLASS] = self.test_class result[self.KEY_BEGIN_TIME] = self.begin_time result[self.KEY_END_TIME] = self.end_time + result[self.KEY_RESULT] = self.result + result[self.KEY_REQUESTED] = self.requested + result[self.KEY_EXECUTED] = self.executed + result[self.KEY_PASSED] = self.passed + result[self.KEY_FAILED] = self.failed + result[self.KEY_SKIPPED] = self.skipped + result[self.KEY_ERROR] = self.error + result[self.KEY_DETAILS] = self.details + result[self.KEY_TERMINATION_SIGNAL_TYPE] = self.termination_signal_type + result[self.KEY_STACKTRACE] = self.stacktrace + result[self.KEY_EXTRAS] = copy.deepcopy(self.extras) return result def __repr__(self): @@ -695,6 +802,16 @@ def is_test_executed(self, test_name): return True return False + def _get_eventually_passing_retry_record_ids(self): + """Returns object IDs of non-passing retry iterations that eventually passed.""" + retried_ids = set() + for record in self.passed: + r = record + while r.parent is not None and r.parent[1] == TestParentType.RETRY: + retried_ids.add(id(r.parent[0])) + r = r.parent[0] + return retried_ids + def _count_eventually_passing_retries(self): """Counts the number of retry iterations that eventually passed. @@ -706,13 +823,7 @@ def _count_eventually_passing_retries(self): Int, the number that should be subtracted from the result altering error counts. """ - count = 0 - for record in self.passed: - r = record - while r.parent is not None and r.parent[1] == TestParentType.RETRY: - count += 1 - r = r.parent[0] - return count + return len(self._get_eventually_passing_retry_record_ids()) @property def is_all_pass(self): diff --git a/mobly/suite_runner.py b/mobly/suite_runner.py index 34622c12..9226201e 100644 --- a/mobly/suite_runner.py +++ b/mobly/suite_runner.py @@ -329,7 +329,9 @@ def _dump_suite_info(suite_record, log_path): """Dumps the suite info record to test summary file.""" summary_path = os.path.join(log_path, records.OUTPUT_FILE_SUMMARY) summary_writer = records.TestSummaryWriter(summary_path) - summary_writer.dump(suite_record.to_dict(), TestSummaryEntryType.SUITE_INFO) + summary_writer.dump( + suite_record.to_dict(), records.TestSummaryEntryType.SUITE_INFO + ) def run_suite_class(argv=None): @@ -361,20 +363,22 @@ def run_suite_class(argv=None): console_level = logging.DEBUG if cli_args.verbose else logging.INFO ok = False with runner.mobly_logger(console_level=console_level) as log_path: + suite_record.suite_begin() try: suite.setup_suite(config.copy()) try: - suite_record.suite_begin() runner.run() ok = runner.results.is_all_pass except signals.TestAbortAll: ok = runner.results.is_all_pass finally: - suite.teardown_suite() - suite_record.suite_end() - suite_record.suite_run_display_name = suite.get_suite_run_display_name() - suite_record.extras = suite.get_suite_info().copy() - _dump_suite_info(suite_record, log_path) + try: + suite.teardown_suite() + finally: + suite_record.suite_end() + suite_record.suite_run_display_name = suite.get_suite_run_display_name() + suite_record.extras = suite.get_suite_info().copy() + _dump_suite_info(suite_record, log_path) if not ok: sys.exit(1) diff --git a/tests/mobly/base_test_test.py b/tests/mobly/base_test_test.py index 961e4d0e..d8979790 100755 --- a/tests/mobly/base_test_test.py +++ b/tests/mobly/base_test_test.py @@ -386,9 +386,27 @@ class MockBaseTest(base_test.BaseTestClass): bt_cls = MockBaseTest(self.mock_test_cls_configs) expected_msg = '.* does not have test method test_something' - with self.assertRaisesRegex(base_test.Error, expected_msg): - bt_cls.run(test_names=['test_something']) + with mock.patch.object( + bt_cls.summary_writer, 'dump', wraps=bt_cls.summary_writer.dump + ) as mock_dump: + with self.assertRaisesRegex(base_test.Error, expected_msg): + bt_cls.run(test_names=['test_something']) self.assertFalse(bt_cls.results.executed) + class_info_calls = [ + call + for call in mock_dump.call_args_list + if call[0][1] == records.TestSummaryEntryType.TEST_CLASS_INFO + ] + self.assertEqual(len(class_info_calls), 1) + class_info_dict = class_info_calls[0][0][0] + self.assertEqual( + class_info_dict[records.TestResultEnums.RECORD_RESULT], + records.TestResultEnums.TEST_RESULT_ERROR, + ) + self.assertEqual( + class_info_dict[records.TestResultEnums.RECORD_TERMINATION_SIGNAL_TYPE], + 'Error', + ) def test_setup_class_fail_by_exception(self): teardown_class_call_check = mock.MagicMock() @@ -1156,6 +1174,44 @@ def test_3(self): 'Error 0, Executed 0, Failed 0, Passed 0, Requested 3, Skipped 3', ) + def test_skip_in_setup_class(self): + class MockBaseTest(base_test.BaseTestClass): + + def setup_class(self): + asserts.skip(MSG_EXPECTED_EXCEPTION, extras=MOCK_EXTRA) + + def test_1(self): + never_call() + + def test_2(self): + never_call() + + bt_cls = MockBaseTest(self.mock_test_cls_configs) + with mock.patch.object( + bt_cls.summary_writer, 'dump', wraps=bt_cls.summary_writer.dump + ) as mock_dump: + bt_cls.run(test_names=['test_1', 'test_2']) + self.assertEqual(len(bt_cls.results.skipped), 2) + self.assertEqual(len(bt_cls.results.error), 0) + self.assertEqual(bt_cls.results.skipped[0].details, MSG_EXPECTED_EXCEPTION) + self.assertEqual(bt_cls.results.skipped[0].extras, MOCK_EXTRA) + class_info_dict = [ + call[0][0] + for call in mock_dump.call_args_list + if call[0][1] == records.TestSummaryEntryType.TEST_CLASS_INFO + ][0] + self.assertEqual( + class_info_dict[records.TestResultEnums.RECORD_RESULT], + records.TestResultEnums.TEST_RESULT_SKIP, + ) + self.assertEqual( + class_info_dict[records.TestResultEnums.RECORD_DETAILS], + MSG_EXPECTED_EXCEPTION, + ) + self.assertEqual( + class_info_dict[records.TestResultEnums.RECORD_EXTRAS], MOCK_EXTRA + ) + def test_abort_class_in_setup_test(self): class MockBaseTest(base_test.BaseTestClass): @@ -2100,6 +2156,30 @@ def teardown_class(self): self.assertEqual(actual_record.extras, MOCK_EXTRA) self.assertIsNotNone(actual_record.end_time) + def test_expect_in_pre_run(self): + must_call = mock.Mock() + + class MockBaseTest(base_test.BaseTestClass): + + def pre_run(self): + expects.expect_true(False, MSG_EXPECTED_EXCEPTION, extras=MOCK_EXTRA) + must_call('ha') + + def setup_class(self): + never_call() + + def test_func(self): + never_call() + + bt_cls = MockBaseTest(self.mock_test_cls_configs) + bt_cls.run() + must_call.assert_called_once_with('ha') + actual_record = bt_cls.results.error[0] + self.assertEqual(actual_record.test_name, 'pre_run') + self.assertEqual(actual_record.details, MSG_EXPECTED_EXCEPTION) + self.assertEqual(actual_record.extras, MOCK_EXTRA) + self.assertIsNotNone(actual_record.end_time) + def test_expect_in_setup_test(self): must_call = mock.Mock() must_call2 = mock.Mock() diff --git a/tests/mobly/records_test.py b/tests/mobly/records_test.py index be0a9eb2..218370c4 100755 --- a/tests/mobly/records_test.py +++ b/tests/mobly/records_test.py @@ -567,6 +567,17 @@ def test_test_class_info_record(self): records.TestResultEnums.RECORD_CLASS: 'SomeTestClass', records.TestResultEnums.RECORD_BEGIN_TIME: None, records.TestResultEnums.RECORD_END_TIME: None, + records.TestResultEnums.RECORD_RESULT: None, + 'Requested': 0, + 'Executed': 0, + 'Passed': 0, + 'Failed': 0, + 'Skipped': 0, + 'Error': 0, + records.TestResultEnums.RECORD_DETAILS: None, + records.TestResultEnums.RECORD_TERMINATION_SIGNAL_TYPE: None, + records.TestResultEnums.RECORD_STACKTRACE: None, + records.TestResultEnums.RECORD_EXTRAS: None, }, ) record.class_begin() @@ -574,6 +585,10 @@ def test_test_class_info_record(self): d = record.to_dict() self.assertIsInstance(d[records.TestResultEnums.RECORD_BEGIN_TIME], int) self.assertIsInstance(d[records.TestResultEnums.RECORD_END_TIME], int) + self.assertEqual( + d[records.TestResultEnums.RECORD_RESULT], + records.TestResultEnums.TEST_RESULT_PASS, + ) self.assertLessEqual( d[records.TestResultEnums.RECORD_BEGIN_TIME], d[records.TestResultEnums.RECORD_END_TIME], @@ -581,6 +596,92 @@ def test_test_class_info_record(self): # Must be serializable by the summary writer. yaml.safe_dump(d) + def test_test_class_info_record_with_results_and_signals(self): + # 1. Class with a setup_class error surfaces the class error details. + tr = records.TestResult() + tr.requested = ['test_a', 'test_b'] + cls_err = records.TestResultRecord('setup_class', 'SomeTestClass') + cls_err.test_begin() + cls_err.test_error(RuntimeError('setup boom')) + tr.add_class_error(cls_err) + skip_rec = records.TestResultRecord('test_a', 'SomeTestClass') + skip_rec.test_skip(signals.TestSkip('skipped')) + tr.add_record(skip_rec) + + info = records.TestClassInfoRecord('SomeTestClass') + info.class_begin() + info.class_end(results=tr) + d = info.to_dict() + self.assertEqual( + d[records.TestResultEnums.RECORD_RESULT], + records.TestResultEnums.TEST_RESULT_ERROR, + ) + self.assertEqual(d['Requested'], 2) + self.assertEqual(d['Executed'], 0) + self.assertEqual(d['Skipped'], 1) + self.assertEqual(d['Error'], 1) + self.assertEqual(d[records.TestResultEnums.RECORD_DETAILS], 'setup boom') + self.assertEqual( + d[records.TestResultEnums.RECORD_TERMINATION_SIGNAL_TYPE], + 'RuntimeError', + ) + + # 2. Class cleanly aborted via TestAbortClass with 0 passed tests -> SKIP. + tr_abort = records.TestResult() + tr_abort.requested = ['test_a'] + tr_abort.add_record(skip_rec) + info_abort = records.TestClassInfoRecord('SomeTestClass') + info_abort.class_begin() + info_abort.class_end( + results=tr_abort, + e=signals.TestAbortClass('device not supported', extras={'k': 'v'}), + ) + d_abort = info_abort.to_dict() + self.assertEqual( + d_abort[records.TestResultEnums.RECORD_RESULT], + records.TestResultEnums.TEST_RESULT_SKIP, + ) + self.assertEqual( + d_abort[records.TestResultEnums.RECORD_DETAILS], 'device not supported' + ) + self.assertEqual( + d_abort[records.TestResultEnums.RECORD_TERMINATION_SIGNAL_TYPE], + 'TestAbortClass', + ) + self.assertEqual(d_abort[records.TestResultEnums.RECORD_EXTRAS], {'k': 'v'}) + + # 3. Class with a failed test -> FAIL. + tr_fail = records.TestResult() + tr_fail.requested = ['test_a'] + fail_rec = records.TestResultRecord('test_a', 'SomeTestClass') + fail_rec.test_begin() + fail_rec.test_fail(signals.TestFailure('assertion failed')) + tr_fail.add_record(fail_rec) + info_fail = records.TestClassInfoRecord('SomeTestClass') + info_fail.class_begin() + info_fail.class_end(results=tr_fail) + self.assertEqual( + info_fail.to_dict()[records.TestResultEnums.RECORD_RESULT], + records.TestResultEnums.TEST_RESULT_FAIL, + ) + + # 4. Class terminated by an unhandled ValueError -> ERROR. + info_exc = records.TestClassInfoRecord('SomeTestClass') + info_exc.class_begin() + info_exc.class_end(results=records.TestResult(), e=ValueError('bad repeat')) + d_exc = info_exc.to_dict() + self.assertEqual( + d_exc[records.TestResultEnums.RECORD_RESULT], + records.TestResultEnums.TEST_RESULT_ERROR, + ) + self.assertEqual( + d_exc[records.TestResultEnums.RECORD_DETAILS], 'bad repeat' + ) + self.assertEqual( + d_exc[records.TestResultEnums.RECORD_TERMINATION_SIGNAL_TYPE], + 'ValueError', + ) + if __name__ == '__main__': unittest.main() diff --git a/tests/mobly/test_runner_test.py b/tests/mobly/test_runner_test.py index 93d55598..02cc5f03 100755 --- a/tests/mobly/test_runner_test.py +++ b/tests/mobly/test_runner_test.py @@ -160,6 +160,16 @@ def test_summary_file_entries(self): self.assertIsInstance( summary_entries[3][records.TestResultEnums.RECORD_END_TIME], int ) + self.assertEqual( + summary_entries[3][records.TestResultEnums.RECORD_RESULT], + records.TestResultEnums.TEST_RESULT_PASS, + ) + self.assertEqual(summary_entries[3]['Requested'], 1) + self.assertEqual(summary_entries[3]['Executed'], 1) + self.assertEqual(summary_entries[3]['Passed'], 1) + self.assertEqual(summary_entries[3]['Failed'], 0) + self.assertEqual(summary_entries[3]['Skipped'], 0) + self.assertEqual(summary_entries[3]['Error'], 0) self.assertEqual( summary_entries[4]['Type'], records.TestSummaryEntryType.SUMMARY.value ) From 3f6f279d293c2bf66150dbdf103a3c5822d04a2a Mon Sep 17 00:00:00 2001 From: Ang Li Date: Wed, 14 Oct 2026 09:03:26 +0000 Subject: [PATCH 2/8] Address review comments on TestClassInfoRecord properties and teardown exception handling --- mobly/base_test.py | 4 +++- mobly/records.py | 14 ++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/mobly/base_test.py b/mobly/base_test.py index ab5b8587..12464c61 100644 --- a/mobly/base_test.py +++ b/mobly/base_test.py @@ -1269,7 +1269,9 @@ def run(self, test_names=None): else: self._clean_up() except BaseException as e: - if class_termination_error is None: + if class_termination_error is None or isinstance( + class_termination_error, (signals.TestAbortClass, signals.TestSkip) + ): class_termination_error = e raise finally: diff --git a/mobly/records.py b/mobly/records.py index 43dd3561..6b928066 100644 --- a/mobly/records.py +++ b/mobly/records.py @@ -264,23 +264,21 @@ def __init__(self, test_class): @property def details(self): - if self.termination_signal: - return self.termination_signal.details + return self.termination_signal.details if self.termination_signal else None @property def termination_signal_type(self): - if self.termination_signal: - return self.termination_signal.type + return self.termination_signal.type if self.termination_signal else None @property def stacktrace(self): - if self.termination_signal: - return self.termination_signal.stacktrace + return ( + self.termination_signal.stacktrace if self.termination_signal else None + ) @property def extras(self): - if self.termination_signal: - return self.termination_signal.extras + return self.termination_signal.extras if self.termination_signal else None def class_begin(self): """Marks the beginning of the test class execution.""" From a1f509f2e6753f199fe32dce8c7ad30af7034a99 Mon Sep 17 00:00:00 2001 From: Ang Li Date: Thu, 15 Oct 2026 22:33:33 +0000 Subject: [PATCH 3/8] Drop per-class counts and simplify run() lifecycle per review * TestClassInfoRecord no longer carries Requested/Executed/Passed/Failed/ Skipped/Error counts; it keeps Result and the termination signal. * run() keeps _pre_run, test-name resolution and _get_test_methods outside the try block as before, so no setup_class_reached flag is needed and _teardown_class() is always the finalizer. * Remove the summary_writer None-guards; summary_writer is always set by TestRunner, consistent with the other dump call sites. --- mobly/base_test.py | 68 ++++++++++++++++----------------- mobly/records.py | 30 +-------------- tests/mobly/base_test_test.py | 22 +---------- tests/mobly/records_test.py | 10 ----- tests/mobly/test_runner_test.py | 6 --- 5 files changed, 36 insertions(+), 100 deletions(-) diff --git a/mobly/base_test.py b/mobly/base_test.py index 12464c61..5e107e4d 100644 --- a/mobly/base_test.py +++ b/mobly/base_test.py @@ -359,10 +359,9 @@ def _record_controller_info(self): # Collect controller information and write to test result. for record in self._controller_manager.get_controller_info_records(): self.results.add_controller_info_record(record) - if self.summary_writer: - self.summary_writer.dump( - record.to_dict(), records.TestSummaryEntryType.CONTROLLER_INFO - ) + self.summary_writer.dump( + record.to_dict(), records.TestSummaryEntryType.CONTROLLER_INFO + ) def _pre_run(self): """Proxy function to guarantee the base implementation of `pre_run` is @@ -407,10 +406,9 @@ def _record_class_error(self, record, e=None, exec_on_fail=False): self._exec_procedure_func(self._on_fail, record) finally: self.results.add_class_error(record) - if self.summary_writer: - self.summary_writer.dump( - record.to_dict(), records.TestSummaryEntryType.RECORD - ) + self.summary_writer.dump( + record.to_dict(), records.TestSummaryEntryType.RECORD + ) def pre_run(self): """Preprocesses that need to be done before setup_class. @@ -1164,28 +1162,30 @@ def run(self, test_names=None): logging.log_path = self.log_path class_info = records.TestClassInfoRecord(self.TAG) class_info.class_begin() - setup_class_reached = False - class_termination_error = None - try: - # Executes pre-setup procedures, like generating test methods. - if not self._pre_run(): - return self.results - logging.info('==========> %s <==========', self.TAG) - # Devise the actual test methods to run in the test class. - if not test_names: - if self.tests: - # Specified by run list in class. - test_names = list(self.tests) - else: - # No test method specified by user, execute all in test class. - test_names = self.get_existing_test_names() - self.results.requested = test_names + # Executes pre-setup procedures, like generating test methods. + if not self._pre_run(): + class_info.class_end(results=self.results) self.summary_writer.dump( - self.results.requested_test_names_dict(), - records.TestSummaryEntryType.TEST_NAME_LIST, + class_info.to_dict(), records.TestSummaryEntryType.TEST_CLASS_INFO ) - tests = self._get_test_methods(test_names) - setup_class_reached = True + return self.results + logging.info('==========> %s <==========', self.TAG) + # Devise the actual test methods to run in the test class. + if not test_names: + if self.tests: + # Specified by run list in class. + test_names = list(self.tests) + else: + # No test method specified by user, execute all in test class. + test_names = self.get_existing_test_names() + self.results.requested = test_names + self.summary_writer.dump( + self.results.requested_test_names_dict(), + records.TestSummaryEntryType.TEST_NAME_LIST, + ) + tests = self._get_test_methods(test_names) + class_termination_error = None + try: setup_class_result = self._setup_class() if setup_class_result: return setup_class_result @@ -1264,10 +1264,7 @@ def run(self, test_names=None): raise finally: try: - if setup_class_reached: - self._teardown_class() - else: - self._clean_up() + self._teardown_class() except BaseException as e: if class_termination_error is None or isinstance( class_termination_error, (signals.TestAbortClass, signals.TestSkip) @@ -1276,10 +1273,9 @@ def run(self, test_names=None): raise finally: class_info.class_end(results=self.results, e=class_termination_error) - if self.summary_writer: - self.summary_writer.dump( - class_info.to_dict(), records.TestSummaryEntryType.TEST_CLASS_INFO - ) + self.summary_writer.dump( + class_info.to_dict(), records.TestSummaryEntryType.TEST_CLASS_INFO + ) logging.info( 'Summary for test class %s: %s', self.TAG, diff --git a/mobly/records.py b/mobly/records.py index 6b928066..9811f4fc 100644 --- a/mobly/records.py +++ b/mobly/records.py @@ -229,21 +229,14 @@ class TestClassInfoRecord: Captures the wall-clock span of the whole class, including `setup_class` and `teardown_class`, which is not derivable from the individual test - records, as well as the per-class summary counts and any class-level - termination signal. Times are epoch milliseconds, consistent with - `TestResultRecord`. + records, as well as the class result and any class-level termination + signal. Times are epoch milliseconds, consistent with `TestResultRecord`. """ KEY_TEST_CLASS = TestResultEnums.RECORD_CLASS KEY_BEGIN_TIME = TestResultEnums.RECORD_BEGIN_TIME KEY_END_TIME = TestResultEnums.RECORD_END_TIME KEY_RESULT = TestResultEnums.RECORD_RESULT - KEY_REQUESTED = 'Requested' - KEY_EXECUTED = 'Executed' - KEY_PASSED = 'Passed' - KEY_FAILED = 'Failed' - KEY_SKIPPED = 'Skipped' - KEY_ERROR = 'Error' KEY_DETAILS = TestResultEnums.RECORD_DETAILS KEY_TERMINATION_SIGNAL_TYPE = TestResultEnums.RECORD_TERMINATION_SIGNAL_TYPE KEY_STACKTRACE = TestResultEnums.RECORD_STACKTRACE @@ -254,12 +247,6 @@ def __init__(self, test_class): self.begin_time = None self.end_time = None self.result = None - self.requested = 0 - self.executed = 0 - self.passed = 0 - self.failed = 0 - self.skipped = 0 - self.error = 0 self.termination_signal = None @property @@ -295,13 +282,6 @@ def class_end(self, results=None, e=None): if e is not None: self.termination_signal = ExceptionRecord(e) if results is not None: - summary = results.summary_dict() - self.requested = summary[self.KEY_REQUESTED] - self.executed = summary[self.KEY_EXECUTED] - self.passed = summary[self.KEY_PASSED] - self.failed = summary[self.KEY_FAILED] - self.skipped = summary[self.KEY_SKIPPED] - self.error = summary[self.KEY_ERROR] if self.termination_signal is None and results.error: executed_ids = {id(r) for r in results.executed} for err_record in results.error: @@ -347,12 +327,6 @@ def to_dict(self): result[self.KEY_BEGIN_TIME] = self.begin_time result[self.KEY_END_TIME] = self.end_time result[self.KEY_RESULT] = self.result - result[self.KEY_REQUESTED] = self.requested - result[self.KEY_EXECUTED] = self.executed - result[self.KEY_PASSED] = self.passed - result[self.KEY_FAILED] = self.failed - result[self.KEY_SKIPPED] = self.skipped - result[self.KEY_ERROR] = self.error result[self.KEY_DETAILS] = self.details result[self.KEY_TERMINATION_SIGNAL_TYPE] = self.termination_signal_type result[self.KEY_STACKTRACE] = self.stacktrace diff --git a/tests/mobly/base_test_test.py b/tests/mobly/base_test_test.py index d8979790..41248258 100755 --- a/tests/mobly/base_test_test.py +++ b/tests/mobly/base_test_test.py @@ -386,27 +386,9 @@ class MockBaseTest(base_test.BaseTestClass): bt_cls = MockBaseTest(self.mock_test_cls_configs) expected_msg = '.* does not have test method test_something' - with mock.patch.object( - bt_cls.summary_writer, 'dump', wraps=bt_cls.summary_writer.dump - ) as mock_dump: - with self.assertRaisesRegex(base_test.Error, expected_msg): - bt_cls.run(test_names=['test_something']) + with self.assertRaisesRegex(base_test.Error, expected_msg): + bt_cls.run(test_names=['test_something']) self.assertFalse(bt_cls.results.executed) - class_info_calls = [ - call - for call in mock_dump.call_args_list - if call[0][1] == records.TestSummaryEntryType.TEST_CLASS_INFO - ] - self.assertEqual(len(class_info_calls), 1) - class_info_dict = class_info_calls[0][0][0] - self.assertEqual( - class_info_dict[records.TestResultEnums.RECORD_RESULT], - records.TestResultEnums.TEST_RESULT_ERROR, - ) - self.assertEqual( - class_info_dict[records.TestResultEnums.RECORD_TERMINATION_SIGNAL_TYPE], - 'Error', - ) def test_setup_class_fail_by_exception(self): teardown_class_call_check = mock.MagicMock() diff --git a/tests/mobly/records_test.py b/tests/mobly/records_test.py index 218370c4..0f1d1f04 100755 --- a/tests/mobly/records_test.py +++ b/tests/mobly/records_test.py @@ -568,12 +568,6 @@ def test_test_class_info_record(self): records.TestResultEnums.RECORD_BEGIN_TIME: None, records.TestResultEnums.RECORD_END_TIME: None, records.TestResultEnums.RECORD_RESULT: None, - 'Requested': 0, - 'Executed': 0, - 'Passed': 0, - 'Failed': 0, - 'Skipped': 0, - 'Error': 0, records.TestResultEnums.RECORD_DETAILS: None, records.TestResultEnums.RECORD_TERMINATION_SIGNAL_TYPE: None, records.TestResultEnums.RECORD_STACKTRACE: None, @@ -616,10 +610,6 @@ def test_test_class_info_record_with_results_and_signals(self): d[records.TestResultEnums.RECORD_RESULT], records.TestResultEnums.TEST_RESULT_ERROR, ) - self.assertEqual(d['Requested'], 2) - self.assertEqual(d['Executed'], 0) - self.assertEqual(d['Skipped'], 1) - self.assertEqual(d['Error'], 1) self.assertEqual(d[records.TestResultEnums.RECORD_DETAILS], 'setup boom') self.assertEqual( d[records.TestResultEnums.RECORD_TERMINATION_SIGNAL_TYPE], diff --git a/tests/mobly/test_runner_test.py b/tests/mobly/test_runner_test.py index 02cc5f03..597320e2 100755 --- a/tests/mobly/test_runner_test.py +++ b/tests/mobly/test_runner_test.py @@ -164,12 +164,6 @@ def test_summary_file_entries(self): summary_entries[3][records.TestResultEnums.RECORD_RESULT], records.TestResultEnums.TEST_RESULT_PASS, ) - self.assertEqual(summary_entries[3]['Requested'], 1) - self.assertEqual(summary_entries[3]['Executed'], 1) - self.assertEqual(summary_entries[3]['Passed'], 1) - self.assertEqual(summary_entries[3]['Failed'], 0) - self.assertEqual(summary_entries[3]['Skipped'], 0) - self.assertEqual(summary_entries[3]['Error'], 0) self.assertEqual( summary_entries[4]['Type'], records.TestSummaryEntryType.SUMMARY.value ) From 041a23bde290afc38de55062a0a373fe557dea80 Mon Sep 17 00:00:00 2001 From: Ang Li Date: Thu, 15 Oct 2026 22:41:27 +0000 Subject: [PATCH 4/8] Simplify TestClassInfoRecord and class termination capture * class_end(results, e=None): derive the termination signal from e or the first class-stage error record, and the result from results.is_all_pass in one short decision chain; drop the four pass-through properties and build to_dict as a literal. * Replace _get_eventually_passing_retry_record_ids with _eventually_passing_retries returning the records themselves. * In run(), whatever escapes _teardown_class is recorded as the terminator, with no precedence special-casing. --- mobly/base_test.py | 10 ++- mobly/records.py | 128 ++++++++++++++---------------------- tests/mobly/records_test.py | 2 +- 3 files changed, 53 insertions(+), 87 deletions(-) diff --git a/mobly/base_test.py b/mobly/base_test.py index 5e107e4d..e6fc3f90 100644 --- a/mobly/base_test.py +++ b/mobly/base_test.py @@ -1164,7 +1164,7 @@ def run(self, test_names=None): class_info.class_begin() # Executes pre-setup procedures, like generating test methods. if not self._pre_run(): - class_info.class_end(results=self.results) + class_info.class_end(self.results) self.summary_writer.dump( class_info.to_dict(), records.TestSummaryEntryType.TEST_CLASS_INFO ) @@ -1266,13 +1266,11 @@ def run(self, test_names=None): try: self._teardown_class() except BaseException as e: - if class_termination_error is None or isinstance( - class_termination_error, (signals.TestAbortClass, signals.TestSkip) - ): - class_termination_error = e + # Whatever escapes teardown is what the caller sees terminate the class. + class_termination_error = e raise finally: - class_info.class_end(results=self.results, e=class_termination_error) + class_info.class_end(self.results, class_termination_error) self.summary_writer.dump( class_info.to_dict(), records.TestSummaryEntryType.TEST_CLASS_INFO ) diff --git a/mobly/records.py b/mobly/records.py index 9811f4fc..2b10659f 100644 --- a/mobly/records.py +++ b/mobly/records.py @@ -229,8 +229,9 @@ class TestClassInfoRecord: Captures the wall-clock span of the whole class, including `setup_class` and `teardown_class`, which is not derivable from the individual test - records, as well as the class result and any class-level termination - signal. Times are epoch milliseconds, consistent with `TestResultRecord`. + records, the overall result of the class, and the signal that terminated + the class early, if any. Times are epoch milliseconds, consistent with + `TestResultRecord`. """ KEY_TEST_CLASS = TestResultEnums.RECORD_CLASS @@ -249,89 +250,55 @@ def __init__(self, test_class): self.result = None self.termination_signal = None - @property - def details(self): - return self.termination_signal.details if self.termination_signal else None - - @property - def termination_signal_type(self): - return self.termination_signal.type if self.termination_signal else None - - @property - def stacktrace(self): - return ( - self.termination_signal.stacktrace if self.termination_signal else None - ) - - @property - def extras(self): - return self.termination_signal.extras if self.termination_signal else None - def class_begin(self): """Marks the beginning of the test class execution.""" self.begin_time = utils.get_current_epoch_time() - def class_end(self, results=None, e=None): + def class_end(self, results, e=None): """Marks the end of the test class execution. Args: - results: Optional `TestResult` object for this test class. - e: Optional exception or `TestSignal` that terminated the class. + results: The `TestResult` of the class. + e: The exception or `TestSignal` that terminated the class early, if + any. Without one, the first class-stage error (`pre_run`, + `setup_class`, `teardown_class`) becomes the termination signal. """ self.end_time = utils.get_current_epoch_time() if e is not None: self.termination_signal = ExceptionRecord(e) - if results is not None: - if self.termination_signal is None and results.error: - executed_ids = {id(r) for r in results.executed} - for err_record in results.error: - if ( - id(err_record) not in executed_ids - and err_record.termination_signal is not None - ): - self.termination_signal = copy.deepcopy( - err_record.termination_signal - ) - break - if e is not None and not isinstance( - e, (signals.TestAbortSignal, signals.TestSkip, signals.TestPass) - ): - if isinstance(e, (signals.TestFailure, AssertionError)): - self.result = TestResultEnums.TEST_RESULT_FAIL - else: - self.result = TestResultEnums.TEST_RESULT_ERROR - elif results is not None: - if not results.is_all_pass: - retried_ids = results._get_eventually_passing_retry_record_ids() - has_error = any(id(r) not in retried_ids for r in results.error) - self.result = ( - TestResultEnums.TEST_RESULT_ERROR - if has_error - else TestResultEnums.TEST_RESULT_FAIL - ) - elif len(results.passed) == 0 and ( - isinstance(e, (signals.TestAbortSignal, signals.TestSkip)) - or len(results.skipped) > 0 - ): - self.result = TestResultEnums.TEST_RESULT_SKIP - else: - self.result = TestResultEnums.TEST_RESULT_PASS - elif isinstance(e, (signals.TestAbortSignal, signals.TestSkip)): - self.result = TestResultEnums.TEST_RESULT_SKIP else: + # Class-stage errors are the error records that are not executed tests. + for record in results.error: + if record not in results.executed: + self.termination_signal = record.termination_signal + break + aborted = isinstance(e, (signals.TestAbortSignal, signals.TestSkip)) + if e is not None and not aborted: + self.result = TestResultEnums.TEST_RESULT_ERROR + elif not results.is_all_pass: + retried = results._eventually_passing_retries() + self.result = ( + TestResultEnums.TEST_RESULT_ERROR + if any(r not in retried for r in results.error) + else TestResultEnums.TEST_RESULT_FAIL + ) + elif results.passed or not (results.skipped or aborted): self.result = TestResultEnums.TEST_RESULT_PASS + else: + self.result = TestResultEnums.TEST_RESULT_SKIP def to_dict(self): - result = {} - result[self.KEY_TEST_CLASS] = self.test_class - result[self.KEY_BEGIN_TIME] = self.begin_time - result[self.KEY_END_TIME] = self.end_time - result[self.KEY_RESULT] = self.result - result[self.KEY_DETAILS] = self.details - result[self.KEY_TERMINATION_SIGNAL_TYPE] = self.termination_signal_type - result[self.KEY_STACKTRACE] = self.stacktrace - result[self.KEY_EXTRAS] = copy.deepcopy(self.extras) - return result + signal = self.termination_signal + return { + self.KEY_TEST_CLASS: self.test_class, + self.KEY_BEGIN_TIME: self.begin_time, + self.KEY_END_TIME: self.end_time, + self.KEY_RESULT: self.result, + self.KEY_DETAILS: signal.details if signal else None, + self.KEY_TERMINATION_SIGNAL_TYPE: signal.type if signal else None, + self.KEY_STACKTRACE: signal.stacktrace if signal else None, + self.KEY_EXTRAS: copy.deepcopy(signal.extras) if signal else None, + } def __repr__(self): return str(self.to_dict()) @@ -774,28 +741,29 @@ def is_test_executed(self, test_name): return True return False - def _get_eventually_passing_retry_record_ids(self): - """Returns object IDs of non-passing retry iterations that eventually passed.""" - retried_ids = set() + def _eventually_passing_retries(self): + """Returns the non-passing retry iterations of tests that eventually passed. + + If a test is retried and eventually passed, all the associated non-passing + iterations should not be considered when devising the final state of the + test run. + """ + retried = [] for record in self.passed: r = record while r.parent is not None and r.parent[1] == TestParentType.RETRY: - retried_ids.add(id(r.parent[0])) + retried.append(r.parent[0]) r = r.parent[0] - return retried_ids + return retried def _count_eventually_passing_retries(self): """Counts the number of retry iterations that eventually passed. - If a test is retried and eventually passed, all the associated non-passing - iterations should not be considered when devising the final state of the - test run. - Returns: Int, the number that should be subtracted from the result altering error counts. """ - return len(self._get_eventually_passing_retry_record_ids()) + return len(self._eventually_passing_retries()) @property def is_all_pass(self): diff --git a/tests/mobly/records_test.py b/tests/mobly/records_test.py index 0f1d1f04..2c8f4656 100755 --- a/tests/mobly/records_test.py +++ b/tests/mobly/records_test.py @@ -575,7 +575,7 @@ def test_test_class_info_record(self): }, ) record.class_begin() - record.class_end() + record.class_end(records.TestResult()) d = record.to_dict() self.assertIsInstance(d[records.TestResultEnums.RECORD_BEGIN_TIME], int) self.assertIsInstance(d[records.TestResultEnums.RECORD_END_TIME], int) From 764967f0a1b1323dac3dfc15ac432e050ec33582 Mon Sep 17 00:00:00 2001 From: Ang Li Date: Thu, 15 Oct 2026 22:57:34 +0000 Subject: [PATCH 5/8] Address review: drop TestSkip handling, KEY_ aliases and retry refactor * Result is derived from TestResult alone: not all-pass -> ERROR if any error record else FAIL; otherwise PASS unless only skips -> SKIP. No signal-type inspection. * to_dict uses TestResultEnums directly instead of 1:1 KEY_ aliases. * _count_eventually_passing_retries is back to its original form. * TestSkip in setup_class is out of scope for this PR; removed with its test. --- mobly/base_test.py | 8 ++--- mobly/records.py | 64 ++++++++++++++--------------------- tests/mobly/base_test_test.py | 38 --------------------- tests/mobly/records_test.py | 6 +--- 4 files changed, 29 insertions(+), 87 deletions(-) diff --git a/mobly/base_test.py b/mobly/base_test.py index e6fc3f90..45850aa5 100644 --- a/mobly/base_test.py +++ b/mobly/base_test.py @@ -439,8 +439,8 @@ def _setup_class(self): try: with self._log_test_stage(STAGE_NAME_SETUP_CLASS): self.setup_class() - except (signals.TestAbortSignal, signals.TestSkip): - # Throw abort/skip signals to outer try block for handling. + except signals.TestAbortSignal: + # Throw abort signals to outer try block for handling. raise except _UNCATCHABLE_EXCEPTIONS: raise @@ -1242,10 +1242,6 @@ def run(self, test_names=None): # Execute test as a regular single test self.exec_one_test(test_name, test_method) return self.results - except signals.TestSkip as e: - class_termination_error = e - self._skip_remaining_tests(e) - return self.results except signals.TestAbortClass as e: e.details = f'Test class aborted due to: {e.details}' class_termination_error = e diff --git a/mobly/records.py b/mobly/records.py index 2b10659f..0b047ab8 100644 --- a/mobly/records.py +++ b/mobly/records.py @@ -234,15 +234,6 @@ class TestClassInfoRecord: `TestResultRecord`. """ - KEY_TEST_CLASS = TestResultEnums.RECORD_CLASS - KEY_BEGIN_TIME = TestResultEnums.RECORD_BEGIN_TIME - KEY_END_TIME = TestResultEnums.RECORD_END_TIME - KEY_RESULT = TestResultEnums.RECORD_RESULT - KEY_DETAILS = TestResultEnums.RECORD_DETAILS - KEY_TERMINATION_SIGNAL_TYPE = TestResultEnums.RECORD_TERMINATION_SIGNAL_TYPE - KEY_STACKTRACE = TestResultEnums.RECORD_STACKTRACE - KEY_EXTRAS = TestResultEnums.RECORD_EXTRAS - def __init__(self, test_class): self.test_class = test_class self.begin_time = None @@ -272,17 +263,13 @@ def class_end(self, results, e=None): if record not in results.executed: self.termination_signal = record.termination_signal break - aborted = isinstance(e, (signals.TestAbortSignal, signals.TestSkip)) - if e is not None and not aborted: - self.result = TestResultEnums.TEST_RESULT_ERROR - elif not results.is_all_pass: - retried = results._eventually_passing_retries() + if not results.is_all_pass: self.result = ( TestResultEnums.TEST_RESULT_ERROR - if any(r not in retried for r in results.error) + if results.error else TestResultEnums.TEST_RESULT_FAIL ) - elif results.passed or not (results.skipped or aborted): + elif results.passed or not results.skipped: self.result = TestResultEnums.TEST_RESULT_PASS else: self.result = TestResultEnums.TEST_RESULT_SKIP @@ -290,14 +277,20 @@ def class_end(self, results, e=None): def to_dict(self): signal = self.termination_signal return { - self.KEY_TEST_CLASS: self.test_class, - self.KEY_BEGIN_TIME: self.begin_time, - self.KEY_END_TIME: self.end_time, - self.KEY_RESULT: self.result, - self.KEY_DETAILS: signal.details if signal else None, - self.KEY_TERMINATION_SIGNAL_TYPE: signal.type if signal else None, - self.KEY_STACKTRACE: signal.stacktrace if signal else None, - self.KEY_EXTRAS: copy.deepcopy(signal.extras) if signal else None, + TestResultEnums.RECORD_CLASS: self.test_class, + TestResultEnums.RECORD_BEGIN_TIME: self.begin_time, + TestResultEnums.RECORD_END_TIME: self.end_time, + TestResultEnums.RECORD_RESULT: self.result, + TestResultEnums.RECORD_DETAILS: signal.details if signal else None, + TestResultEnums.RECORD_TERMINATION_SIGNAL_TYPE: ( + signal.type if signal else None + ), + TestResultEnums.RECORD_STACKTRACE: ( + signal.stacktrace if signal else None + ), + TestResultEnums.RECORD_EXTRAS: ( + copy.deepcopy(signal.extras) if signal else None + ), } def __repr__(self): @@ -741,29 +734,24 @@ def is_test_executed(self, test_name): return True return False - def _eventually_passing_retries(self): - """Returns the non-passing retry iterations of tests that eventually passed. + def _count_eventually_passing_retries(self): + """Counts the number of retry iterations that eventually passed. If a test is retried and eventually passed, all the associated non-passing iterations should not be considered when devising the final state of the test run. - """ - retried = [] - for record in self.passed: - r = record - while r.parent is not None and r.parent[1] == TestParentType.RETRY: - retried.append(r.parent[0]) - r = r.parent[0] - return retried - - def _count_eventually_passing_retries(self): - """Counts the number of retry iterations that eventually passed. Returns: Int, the number that should be subtracted from the result altering error counts. """ - return len(self._eventually_passing_retries()) + count = 0 + for record in self.passed: + r = record + while r.parent is not None and r.parent[1] == TestParentType.RETRY: + count += 1 + r = r.parent[0] + return count @property def is_all_pass(self): diff --git a/tests/mobly/base_test_test.py b/tests/mobly/base_test_test.py index 41248258..a976eb96 100755 --- a/tests/mobly/base_test_test.py +++ b/tests/mobly/base_test_test.py @@ -1156,44 +1156,6 @@ def test_3(self): 'Error 0, Executed 0, Failed 0, Passed 0, Requested 3, Skipped 3', ) - def test_skip_in_setup_class(self): - class MockBaseTest(base_test.BaseTestClass): - - def setup_class(self): - asserts.skip(MSG_EXPECTED_EXCEPTION, extras=MOCK_EXTRA) - - def test_1(self): - never_call() - - def test_2(self): - never_call() - - bt_cls = MockBaseTest(self.mock_test_cls_configs) - with mock.patch.object( - bt_cls.summary_writer, 'dump', wraps=bt_cls.summary_writer.dump - ) as mock_dump: - bt_cls.run(test_names=['test_1', 'test_2']) - self.assertEqual(len(bt_cls.results.skipped), 2) - self.assertEqual(len(bt_cls.results.error), 0) - self.assertEqual(bt_cls.results.skipped[0].details, MSG_EXPECTED_EXCEPTION) - self.assertEqual(bt_cls.results.skipped[0].extras, MOCK_EXTRA) - class_info_dict = [ - call[0][0] - for call in mock_dump.call_args_list - if call[0][1] == records.TestSummaryEntryType.TEST_CLASS_INFO - ][0] - self.assertEqual( - class_info_dict[records.TestResultEnums.RECORD_RESULT], - records.TestResultEnums.TEST_RESULT_SKIP, - ) - self.assertEqual( - class_info_dict[records.TestResultEnums.RECORD_DETAILS], - MSG_EXPECTED_EXCEPTION, - ) - self.assertEqual( - class_info_dict[records.TestResultEnums.RECORD_EXTRAS], MOCK_EXTRA - ) - def test_abort_class_in_setup_test(self): class MockBaseTest(base_test.BaseTestClass): diff --git a/tests/mobly/records_test.py b/tests/mobly/records_test.py index 2c8f4656..22dcecc1 100755 --- a/tests/mobly/records_test.py +++ b/tests/mobly/records_test.py @@ -655,15 +655,11 @@ def test_test_class_info_record_with_results_and_signals(self): records.TestResultEnums.TEST_RESULT_FAIL, ) - # 4. Class terminated by an unhandled ValueError -> ERROR. + # 4. An exception terminating the class is recorded as the signal. info_exc = records.TestClassInfoRecord('SomeTestClass') info_exc.class_begin() info_exc.class_end(results=records.TestResult(), e=ValueError('bad repeat')) d_exc = info_exc.to_dict() - self.assertEqual( - d_exc[records.TestResultEnums.RECORD_RESULT], - records.TestResultEnums.TEST_RESULT_ERROR, - ) self.assertEqual( d_exc[records.TestResultEnums.RECORD_DETAILS], 'bad repeat' ) From 89598cbd6fbd1f79acc91f6bc3effe2e03c86084 Mon Sep 17 00:00:00 2001 From: Ang Li Date: Thu, 15 Oct 2026 23:04:54 +0000 Subject: [PATCH 6/8] Drop suite_runner changes; keep this PR to the class-level record --- mobly/records.py | 2 -- mobly/suite_runner.py | 18 +++++++----------- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/mobly/records.py b/mobly/records.py index 0b047ab8..1c8ba17d 100644 --- a/mobly/records.py +++ b/mobly/records.py @@ -99,8 +99,6 @@ class TestSummaryEntryType(enum.Enum): # and ended (including setup_class and teardown_class). Dumped once per test # class after the class finishes. TEST_CLASS_INFO = 'TestClassInfo' - # Information on the execution of a test suite. - SUITE_INFO = 'SuiteInfo' class TestSummaryWriter: diff --git a/mobly/suite_runner.py b/mobly/suite_runner.py index 9226201e..34622c12 100644 --- a/mobly/suite_runner.py +++ b/mobly/suite_runner.py @@ -329,9 +329,7 @@ def _dump_suite_info(suite_record, log_path): """Dumps the suite info record to test summary file.""" summary_path = os.path.join(log_path, records.OUTPUT_FILE_SUMMARY) summary_writer = records.TestSummaryWriter(summary_path) - summary_writer.dump( - suite_record.to_dict(), records.TestSummaryEntryType.SUITE_INFO - ) + summary_writer.dump(suite_record.to_dict(), TestSummaryEntryType.SUITE_INFO) def run_suite_class(argv=None): @@ -363,22 +361,20 @@ def run_suite_class(argv=None): console_level = logging.DEBUG if cli_args.verbose else logging.INFO ok = False with runner.mobly_logger(console_level=console_level) as log_path: - suite_record.suite_begin() try: suite.setup_suite(config.copy()) try: + suite_record.suite_begin() runner.run() ok = runner.results.is_all_pass except signals.TestAbortAll: ok = runner.results.is_all_pass finally: - try: - suite.teardown_suite() - finally: - suite_record.suite_end() - suite_record.suite_run_display_name = suite.get_suite_run_display_name() - suite_record.extras = suite.get_suite_info().copy() - _dump_suite_info(suite_record, log_path) + suite.teardown_suite() + suite_record.suite_end() + suite_record.suite_run_display_name = suite.get_suite_run_display_name() + suite_record.extras = suite.get_suite_info().copy() + _dump_suite_info(suite_record, log_path) if not ok: sys.exit(1) From 11f75d9ebe2b1de4c3ddab6e8977473431ae33c9 Mon Sep 17 00:00:00 2001 From: Ang Li Date: Thu, 15 Oct 2026 23:23:52 +0000 Subject: [PATCH 7/8] Drop class Result; TestClassInfo records only the termination signal The result of a class is already fully described by its test records and TestResult; computing it again at the class level duplicated that logic. class_end(e=None) now only stores the terminating signal. --- mobly/base_test.py | 4 +- mobly/records.py | 30 ++--------- tests/mobly/base_test_test.py | 17 ++++++- tests/mobly/records_test.py | 88 +++++---------------------------- tests/mobly/test_runner_test.py | 4 -- 5 files changed, 33 insertions(+), 110 deletions(-) diff --git a/mobly/base_test.py b/mobly/base_test.py index 45850aa5..cd56f37f 100644 --- a/mobly/base_test.py +++ b/mobly/base_test.py @@ -1164,7 +1164,7 @@ def run(self, test_names=None): class_info.class_begin() # Executes pre-setup procedures, like generating test methods. if not self._pre_run(): - class_info.class_end(self.results) + class_info.class_end() self.summary_writer.dump( class_info.to_dict(), records.TestSummaryEntryType.TEST_CLASS_INFO ) @@ -1266,7 +1266,7 @@ def run(self, test_names=None): class_termination_error = e raise finally: - class_info.class_end(self.results, class_termination_error) + class_info.class_end(class_termination_error) self.summary_writer.dump( class_info.to_dict(), records.TestSummaryEntryType.TEST_CLASS_INFO ) diff --git a/mobly/records.py b/mobly/records.py index 1c8ba17d..e3a41a7f 100644 --- a/mobly/records.py +++ b/mobly/records.py @@ -227,50 +227,29 @@ class TestClassInfoRecord: Captures the wall-clock span of the whole class, including `setup_class` and `teardown_class`, which is not derivable from the individual test - records, the overall result of the class, and the signal that terminated - the class early, if any. Times are epoch milliseconds, consistent with - `TestResultRecord`. + records, and the signal that terminated the class early, if any. Times are + epoch milliseconds, consistent with `TestResultRecord`. """ def __init__(self, test_class): self.test_class = test_class self.begin_time = None self.end_time = None - self.result = None self.termination_signal = None def class_begin(self): """Marks the beginning of the test class execution.""" self.begin_time = utils.get_current_epoch_time() - def class_end(self, results, e=None): + def class_end(self, e=None): """Marks the end of the test class execution. Args: - results: The `TestResult` of the class. - e: The exception or `TestSignal` that terminated the class early, if - any. Without one, the first class-stage error (`pre_run`, - `setup_class`, `teardown_class`) becomes the termination signal. + e: The exception or `TestSignal` that terminated the class early, if any. """ self.end_time = utils.get_current_epoch_time() if e is not None: self.termination_signal = ExceptionRecord(e) - else: - # Class-stage errors are the error records that are not executed tests. - for record in results.error: - if record not in results.executed: - self.termination_signal = record.termination_signal - break - if not results.is_all_pass: - self.result = ( - TestResultEnums.TEST_RESULT_ERROR - if results.error - else TestResultEnums.TEST_RESULT_FAIL - ) - elif results.passed or not results.skipped: - self.result = TestResultEnums.TEST_RESULT_PASS - else: - self.result = TestResultEnums.TEST_RESULT_SKIP def to_dict(self): signal = self.termination_signal @@ -278,7 +257,6 @@ def to_dict(self): TestResultEnums.RECORD_CLASS: self.test_class, TestResultEnums.RECORD_BEGIN_TIME: self.begin_time, TestResultEnums.RECORD_END_TIME: self.end_time, - TestResultEnums.RECORD_RESULT: self.result, TestResultEnums.RECORD_DETAILS: signal.details if signal else None, TestResultEnums.RECORD_TERMINATION_SIGNAL_TYPE: ( signal.type if signal else None diff --git a/tests/mobly/base_test_test.py b/tests/mobly/base_test_test.py index a976eb96..3a28fb36 100755 --- a/tests/mobly/base_test_test.py +++ b/tests/mobly/base_test_test.py @@ -1149,12 +1149,27 @@ def test_3(self): never_call() bt_cls = MockBaseTest(self.mock_test_cls_configs) - bt_cls.run(test_names=['test_1', 'test_2', 'test_3']) + with mock.patch.object(bt_cls.summary_writer, 'dump') as mock_dump: + bt_cls.run(test_names=['test_1', 'test_2', 'test_3']) self.assertEqual(len(bt_cls.results.skipped), 3) self.assertEqual( bt_cls.results.summary_str(), 'Error 0, Executed 0, Failed 0, Passed 0, Requested 3, Skipped 3', ) + # The abort signal is recorded as the class's termination signal. + class_info = next( + call.args[0] + for call in mock_dump.call_args_list + if call.args[1] == records.TestSummaryEntryType.TEST_CLASS_INFO + ) + self.assertEqual( + class_info[records.TestResultEnums.RECORD_TERMINATION_SIGNAL_TYPE], + 'TestAbortClass', + ) + self.assertIn( + MSG_EXPECTED_EXCEPTION, + class_info[records.TestResultEnums.RECORD_DETAILS], + ) def test_abort_class_in_setup_test(self): class MockBaseTest(base_test.BaseTestClass): diff --git a/tests/mobly/records_test.py b/tests/mobly/records_test.py index 22dcecc1..54dc5930 100755 --- a/tests/mobly/records_test.py +++ b/tests/mobly/records_test.py @@ -567,7 +567,6 @@ def test_test_class_info_record(self): records.TestResultEnums.RECORD_CLASS: 'SomeTestClass', records.TestResultEnums.RECORD_BEGIN_TIME: None, records.TestResultEnums.RECORD_END_TIME: None, - records.TestResultEnums.RECORD_RESULT: None, records.TestResultEnums.RECORD_DETAILS: None, records.TestResultEnums.RECORD_TERMINATION_SIGNAL_TYPE: None, records.TestResultEnums.RECORD_STACKTRACE: None, @@ -575,14 +574,10 @@ def test_test_class_info_record(self): }, ) record.class_begin() - record.class_end(records.TestResult()) + record.class_end() d = record.to_dict() self.assertIsInstance(d[records.TestResultEnums.RECORD_BEGIN_TIME], int) self.assertIsInstance(d[records.TestResultEnums.RECORD_END_TIME], int) - self.assertEqual( - d[records.TestResultEnums.RECORD_RESULT], - records.TestResultEnums.TEST_RESULT_PASS, - ) self.assertLessEqual( d[records.TestResultEnums.RECORD_BEGIN_TIME], d[records.TestResultEnums.RECORD_END_TIME], @@ -590,83 +585,22 @@ def test_test_class_info_record(self): # Must be serializable by the summary writer. yaml.safe_dump(d) - def test_test_class_info_record_with_results_and_signals(self): - # 1. Class with a setup_class error surfaces the class error details. - tr = records.TestResult() - tr.requested = ['test_a', 'test_b'] - cls_err = records.TestResultRecord('setup_class', 'SomeTestClass') - cls_err.test_begin() - cls_err.test_error(RuntimeError('setup boom')) - tr.add_class_error(cls_err) - skip_rec = records.TestResultRecord('test_a', 'SomeTestClass') - skip_rec.test_skip(signals.TestSkip('skipped')) - tr.add_record(skip_rec) - - info = records.TestClassInfoRecord('SomeTestClass') - info.class_begin() - info.class_end(results=tr) - d = info.to_dict() - self.assertEqual( - d[records.TestResultEnums.RECORD_RESULT], - records.TestResultEnums.TEST_RESULT_ERROR, - ) - self.assertEqual(d[records.TestResultEnums.RECORD_DETAILS], 'setup boom') - self.assertEqual( - d[records.TestResultEnums.RECORD_TERMINATION_SIGNAL_TYPE], - 'RuntimeError', - ) - - # 2. Class cleanly aborted via TestAbortClass with 0 passed tests -> SKIP. - tr_abort = records.TestResult() - tr_abort.requested = ['test_a'] - tr_abort.add_record(skip_rec) - info_abort = records.TestClassInfoRecord('SomeTestClass') - info_abort.class_begin() - info_abort.class_end( - results=tr_abort, - e=signals.TestAbortClass('device not supported', extras={'k': 'v'}), - ) - d_abort = info_abort.to_dict() - self.assertEqual( - d_abort[records.TestResultEnums.RECORD_RESULT], - records.TestResultEnums.TEST_RESULT_SKIP, + def test_test_class_info_record_with_termination_signal(self): + record = records.TestClassInfoRecord('SomeTestClass') + record.class_begin() + record.class_end( + signals.TestAbortClass('device not supported', extras={'k': 'v'}) ) + d = record.to_dict() self.assertEqual( - d_abort[records.TestResultEnums.RECORD_DETAILS], 'device not supported' + d[records.TestResultEnums.RECORD_DETAILS], 'device not supported' ) self.assertEqual( - d_abort[records.TestResultEnums.RECORD_TERMINATION_SIGNAL_TYPE], + d[records.TestResultEnums.RECORD_TERMINATION_SIGNAL_TYPE], 'TestAbortClass', ) - self.assertEqual(d_abort[records.TestResultEnums.RECORD_EXTRAS], {'k': 'v'}) - - # 3. Class with a failed test -> FAIL. - tr_fail = records.TestResult() - tr_fail.requested = ['test_a'] - fail_rec = records.TestResultRecord('test_a', 'SomeTestClass') - fail_rec.test_begin() - fail_rec.test_fail(signals.TestFailure('assertion failed')) - tr_fail.add_record(fail_rec) - info_fail = records.TestClassInfoRecord('SomeTestClass') - info_fail.class_begin() - info_fail.class_end(results=tr_fail) - self.assertEqual( - info_fail.to_dict()[records.TestResultEnums.RECORD_RESULT], - records.TestResultEnums.TEST_RESULT_FAIL, - ) - - # 4. An exception terminating the class is recorded as the signal. - info_exc = records.TestClassInfoRecord('SomeTestClass') - info_exc.class_begin() - info_exc.class_end(results=records.TestResult(), e=ValueError('bad repeat')) - d_exc = info_exc.to_dict() - self.assertEqual( - d_exc[records.TestResultEnums.RECORD_DETAILS], 'bad repeat' - ) - self.assertEqual( - d_exc[records.TestResultEnums.RECORD_TERMINATION_SIGNAL_TYPE], - 'ValueError', - ) + self.assertEqual(d[records.TestResultEnums.RECORD_EXTRAS], {'k': 'v'}) + yaml.safe_dump(d) if __name__ == '__main__': diff --git a/tests/mobly/test_runner_test.py b/tests/mobly/test_runner_test.py index 597320e2..93d55598 100755 --- a/tests/mobly/test_runner_test.py +++ b/tests/mobly/test_runner_test.py @@ -160,10 +160,6 @@ def test_summary_file_entries(self): self.assertIsInstance( summary_entries[3][records.TestResultEnums.RECORD_END_TIME], int ) - self.assertEqual( - summary_entries[3][records.TestResultEnums.RECORD_RESULT], - records.TestResultEnums.TEST_RESULT_PASS, - ) self.assertEqual( summary_entries[4]['Type'], records.TestSummaryEntryType.SUMMARY.value ) From 961e801c804c359725fba8841a4d9fcc957171cb Mon Sep 17 00:00:00 2001 From: Ang Li Date: Thu, 15 Oct 2026 23:27:22 +0000 Subject: [PATCH 8/8] Trim tests: decorator-style dump mock, drop redundant assertions --- tests/mobly/base_test_test.py | 14 +++----------- tests/mobly/records_test.py | 1 - 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/tests/mobly/base_test_test.py b/tests/mobly/base_test_test.py index 3a28fb36..a2a6efdb 100755 --- a/tests/mobly/base_test_test.py +++ b/tests/mobly/base_test_test.py @@ -1126,7 +1126,8 @@ def test_something(self): ) self.assertEqual(bt_cls.results.summary_str(), expected_summary) - def test_abort_class_setup_class(self): + @mock.patch('mobly.records.TestSummaryWriter.dump') + def test_abort_class_setup_class(self, mock_dump): """A class was intentionally aborted by the test. This is not considered an error as the abort class is used as a skip @@ -1149,8 +1150,7 @@ def test_3(self): never_call() bt_cls = MockBaseTest(self.mock_test_cls_configs) - with mock.patch.object(bt_cls.summary_writer, 'dump') as mock_dump: - bt_cls.run(test_names=['test_1', 'test_2', 'test_3']) + bt_cls.run(test_names=['test_1', 'test_2', 'test_3']) self.assertEqual(len(bt_cls.results.skipped), 3) self.assertEqual( bt_cls.results.summary_str(), @@ -1166,10 +1166,6 @@ def test_3(self): class_info[records.TestResultEnums.RECORD_TERMINATION_SIGNAL_TYPE], 'TestAbortClass', ) - self.assertIn( - MSG_EXPECTED_EXCEPTION, - class_info[records.TestResultEnums.RECORD_DETAILS], - ) def test_abort_class_in_setup_test(self): class MockBaseTest(base_test.BaseTestClass): @@ -2127,9 +2123,6 @@ def pre_run(self): def setup_class(self): never_call() - def test_func(self): - never_call() - bt_cls = MockBaseTest(self.mock_test_cls_configs) bt_cls.run() must_call.assert_called_once_with('ha') @@ -2137,7 +2130,6 @@ def test_func(self): self.assertEqual(actual_record.test_name, 'pre_run') self.assertEqual(actual_record.details, MSG_EXPECTED_EXCEPTION) self.assertEqual(actual_record.extras, MOCK_EXTRA) - self.assertIsNotNone(actual_record.end_time) def test_expect_in_setup_test(self): must_call = mock.Mock() diff --git a/tests/mobly/records_test.py b/tests/mobly/records_test.py index 54dc5930..f246b18a 100755 --- a/tests/mobly/records_test.py +++ b/tests/mobly/records_test.py @@ -600,7 +600,6 @@ def test_test_class_info_record_with_termination_signal(self): 'TestAbortClass', ) self.assertEqual(d[records.TestResultEnums.RECORD_EXTRAS], {'k': 'v'}) - yaml.safe_dump(d) if __name__ == '__main__':