Repository navigation
[Android] Integrate exit code handling into process_handler #5455
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
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 |
|---|---|---|
|
|
@@ -174,6 +174,7 @@ def run_process(cmdline, | |
| if is_android: | ||
| # Clear the log upfront. | ||
| android.logger.clear_log() | ||
| initial_uptime = android.adb.time_since_last_reboot() | ||
|
|
||
| # Run the app. | ||
| adb_output = android.adb.run_command( | ||
|
|
@@ -253,15 +254,26 @@ def run_process(cmdline, | |
| # waits for device to be online. | ||
| time.sleep(ANDROID_CRASH_LOGCAT_WAIT_TIME) | ||
| output = android.logger.log_output() | ||
| app_package = android.app.get_package_name() | ||
| process_pid = android.util.get_latest_pid_for_package(app_package) | ||
| exit_info = android.util.get_exit_info_for_pid(app_package, process_pid) | ||
|
|
||
| if android.constants.LOW_MEMORY_REGEX.search(output): | ||
| if android.util.activity_crashed(exit_info): | ||
| logs.warning(f'Activity Crashed with: {exit_info}') | ||
| return_code = exit_info.reason | ||
|
|
||
| elif android.constants.LOW_MEMORY_REGEX.search(output): | ||
|
Collaborator
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. nit: I would use exit reason low memory. I'd consider that a bit more consistent
Collaborator
Author
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. Currently when memory on the device is low, CF performs a reset on the device and then continues the execution without marking the test case or the bad build check as a crash, returning a non 0 exit reason here would change that, so i don't think this one is feasable. |
||
| # If the device is low on memory, we should force reboot and bail out to | ||
| # prevent device from getting in a frozen state. | ||
| logs.info('Device is low on memory, rebooting.', output=output) | ||
| android.adb.hard_reset() | ||
| android.adb.wait_for_device() | ||
|
|
||
| elif android.adb.time_since_last_reboot() < time.time() - start_time: | ||
| elif android.adb.time_since_last_reboot() < initial_uptime: | ||
| logs.info( | ||
| 'Device rebooted mid-run', | ||
| output=f'initial uptime: {initial_uptime}, ' | ||
| f'current uptime: {android.adb.time_since_last_reboot()}') | ||
| # Check if a reboot has happened, if yes, append log output before reboot | ||
| # and kernel logs content to output. | ||
| log_before_last_reboot = android.logger.log_output_before_last_reboot() | ||
|
|
@@ -333,6 +345,11 @@ def run_process(cmdline, | |
|
|
||
| if testcase_run and (crash_analyzer.is_memory_tool_crash(output) or | ||
| crash_analyzer.is_check_failure_crash(output)): | ||
| if return_code: | ||
| logs.warning( | ||
| f'Process {cmdline!r} ended with exit code {return_code}, ' | ||
| 'but crash has been detected, overriding return code to 1.', | ||
| output=output) | ||
| return_code = 1 | ||
|
|
||
| # If a crash is found, then we add the memory state as well. | ||
|
|
||
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.
Just to confirm, it seems that return_code can be overwritten, could this cause any issues?
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.
Other than bringing more clarity it shouldn't bring any issues, actually in this method we usually change the return_code to tell callers if the invoked process crashed, see example:
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.
What I meant is that the value assigned to return_code here can be overwritten later in the function.
I just wanted to flag this to confirm if it is not a problem.
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.
You are right, i see it can be overwritten to
If there was a memory crash... this seems very delicate so I'll add a log there to note the dev that the return_code was modified midflight.
Thanks for catching that!