google / google/mobly

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

Open
#1,029 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
752
Forks
222
Avg merge
14d 18h
Merged PRs (30d)
1

Description

## 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

```python
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:

```python
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

- **#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`.
- **#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:

```python
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).

Contributor guide

Open the contributing guide

Research direction

Start in mobly/base_test.py at BaseTestClass._setup_class and compare its error branches with exec_one_test. Ensure the setup_class record is updated and dumped even when on_fail raises TestAbortAll, then run or extend test_abort_all_in_on_fail_from_setup_class to verify summary_writer receives a setup_class ERROR record.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
testing
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
82/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.