-
Notifications
You must be signed in to change notification settings - Fork 207
Capture logcat messages while logcat collection was suspended #1001
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
base: master
Are you sure you want to change the base?
Conversation
c5e6855 to
894493f
Compare
tests/mobly/controllers/android_device_lib/services/logcat_test.py
Outdated
Show resolved
Hide resolved
| FastbootProxy, | ||
| MockAdbProxy, | ||
| ): | ||
| DATE_COMMAND = ['date', '-Is', r'+%Y-%m-%d\ %H:%M:%S.%N'] |
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.
I didn't expect to see so much change in the existing test.
is it possible to simplify the logic here?
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.
I added comments around the code, but I feel this is as simple as it gets, not sure how to simplify this any further.
| clear_adb_mock.assert_called_once_with() | ||
| self.assertTrue(logcat_service.is_alive) | ||
|
|
||
| def pause_resume(date): |
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.
It's difficult to understand the purpose of this from the name.
what are we doing here? could we make the test more readable and easier to understand? E.g. simplify the structure, change the name, add inline comments.
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.
I have renamed the function to pause_logcat_service_resume_logcat_service and added comments into the code to explain what we are doing here. I hope that helps.
894493f to
7846301
Compare
Prior to this change the logcat collection would be missing logcat messages that got created while the logcat collection was suspended if the logcat collection got subsequently resumed. This change stores the local time when the adb connection got suspended, so that we can use it to specify from which point in time we want to collect logcat messages again. This ensures that the final log file contains the log messages while the logcat collection was suspended once the logcat collection got resumed.
7846301 to
b6d3318
Compare
Prior to this change the logcat collection would be missing logcat messages that got created while the logcat collection was suspended if the logcat collection got subsequently resumed.
This change stores the local time when the adb connection got suspended, so that we can use it to specify from which point in time we want to collect logcat messages again. This ensures that the final log file contains the log messages while the logcat collection was suspended once the logcat collection got resumed.
Fixes #1000