-
Notifications
You must be signed in to change notification settings - Fork 120
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix applaunch workload #1261
base: master
Are you sure you want to change the base?
Fix applaunch workload #1261
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,7 @@ | |
# | ||
# pylint: disable=attribute-defined-outside-init | ||
|
||
from wa import ApkUiautoWorkload, Parameter | ||
from wa import ApkUiautoWorkload, Parameter, TargetError | ||
from wa.framework import pluginloader | ||
|
||
|
||
|
@@ -89,6 +89,54 @@ class Applaunch(ApkUiautoWorkload): | |
"""), | ||
] | ||
|
||
def __init__(self, target, **kwargs): | ||
super(Applaunch, self).__init__(target, **kwargs) | ||
# Android doesn't allow to writable dex files starting 14 version and | ||
# default location (/sdcard/devlib-target) doesn't support readonly files | ||
# so we use /data/local/tmp as asset directory for this workload. | ||
self.asset_directory = '/data/local/tmp' | ||
self._su_has_command_option = None | ||
|
||
def workload_apk(self): | ||
return self.deployed_assets[-1] | ||
|
||
def initialize(self, context): | ||
super(Applaunch, self).initialize(context) | ||
|
||
worload_apk = self.workload_apk() | ||
self.gui.uiauto_params['workload_apk'] = worload_apk | ||
|
||
# Make workload apk readonly to comply with Android >= 14. | ||
if self.target.get_sdk_version() >= 34: | ||
self.target.execute(f'chmod -w {worload_apk}') | ||
|
||
# Check installed su version and return whether it supports -c argument. | ||
# | ||
# Targets can have different versions of su | ||
# - Targets with engineering Android version have su with following usage: | ||
# su [WHO [COMMAND...]] | ||
# - Targets with rooted user Android version have su that supports passing | ||
# command via -c argument. | ||
def su_has_command_option(self): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thinking about this a bit more, do other workloads that require root work on these engineering versions if they are relying on the In devlib we try to detect [1] which [1] https://github.com/ARM-software/devlib/blob/master/devlib/utils/android.py#L466 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I think moving this check to devlib is a good idea, thank you! I will create a PR for it. |
||
if self._su_has_command_option is None: | ||
try: | ||
self.target.execute('su -c id') | ||
self._su_has_command_option = True | ||
except TargetError: | ||
self._su_has_command_option = False | ||
|
||
if self._su_has_command_option is False: | ||
try: | ||
self.target.execute('su root id') | ||
except TargetError: | ||
raise WorkloadError( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing import for |
||
'su must be installed and support passing command ' | ||
'via -c argument (su -c <command>) or as positional ' | ||
'argument after user (su <user> <command>)' | ||
) | ||
|
||
return self._su_has_command_option | ||
|
||
def init_resources(self, context): | ||
super(Applaunch, self).init_resources(context) | ||
self.workload_params['markers_enabled'] = True | ||
|
@@ -112,6 +160,7 @@ def pass_parameters(self): | |
self.gui.uiauto_params['launch_activity'] = "None" | ||
self.gui.uiauto_params['applaunch_type'] = self.applaunch_type | ||
self.gui.uiauto_params['applaunch_iterations'] = self.applaunch_iterations | ||
self.gui.uiauto_params['su_has_command_option'] = self.su_has_command_option() | ||
|
||
def setup(self, context): | ||
self.workload.gui.uiauto_params['package_name'] = self.workload.apk.apk_info.package | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We tend to try and avoid hardcoding paths to support different configurations if possible.
Currently we have the default
self.executables_directory
set to/data/local/tmp/bin
. I'm wondering if these directories would share the same requirements which we could also make use of here rather than setting this parameter in two locations?If not perhaps this be a workload Parameter with a default so a user can understand the requirements for the directory and optionally change this for their system if required?
[1] https://github.com/ARM-software/devlib/blob/7276097d4e12ff2b3cfa1bb0ba40cee24ae3372b/devlib/target.py#L2532
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main requirement for
assert_directory
forapplaunch
workload is a ability to create readonly files there and looks likeexecutables_directory
in general doesn't require it. But adding a new parameter forapplauch
workload with a default value is a good idea, thank you!