`setup_class` error record is not written to `test_summary.yaml` when `on_fail` raises `TestAbortAll`
- 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
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