Merge upstream into main Merge remote-tracking branch 'origin/upstream/master' into main Test: fx build Change-Id: Iaa1603afd803c27da96f403c2153e0c260354170 Reviewed-on: https://fuchsia-review.googlesource.com/c/third_party/github.com/google/mobly/+/1676374 Reviewed-by: Christopher Johnson <crjohns@google.com> Reviewed-by: Prashanth Swaminathan <prashanthsw@google.com>
diff --git a/docs/tutorial_custom_controller.md b/docs/tutorial_custom_controller.md index 10a7255..e6b193b 100644 --- a/docs/tutorial_custom_controller.md +++ b/docs/tutorial_custom_controller.md
@@ -34,7 +34,7 @@ class SmartLight: """A class representing a smart light device. - + Attributes: name: the name of the device. ip: the IP address of the device. @@ -81,7 +81,7 @@ raise ValueError( f"Invalid config: {config}. 'name' and 'ip' are required." ) - + devices.append(SmartLight( name=config["name"], ip=config["ip"] @@ -91,7 +91,7 @@ def destroy(objects: List[SmartLight]) -> None: """Cleans up SmartLight instances. - + Args: objects: A list of SmartLight objects to be destroyed. """ @@ -136,7 +136,7 @@ def test_turn_on(self): light = self.lights[0] light.power_on() - + # Verify the light is on if not light.is_on: raise signals.TestFailure(f"Light {light.name} should be on!")
diff --git a/mobly/base_test.py b/mobly/base_test.py index a62fac2..e4ac9a0 100644 --- a/mobly/base_test.py +++ b/mobly/base_test.py
@@ -373,7 +373,7 @@ return True except Exception as e: logging.exception('%s failed for %s.', stage_name, self.TAG) - record.test_error(e) + record.record_exception_chain(e) self.results.add_class_error(record) self.summary_writer.dump( record.to_dict(), records.TestSummaryEntryType.RECORD @@ -416,7 +416,7 @@ # Setup class failed for unknown reasons. # Fail the class and skip all tests. logging.exception('Error in %s#setup_class.', self.TAG) - class_record.test_error(e) + class_record.record_exception_chain(e) self.results.add_class_error(class_record) self._exec_procedure_func(self._on_fail, class_record) class_record.update_record() @@ -466,7 +466,7 @@ raise except Exception as e: logging.exception('Error encountered in %s.', stage_name) - record.test_error(e) + record.record_exception_chain(e) record.update_record() self.results.add_class_error(record) self.summary_writer.dump( @@ -827,20 +827,20 @@ tr_record.test_error() teardown_test_failed = True except (signals.TestFailure, AssertionError) as e: - tr_record.test_fail(e) + tr_record.record_exception_chain(e) except signals.TestSkip as e: # Test skipped. - tr_record.test_skip(e) + tr_record.record_exception_chain(e) except signals.TestAbortSignal as e: # Abort signals, pass along. - tr_record.test_fail(e) + tr_record.record_exception_chain(e) raise except signals.TestPass as e: # Explicit test pass. - tr_record.test_pass(e) + tr_record.record_exception_chain(e) except Exception as e: # Exception happened during test. - tr_record.test_error(e) + tr_record.record_exception_chain(e) else: # No exception is thrown from test and teardown, if `expects` has # error, the test should fail with the first error in `expects`.
diff --git a/mobly/controllers/android_device.py b/mobly/controllers/android_device.py index eea6d48..c99b160 100644 --- a/mobly/controllers/android_device.py +++ b/mobly/controllers/android_device.py
@@ -522,7 +522,7 @@ device. """ - def __init__(self, serial=''): + def __init__(self, serial='', host='', port=None): self._serial = str(serial) # logging.log_path only exists when this is used in an Mobly test run. _log_path_base = utils.abs_path(getattr(logging, 'log_path', '/tmp/logs')) @@ -535,7 +535,7 @@ ) self._build_info = None self._is_rebooting = False - self.adb = adb.AdbProxy(serial) + self.adb = adb.AdbProxy(serial, host, port) self.fastboot = fastboot.FastbootProxy(serial) if self.is_rootable: self.root_adb() @@ -840,7 +840,7 @@ @property def is_rootable(self): - return self.is_adb_detectable() and self.build_info['debuggable'] == '1' + return False @functools.cached_property def model(self):
diff --git a/mobly/controllers/android_device_lib/adb.py b/mobly/controllers/android_device_lib/adb.py index 99d9e3e..4934d84 100644 --- a/mobly/controllers/android_device_lib/adb.py +++ b/mobly/controllers/android_device_lib/adb.py
@@ -166,8 +166,10 @@ >> adb.shell('cat /foo > /tmp/file', shell=True) """ - def __init__(self, serial=''): + def __init__(self, serial='', host='', port=None): self.serial = serial + self.host = host + self.port = port def _exec_cmd(self, args, shell, timeout, stderr) -> bytes: """Executes adb commands. @@ -282,13 +284,17 @@ args = utils.cli_cmd_to_string(args) # Add quotes around "adb" in case the ADB path contains spaces. This # is pretty common on Windows (e.g. Program Files). - if self.serial: + if self.serial and self.host and self.port: + adb_cmd = '"%s" -s "%s" -H "%s" -P "%s" %s %s' % (ADB, self.serial, self.host, str(self.port), name, args) + elif self.serial: adb_cmd = '"%s" -s "%s" %s %s' % (ADB, self.serial, name, args) else: adb_cmd = '"%s" %s %s' % (ADB, name, args) else: adb_cmd = [ADB] - if self.serial: + if self.serial and self.host and self.port: + adb_cmd.extend(['-s', self.serial, '-H', self.host, '-P', str(self.port)]) + elif self.serial: adb_cmd.extend(['-s', self.serial]) adb_cmd.append(name) if args:
diff --git a/mobly/controllers/android_device_lib/snippet_client_v2.py b/mobly/controllers/android_device_lib/snippet_client_v2.py index 3382609..1b9c9ea 100644 --- a/mobly/controllers/android_device_lib/snippet_client_v2.py +++ b/mobly/controllers/android_device_lib/snippet_client_v2.py
@@ -17,7 +17,9 @@ import enum import json import re +import selectors import socket +import time from typing import Dict, Union from mobly import utils @@ -94,7 +96,9 @@ _SOCKET_CONNECTION_TIMEOUT = 60 # Maximum time to wait for a response message on the socket. -_SOCKET_READ_TIMEOUT = 60 * 10 +# Limit the time to under 6 minutes, the standard I/O timeout on Fuchsia +# infrastructure bots. +_SOCKET_READ_TIMEOUT = 60 * 10 - 10 # The default timeout for callback handlers returned by this client _CALLBACK_DEFAULT_TIMEOUT_SEC = 60 * 2 @@ -336,7 +340,9 @@ def _run_adb_cmd(self, cmd): """Starts a long-running adb subprocess and returns it immediately.""" adb_cmd = [adb.ADB] - if self._adb.serial: + if self._adb.serial and self._adb.host and self._adb.port: + adb_cmd += ['-s', self._adb.serial, '-H', self._adb.host, '-P', str(self._adb.port)] + elif self._adb.serial: adb_cmd += ['-s', self._adb.serial] adb_cmd += ['shell', cmd] return utils.start_standing_subprocess(adb_cmd, shell=False) @@ -388,7 +394,7 @@ return '' return f'--user {self.user_id}' - def _read_protocol_line(self): + def _read_protocol_line(self, timeout=30): """Reads the next line of instrumentation output relevant to snippets. This method will skip over lines that don't start with 'SNIPPET ' or @@ -403,26 +409,46 @@ being read. """ self._server_start_stdout = [] - while True: - line = self._proc.stdout.readline().decode('utf-8') - if not line: - raise errors.ServerStartError( - self._device, 'Unexpected EOF when waiting for server to start.' - ) + ACCEPTED_PREFIXES = ('INSTRUMENTATION_RESULT:', 'SNIPPET ') + start_time = time.time() - # readline() uses an empty string to mark EOF, and a single newline - # to mark regular empty lines in the output. Don't move the strip() - # call above the truthiness check, or this method will start - # considering any blank output line to be EOF. - line = line.strip() - if line.startswith('INSTRUMENTATION_RESULT:') or line.startswith( - 'SNIPPET ' - ): - self.log.debug('Accepted line from instrumentation output: "%s"', line) - return line + with selectors.DefaultSelector() as selector: + selector.register(self._proc.stdout, selectors.EVENT_READ) - self._server_start_stdout.append(line) - self.log.debug('Discarded line from instrumentation output: "%s"', line) + while True: + elapsed = time.time() - start_time + remaining_time = timeout - elapsed + if remaining_time <= 0: + raise errors.ServerStartError( + self._device, f'Timeout ({timeout}s) waiting for protocol line.' + ) + + # Wait for the stream to be ready to read, with a timeout. + ready = selector.select(timeout=remaining_time) + if not ready: + # This is the case where selector.select() times out. + raise errors.ServerStartError( + self._device, f'Timeout ({timeout}s) waiting for protocol line.' + ) + + # Data is ready, so readline() will not block indefinitely. + line_bytes = self._proc.stdout.readline() + if not line_bytes: + raise errors.ServerStartError( + self._device, 'Unexpected EOF when waiting for server to start.' + ) + + # readline() uses an empty string to mark EOF, and a single newline + # to mark regular empty lines in the output. Don't move the strip() + # call above the truthiness check, or this method will start + # considering any blank output line to be EOF. + line = line_bytes.decode('utf-8').strip() + if line.startswith(ACCEPTED_PREFIXES): + self.log.debug('Accepted line from instrumentation output: "%s"', line) + return line + + self._server_start_stdout.append(line) + self.log.debug('Discarded line from instrumentation output: "%s"', line) def make_connection(self): """Makes a connection to the snippet server on the remote device.
diff --git a/mobly/records.py b/mobly/records.py index 10d3063..28b805e 100644 --- a/mobly/records.py +++ b/mobly/records.py
@@ -459,6 +459,67 @@ """ self._test_end(TestResultEnums.TEST_RESULT_ERROR, e) + def record_exception_chain(self, exc): + """Unrolls context/cause exceptions and updates the test result record. + + Args: + exc: An exception object. + """ + seen = set() + chain = [] + + def _traverse(e): + """Recursively explores both __context__ and __cause__ branches to collect + + the full ancestry graph, capturing all the exceptions in chain[] from + throughout the tree and returning the deepest main path root cause. + """ + if e is None or id(e) in seen: + return e + seen.add(id(e)) + chain.append(e) + + ctx = getattr(e, '__context__', None) + cause = getattr(e, '__cause__', None) + + main_ancestor = ctx or cause + side_ancestor = cause if (ctx and cause and ctx is not cause) else None + + if side_ancestor: + _traverse(side_ancestor) + + if main_ancestor: + return _traverse(main_ancestor) + + return e + + primary_exc = exc + + # We only unroll to the root cause if the outer exception is an abort signal. + # Otherwise (e.g. for standard test assertions), we record the outer exception. + if isinstance(exc, signals.TestAbortSignal): + # Traverse the full exception tree to extract the true root cause. + primary_exc = _traverse(exc) + + # Register intermediate aborts as extra errors + for intermediate_exc in chain: + if intermediate_exc is not primary_exc: + position = f'context_error_{type(intermediate_exc).__name__}' + if position not in self.extra_errors: + self.add_error(position, intermediate_exc) + + # Record the primary failure + if isinstance(primary_exc, (signals.TestFailure, AssertionError)): + self.test_fail(primary_exc) + elif isinstance(primary_exc, signals.TestSkip): + self.test_skip(primary_exc) + elif isinstance(primary_exc, signals.TestPass): + self.test_pass(primary_exc) + elif isinstance(primary_exc, signals.TestAbortSignal): + self.test_fail(primary_exc) + else: + self.test_error(primary_exc) + def add_error(self, position, e): """Add extra error happened during a test.
diff --git a/mobly/test_runner.py b/mobly/test_runner.py index b32f5b0..6b4a2bc 100644 --- a/mobly/test_runner.py +++ b/mobly/test_runner.py
@@ -32,6 +32,9 @@ pass +EXIT_ABORT_ALL = 40 + + def main(argv=None): """Execute the test class in a test module. @@ -79,7 +82,7 @@ runner.run() ok = runner.results.is_all_pass and ok except signals.TestAbortAll: - pass + sys.exit(EXIT_ABORT_ALL) # Exit now, without running any more tests. except Exception: logging.exception('Exception when executing %s.', config.testbed_name) ok = False
diff --git a/mobly/utils.py b/mobly/utils.py index 96ae94a..0bea52d 100644 --- a/mobly/utils.py +++ b/mobly/utils.py
@@ -281,10 +281,9 @@ 'pid', '--ppid', str(pid), - '--noheaders', ] try: - ps_results = subprocess.check_output(command).decode().strip() + ps_results = subprocess.check_output(command).decode().strip().split('\n', 1)[1] except subprocess.CalledProcessError: # Ignore if there is not child process. continue
diff --git a/tests/mobly/base_test_test.py b/tests/mobly/base_test_test.py index f389133..81d3f7e 100755 --- a/tests/mobly/base_test_test.py +++ b/tests/mobly/base_test_test.py
@@ -1375,6 +1375,105 @@ 'Error 1, Executed 0, Failed 0, Passed 0, Requested 3, Skipped 3', ) + def test_exception_chain_unrolled_in_test_execution(self): + class MockBaseTest(base_test.BaseTestClass): + + def test_1(self): + try: + asserts.fail('root failure') + except signals.TestFailure as e: + raise signals.TestAbortClass('aborting test') from e + + def test_2(self): + never_call() + + bt_cls = MockBaseTest(self.mock_test_cls_configs) + bt_cls.run(test_names=['test_1', 'test_2']) + + test_1_record = bt_cls.results.failed[0] + self.assertEqual(test_1_record.test_name, 'test_1') + self.assertEqual(test_1_record.result, records.TestResultEnums.TEST_RESULT_FAIL) + # The primary exception should be the root cause TestFailure. + self.assertEqual(test_1_record.termination_signal_type, 'TestFailure') + self.assertEqual(test_1_record.details, 'root failure') + # The intermediate TestAbortClass should be recorded as an extra error. + self.assertIn('context_error_TestAbortClass', test_1_record.extra_errors) + self.assertEqual( + test_1_record.extra_errors['context_error_TestAbortClass'].type, + 'TestAbortClass', + ) + + # test_2 should be skipped due to the class abort. + test_2_record = bt_cls.results.skipped[0] + self.assertEqual(test_2_record.test_name, 'test_2') + self.assertEqual(test_2_record.result, records.TestResultEnums.TEST_RESULT_SKIP) + self.assertEqual(test_2_record.termination_signal_type, 'TestAbortClass') + self.assertEqual(test_2_record.details, 'Test class aborted due to: aborting test') + + def test_exception_chain_unrolled_in_setup_class(self): + class MockBaseTest(base_test.BaseTestClass): + + def setup_class(self): + try: + raise ValueError('root exception') + except ValueError as e: + raise signals.TestAbortClass('aborting class') from e + + def test_1(self): + never_call() + + bt_cls = MockBaseTest(self.mock_test_cls_configs) + bt_cls.run(test_names=['test_1']) + + # Since setup_class raised TestAbortClass, there is no class error recorded. + self.assertEqual(len(bt_cls.results.error), 0) + + # The requested test should be skipped with primary exception as root cause ValueError. + self.assertEqual(len(bt_cls.results.skipped), 1) + test_1_record = bt_cls.results.skipped[0] + self.assertEqual(test_1_record.test_name, 'test_1') + self.assertEqual(test_1_record.result, records.TestResultEnums.TEST_RESULT_SKIP) + self.assertEqual(test_1_record.termination_signal_type, 'TestAbortClass') + self.assertEqual(test_1_record.details, 'Test class aborted due to: aborting class') + + def test_exception_chain_in_pre_run(self): + class MockBaseTest(base_test.BaseTestClass): + + def pre_run(self): + raise ValueError('pre_run error') + + def test_1(self): + never_call() + + bt_cls = MockBaseTest(self.mock_test_cls_configs) + bt_cls.run(test_names=['test_1']) + + self.assertEqual(len(bt_cls.results.error), 1) + pre_run_record = bt_cls.results.error[0] + self.assertEqual(pre_run_record.test_name, 'pre_run') + self.assertEqual(pre_run_record.result, records.TestResultEnums.TEST_RESULT_ERROR) + self.assertEqual(pre_run_record.termination_signal_type, 'ValueError') + self.assertEqual(pre_run_record.details, 'pre_run error') + + def test_exception_chain_in_teardown_class(self): + class MockBaseTest(base_test.BaseTestClass): + + def test_1(self): + pass + + def teardown_class(self): + raise ValueError('teardown_class error') + + bt_cls = MockBaseTest(self.mock_test_cls_configs) + bt_cls.run(test_names=['test_1']) + + self.assertEqual(len(bt_cls.results.error), 1) + teardown_class_record = bt_cls.results.error[0] + self.assertEqual(teardown_class_record.test_name, 'teardown_class') + self.assertEqual(teardown_class_record.result, records.TestResultEnums.TEST_RESULT_ERROR) + self.assertEqual(teardown_class_record.termination_signal_type, 'ValueError') + self.assertEqual(teardown_class_record.details, 'teardown_class error') + def test_abort_all_in_test(self): class MockBaseTest(base_test.BaseTestClass):
diff --git a/tests/mobly/records_test.py b/tests/mobly/records_test.py index 5c1d027..ab4edd1 100755 --- a/tests/mobly/records_test.py +++ b/tests/mobly/records_test.py
@@ -542,6 +542,146 @@ self.assertEqual(test_uid_helper.uid, 'some-uuid') + def test_record_exception_chain_no_chain(self): + record = records.TestResultRecord(self.tn) + record.test_begin() + exc = ValueError('error') + record.record_exception_chain(exc) + self.assertEqual(record.result, records.TestResultEnums.TEST_RESULT_ERROR) + self.assertEqual(record.termination_signal.details, 'error') + + def test_record_exception_chain_failure_no_chain(self): + record = records.TestResultRecord(self.tn) + record.test_begin() + exc = signals.TestFailure('fail') + record.record_exception_chain(exc) + self.assertEqual(record.result, records.TestResultEnums.TEST_RESULT_FAIL) + self.assertEqual(record.termination_signal.details, 'fail') + + def test_record_exception_chain_standard_with_context(self): + record = records.TestResultRecord(self.tn) + record.test_begin() + try: + try: + raise KeyError('inner') + except KeyError as e: + raise ValueError('outer') from e + except ValueError as e: + record.record_exception_chain(e) + # The outer ValueError is recorded since it's not a TestAbortSignal. + self.assertEqual(record.result, records.TestResultEnums.TEST_RESULT_ERROR) + self.assertEqual(record.termination_signal.type, 'ValueError') + self.assertEqual(record.termination_signal.details, 'outer') + self.assertEqual(len(record.extra_errors), 0) + + def test_record_exception_chain_abort_with_context(self): + record = records.TestResultRecord(self.tn) + record.test_begin() + try: + try: + raise AssertionError('root assertion') + except AssertionError as e: + raise signals.TestAbortClass('abort class') from e + except signals.TestAbortClass as e: + record.record_exception_chain(e) + # The AssertionError is recorded as primary exc, and intermediate abort is recorded as extra error. + self.assertEqual(record.result, records.TestResultEnums.TEST_RESULT_FAIL) + self.assertEqual(record.termination_signal.type, 'AssertionError') + self.assertEqual(record.termination_signal.details, 'root assertion') + self.assertIn('context_error_TestAbortClass', record.extra_errors) + self.assertEqual( + record.extra_errors['context_error_TestAbortClass'].type, + 'TestAbortClass', + ) + + def test_record_exception_chain_multiple_intermediate_exceptions(self): + record = records.TestResultRecord(self.tn) + record.test_begin() + try: + try: + try: + raise signals.TestFailure('root failure') + except signals.TestFailure as e1: + raise signals.TestAbortClass('intermediate abort') from e1 + except signals.TestAbortClass as e2: + raise signals.TestAbortAll('outer abort') from e2 + except signals.TestAbortAll as e: + record.record_exception_chain(e) + + self.assertEqual(record.result, records.TestResultEnums.TEST_RESULT_FAIL) + self.assertEqual(record.termination_signal.type, 'TestFailure') + self.assertEqual(record.termination_signal.details, 'root failure') + # Both intermediate exceptions in chain[:-1] should be collected in extra_errors. + self.assertIn('context_error_TestAbortAll', record.extra_errors) + self.assertEqual( + record.extra_errors['context_error_TestAbortAll'].type, 'TestAbortAll' + ) + self.assertIn('context_error_TestAbortClass', record.extra_errors) + self.assertEqual( + record.extra_errors['context_error_TestAbortClass'].type, + 'TestAbortClass', + ) + + def test_record_exception_chain_full_tree_traversal(self): + record = records.TestResultRecord(self.tn) + record.test_begin() + # Construct a tree where an exception populates both __context__ and __cause__ + root_main = signals.TestFailure('root main') + side_exc = ValueError('side exception') + + mid_abort = signals.TestAbortClass('mid abort') + mid_abort.__context__ = root_main + mid_abort.__cause__ = side_exc + + outer_abort = signals.TestAbortAll('outer abort') + outer_abort.__context__ = mid_abort + + record.record_exception_chain(outer_abort) + + self.assertEqual(record.result, records.TestResultEnums.TEST_RESULT_FAIL) + self.assertEqual(record.termination_signal.type, 'TestFailure') + self.assertEqual(record.termination_signal.details, 'root main') + # Confirm both main and side ancestors were explored and collected. + self.assertIn('context_error_TestAbortAll', record.extra_errors) + self.assertIn('context_error_TestAbortClass', record.extra_errors) + self.assertIn('context_error_ValueError', record.extra_errors) + + def test_record_exception_chain_with_skip(self): + record = records.TestResultRecord(self.tn) + record.test_begin() + exc = signals.TestSkip('skip reason') + record.record_exception_chain(exc) + self.assertEqual(record.result, records.TestResultEnums.TEST_RESULT_SKIP) + self.assertEqual(record.termination_signal.details, 'skip reason') + + def test_record_exception_chain_with_pass(self): + record = records.TestResultRecord(self.tn) + record.test_begin() + exc = signals.TestPass('pass reason') + record.record_exception_chain(exc) + self.assertEqual(record.result, records.TestResultEnums.TEST_RESULT_PASS) + self.assertEqual(record.termination_signal.details, 'pass reason') + + def test_record_exception_chain_with_standalone_abort(self): + record = records.TestResultRecord(self.tn) + record.test_begin() + exc = signals.TestAbortClass('abort class') + record.record_exception_chain(exc) + self.assertEqual(record.result, records.TestResultEnums.TEST_RESULT_FAIL) + self.assertEqual(record.termination_signal.details, 'abort class') + + def test_record_exception_chain_loop_detection(self): + record = records.TestResultRecord(self.tn) + record.test_begin() + exc1 = ValueError('one') + exc2 = ValueError('two') + exc1.__context__ = exc2 + exc2.__context__ = exc1 + record.record_exception_chain(exc1) + # Should not infinite loop and should terminate. + self.assertEqual(record.result, records.TestResultEnums.TEST_RESULT_ERROR) + self.assertEqual(record.termination_signal.details, 'one') + if __name__ == '__main__': unittest.main()