From c8b630139b7969ae17fe0c43dc600c198d9010e7 Mon Sep 17 00:00:00 2001 From: Douglas Raillard Date: Thu, 10 Aug 2023 11:25:14 +0100 Subject: [PATCH 1/2] Squashed 'external/devlib/' changes from be988bb42..1730f6946 1730f6946 target: Avoid intermittent error when installing binary cf4d3b5f4 collector/dmesg: Avoid unnecessary dmesg command eb2c7e488 devlib/utils/serial_port: Avoid use of deprecated disutils 306fd0624 devlib/utils/ssh: Avoid using deprecated distutils fe28e086c devlib/host: Remove use of deprecated distutils 59ff6100d utils.rendering: Fix activity matching git-subtree-dir: external/devlib git-subtree-split: 1730f69461cc758d016508d9d0f0db693cb355e4 --- devlib/collector/dmesg.py | 41 +++++++++++++++++++------------------ devlib/host.py | 30 +++++++++++++++++++-------- devlib/target.py | 2 +- devlib/utils/rendering.py | 15 +++++++++++--- devlib/utils/serial_port.py | 11 ++++++---- devlib/utils/ssh.py | 10 +++++---- 6 files changed, 69 insertions(+), 40 deletions(-) diff --git a/devlib/collector/dmesg.py b/devlib/collector/dmesg.py index faf968251..72529f53c 100644 --- a/devlib/collector/dmesg.py +++ b/devlib/collector/dmesg.py @@ -234,14 +234,33 @@ class DmesgCollector(CollectorBase): if entry.timestamp > timestamp ] + def _get_output(self): + levels_list = list(takewhile( + lambda level: level != self.level, + self.LOG_LEVELS + )) + levels_list.append(self.level) + if self.basic_dmesg: + cmd = 'dmesg -r' + else: + cmd = 'dmesg --facility={facility} --force-prefix --decode --level={levels}'.format( + levels=','.join(levels_list), + facility=self.facility, + ) + + self._dmesg_out = self.target.execute(cmd, as_root=self.needs_root) + def reset(self): + self._dmesg_out = None + + def start(self): # If the buffer is emptied on start(), it does not matter as we will # not end up with entries dating from before start() if self.empty_buffer: # Empty the dmesg ring buffer. This requires root in all cases self.target.execute('dmesg -c', as_root=True) else: - self.stop() + self._get_output() try: entry = self.entries[-1] except IndexError: @@ -249,26 +268,8 @@ class DmesgCollector(CollectorBase): else: self._begin_timestamp = entry.timestamp - self._dmesg_out = None - - def start(self): - self.reset() - def stop(self): - levels_list = list(takewhile( - lambda level: level != self.level, - self.LOG_LEVELS - )) - levels_list.append(self.level) - if self.basic_dmesg: - cmd = 'dmesg -r' - else: - cmd = 'dmesg --facility={facility} --force-prefix --decode --level={levels}'.format( - levels=','.join(levels_list), - facility=self.facility, - ) - - self._dmesg_out = self.target.execute(cmd, as_root=self.needs_root) + self._get_output() def set_output(self, output_path): self.output_path = output_path diff --git a/devlib/host.py b/devlib/host.py index 68c535d39..ff8546c11 100644 --- a/devlib/host.py +++ b/devlib/host.py @@ -18,7 +18,7 @@ import signal import shutil import subprocess import logging -from distutils.dir_util import copy_tree +import sys from getpass import getpass from shlex import quote @@ -30,6 +30,26 @@ from devlib.utils.misc import check_output from devlib.connection import ConnectionBase, PopenBackgroundCommand +if sys.version_info >= (3, 8): + def copy_tree(src, dst): + from shutils import copy, copytree + copytree( + src, + dst, + # dirs_exist_ok=True only exists in Python >= 3.8 + dirs_exist_ok=True, + # Do not copy creation and modification time to behave like other + # targets. + copy_function=copy + ) +else: + def copy_tree(src, dst): + from distutils.dir_util import copy_tree + # Mirror the behavior of all other targets which only copy the + # content without metadata + copy_tree(src, dst, preserve_mode=False, preserve_times=False) + + PACKAGE_BIN_DIRECTORY = os.path.join(os.path.dirname(__file__), 'bin') @@ -71,13 +91,7 @@ class LocalConnection(ConnectionBase): def _copy_path(self, source, dest): self.logger.debug('copying {} to {}'.format(source, dest)) if os.path.isdir(source): - # Use distutils copy_tree since it behaves the same as - # shutils.copytree except that it won't fail if some folders - # already exist. - # - # Mirror the behavior of all other targets which only copy the - # content without metadata - copy_tree(source, dest, preserve_mode=False, preserve_times=False) + copy_tree(source, dest) else: shutil.copy(source, dest) diff --git a/devlib/target.py b/devlib/target.py index 811fc3b6e..8624aab79 100644 --- a/devlib/target.py +++ b/devlib/target.py @@ -2134,7 +2134,7 @@ class AndroidTarget(Target): on_device_executable = self.path.join(self.executables_directory, executable_name) await self.push.asyn(filepath, on_device_file, timeout=timeout) if on_device_file != on_device_executable: - await self.execute.asyn('cp {} {}'.format(quote(on_device_file), quote(on_device_executable)), + await self.execute.asyn('cp -f -- {} {}'.format(quote(on_device_file), quote(on_device_executable)), as_root=self.needs_su, timeout=timeout) await self.remove.asyn(on_device_file, as_root=self.needs_su) await self.execute.asyn("chmod 0777 {}".format(quote(on_device_executable)), as_root=self.needs_su) diff --git a/devlib/utils/rendering.py b/devlib/utils/rendering.py index e66dd8c98..3df82d61c 100644 --- a/devlib/utils/rendering.py +++ b/devlib/utils/rendering.py @@ -131,9 +131,18 @@ class SurfaceFlingerFrameCollector(FrameCollector): self.header = header or SurfaceFlingerFrame._fields def collect_frames(self, wfh): - for activity in self.list(): - if activity == self.view: - wfh.write(self.get_latencies(activity).encode('utf-8')) + activities = [a for a in self.list() if a.startswith(self.view)] + + if len(activities) > 1: + raise ValueError( + "More than one activity matching view '{}' was found: {}".format(self.view, activities) + ) + + if not activities: + logger.warning("No activities matching view '{}' were found".format(self.view)) + + for activity in activities: + wfh.write(self.get_latencies(activity).encode('utf-8')) def clear(self): self.target.execute('dumpsys SurfaceFlinger --latency-clear ') diff --git a/devlib/utils/serial_port.py b/devlib/utils/serial_port.py index d58d77fd9..e827e98f2 100644 --- a/devlib/utils/serial_port.py +++ b/devlib/utils/serial_port.py @@ -22,11 +22,14 @@ import serial # pylint: disable=import-error,wrong-import-position,ungrouped-imports,wrong-import-order import pexpect -from distutils.version import StrictVersion as V -if V(pexpect.__version__) < V('4.0.0'): - import fdpexpect -else: + +try: from pexpect import fdpexpect +# pexpect < 4.0.0 does not have fdpexpect module +except ImportError: + import fdpexpect + + # Adding pexpect exceptions into this module's namespace from pexpect import EOF, TIMEOUT # NOQA pylint: disable=W0611 diff --git a/devlib/utils/ssh.py b/devlib/utils/ssh.py index 1f517a3ac..23b0a6e3c 100644 --- a/devlib/utils/ssh.py +++ b/devlib/utils/ssh.py @@ -42,11 +42,13 @@ logging.getLogger("paramiko").setLevel(logging.WARNING) # pylint: disable=import-error,wrong-import-position,ungrouped-imports,wrong-import-order import pexpect -from distutils.version import StrictVersion as V -if V(pexpect.__version__) < V('4.0.0'): - import pxssh -else: + +try: from pexpect import pxssh +# pexpect < 4.0.0 does not have a pxssh module +except ImportError: + import pxssh + from pexpect import EOF, TIMEOUT, spawn # pylint: disable=redefined-builtin,wrong-import-position -- GitLab From 6973111aefa9d9a5d0b51c0ff1b40bd495183102 Mon Sep 17 00:00:00 2001 From: Douglas Raillard Date: Thu, 10 Aug 2023 11:25:25 +0100 Subject: [PATCH 2/2] Squashed 'external/workload-automation/' changes from 951eec991..839242d63 839242d63 target/descriptor: Add max_async generic target parameter b9b02f83f build(deps): bump cryptography from 41.0.0 to 41.0.3 6aa1caad9 build(deps): bump certifi from 2022.12.7 to 2023.7.22 bf72a576e uibenchjanktests: Rework to allow listing subtests git-subtree-dir: external/workload-automation git-subtree-split: 839242d636dd1bd76d6df61c5c8309e38931a7c5 --- requirements.txt | 4 +- wa/framework/target/descriptor.py | 6 ++ wa/workloads/uibenchjanktests/__init__.py | 110 +++++++++++++++------- 3 files changed, 86 insertions(+), 34 deletions(-) diff --git a/requirements.txt b/requirements.txt index 4007061de..99a342d11 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,9 +1,9 @@ bcrypt==4.0.1 -certifi==2022.12.7 +certifi==2023.7.22 cffi==1.15.1 charset-normalizer==3.1.0 colorama==0.4.6 -cryptography==41.0.0 +cryptography==41.0.3 devlib==1.3.4 future==0.18.3 idna==3.4 diff --git a/wa/framework/target/descriptor.py b/wa/framework/target/descriptor.py index 0b5941e33..b04e67c0c 100644 --- a/wa/framework/target/descriptor.py +++ b/wa/framework/target/descriptor.py @@ -198,6 +198,12 @@ COMMON_TARGET_PARAMS = [ description=''' A regex that matches the shell prompt on the target. '''), + + Parameter('max_async', kind=int, default=50, + description=''' + The maximum number of concurent asynchronous connections to the + target maintained at any time. + '''), ] COMMON_PLATFORM_PARAMS = [ diff --git a/wa/workloads/uibenchjanktests/__init__.py b/wa/workloads/uibenchjanktests/__init__.py index a56701d71..7fe7931ba 100644 --- a/wa/workloads/uibenchjanktests/__init__.py +++ b/wa/workloads/uibenchjanktests/__init__.py @@ -14,16 +14,21 @@ # import re -from wa import Parameter, ApkWorkload, PackageHandler, TestPackageHandler +from wa import Parameter, ApkWorkload, PackageHandler, TestPackageHandler, ConfigError +from wa.utils.types import list_or_string +from wa.framework.exception import WorkloadError class Uibenchjanktests(ApkWorkload): name = 'uibenchjanktests' description = """ - Runs a particular test of the UIBench JankTests_ test suite. The suite - is provided by Google as an automated version of the UIBench testbench - for the Android UI. + Runs a particular test (or list of tests) of the UIBench JankTests_ + test suite. The suite is provided by Google as an automated version + of the UIBench testbench for the Android UI. + The workload supports running the default set of tests without + restarting the app or running an arbitrary set of tests with + restarting the app in between each test. .. _JankTests: https://android.googlesource.com/platform/platform_testing/+/master/tests/jank/uibench/src/com/android/uibench/janktests """ @@ -37,15 +42,26 @@ class Uibenchjanktests(ApkWorkload): r'INSTRUMENTATION_STATUS: (?P[\w-]+)=(?P[-+\d.]+)') parameters = [ - Parameter('test', kind=str, - description='Test to be run. Defaults to full run.'), + Parameter('tests', kind=list_or_string, + description=""" + Tests to be run. Defaults to running every available + subtest in alphabetical order. The app will be restarted + for each subtest, unlike when using full=True. + """, default=None, aliases=['test']), + Parameter('full', kind=bool, default=False, + description=""" + Runs the full suite of tests that the app defaults to + when no subtests are specified. The actual tests and their + order might depend on the version of the app. The subtests + will be run back to back without restarting the app in between. + """), Parameter('wait', kind=bool, default=True, description='Forces am instrument to wait until the ' 'instrumentation terminates before terminating itself. The ' 'net effect is to keep the shell open until the tests have ' 'finished. This flag is not required, but if you do not use ' 'it, you will not see the results of your tests.'), - Parameter('raw', kind=bool, default=False, + Parameter('raw', kind=bool, default=True, description='Outputs results in raw format. Use this flag ' 'when you want to collect performance measurements, so that ' 'they are not formatted as test results. This flag is ' @@ -92,40 +108,70 @@ class Uibenchjanktests(ApkWorkload): instrument_wait=self.wait, no_hidden_api_checks=self.no_hidden_api_checks) + def validate(self): + if self.full and self.tests is not None: + raise ConfigError("Can't select subtests while 'full' is True") + def initialize(self, context): super(Uibenchjanktests, self).initialize(context) self.dut_apk.initialize(context) self.dut_apk.initialize_package(context) - if 'class' not in self.apk.args: - class_for_method = dict(self.apk.apk_info.methods) - class_for_method[None] = self._DEFAULT_CLASS - try: - method = class_for_method[self.test] - except KeyError as e: - msg = 'Unknown test "{}". Known tests:\n\t{}' - known_tests = '\n\t'.join( - m for m in class_for_method.keys() - if m is not None and m.startswith('test')) - raise ValueError(msg.format(e, known_tests)) - klass = '{}.{}'.format(self.package_names[0], method) - - if self.test: - klass += '#{}'.format(self.test) - self.apk.args['class'] = klass + + self.output = {} + + # Full run specified, don't select subtests + if self.full: + self.apk.args['class'] = '{}.{}'.format( + self.package_names[0], self._DEFAULT_CLASS + ) + return + + self.available_tests = { + test: cl for test, cl in self.apk.apk_info.methods + if test.startswith('test') + } + + # default to running all tests in alphabetical order + # pylint: disable=access-member-before-definition + if not self.tests: + self.tests = sorted(self.available_tests.keys()) + # raise error if any of the tests are not available + elif any([t not in self.available_tests for t in self.tests]): + msg = 'Unknown test(s) specified. Known tests: {}' + known_tests = '\n'.join(self.available_tests.keys()) + raise ValueError(msg.format(known_tests)) def run(self, context): - self.apk.start_activity() - self.apk.wait_instrument_over() + # Full run, just run the activity directly + if self.full: + self.apk.start_activity() + self.apk.wait_instrument_over() + self.output['full'] = self.apk.instrument_output + return + + for test in self.tests: + self.apk.args['class'] = '{}.{}#{}'.format( + self.package_names[0], + self.available_tests[test], test + ) + self.apk.setup(context) + self.apk.start_activity() + try: + self.apk.wait_instrument_over() + except WorkloadError as e: + self.logger.warning(str(e)) + self.output[test] = self.apk.instrument_output def update_output(self, context): super(Uibenchjanktests, self).update_output(context) - output = self.apk.instrument_output - for section in self._OUTPUT_SECTION_REGEX.finditer(output): - if int(section.group('code')) != -1: - msg = 'Run failed (INSTRUMENTATION_STATUS_CODE: {}). See log.' - raise RuntimeError(msg.format(section.group('code'))) - for metric in self._OUTPUT_GFXINFO_REGEX.finditer(section.group()): - context.add_metric(metric.group('name'), metric.group('value')) + for test, test_output in self.output.items(): + for section in self._OUTPUT_SECTION_REGEX.finditer(test_output): + if int(section.group('code')) != -1: + msg = 'Run failed (INSTRUMENTATION_STATUS_CODE: {}). See log.' + raise RuntimeError(msg.format(section.group('code'))) + for metric in self._OUTPUT_GFXINFO_REGEX.finditer(section.group()): + context.add_metric(metric.group('name'), metric.group('value'), + classifiers={'test_name': test}) def teardown(self, context): super(Uibenchjanktests, self).teardown(context) -- GitLab