Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion mobly/controllers/android_device_lib/services/logcat.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ def __init__(self, android_device, configs=None):
self._adb_logcat_process = None
self._adb_logcat_file_obj = None
self.adb_logcat_file_path = None
self._last_connection_time = None
# Logcat service uses a single config obj, using singular internal
# name: `_config`.
self._config = configs if configs else Config()
Expand Down Expand Up @@ -247,9 +248,14 @@ def _start(self):
# In debugging mode of IntelijIDEA, "patch_args" remove
# double quotes in args if starting and ending with it.
# Add spaces at beginning and at last to fix this issue.
cmd = ' "%s" -s %s logcat -v threadtime -T 1 %s >> "%s" ' % (
if self._last_connection_time is not None:
t_argument_value = self._last_connection_time
else:
t_argument_value = '1'
cmd = ' "%s" -s %s logcat -v threadtime -T "%s" %s >> "%s" ' % (
adb.ADB,
self._ad.serial,
t_argument_value,
self._config.logcat_params,
self.adb_logcat_file_path,
)
Expand All @@ -270,6 +276,7 @@ def _stop(self):
except Exception:
self._ad.log.exception('Failed to stop adb logcat.')
self._adb_logcat_process = None
self._last_connection_time = None

def pause(self):
"""Pauses logcat.
Expand All @@ -279,6 +286,12 @@ def pause(self):
some logs would be lost.
"""
self._stop()
try:
response = self._ad.adb.shell(['date', '-Is', r'+%Y-%m-%d\ %H:%M:%S.%N'])
if response:
self._last_connection_time = response.decode('utf-8').strip()
except adb.AdbError as e:
self._ad.log.exception('Failed to get host time.')

def resume(self):
"""Resumes a paused logcat service."""
Expand Down
161 changes: 157 additions & 4 deletions tests/mobly/controllers/android_device_lib/services/logcat_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ def test_start_and_stop(
'logcat,%s,fakemodel,123.txt' % ad.serial,
)
create_dir_mock.assert_called_with(os.path.dirname(expected_log_path))
adb_cmd = ' "adb" -s %s logcat -v threadtime -T 1 >> %s'
adb_cmd = ' "adb" -s %s logcat -v threadtime -T "1" >> %s'
start_proc_mock.assert_called_with(
adb_cmd % (ad.serial, '"%s" ' % expected_log_path), shell=True
)
Expand Down Expand Up @@ -206,7 +206,8 @@ def test_update_config(
self.assertTrue(logcat_service._adb_logcat_process)
create_dir_mock.assert_has_calls([mock.call('some/path')])
expected_adb_cmd = (
' "adb" -s 1 logcat -v threadtime -T 1 -a -b -c >> "some/path/log.txt" '
' "adb" -s 1 logcat -v threadtime -T "1" -a -b -c >>'
' "some/path/log.txt" '
)
start_proc_mock.assert_called_with(expected_adb_cmd, shell=True)
self.assertEqual(logcat_service.adb_logcat_file_path, 'some/path/log.txt')
Expand Down Expand Up @@ -264,8 +265,118 @@ def test_update_config_while_running(
'mobly.controllers.android_device_lib.services.logcat.Logcat.clear_adb_log',
return_value=mock_android_device.MockAdbProxy('1'),
)
@mock.patch('mobly.logger.get_log_file_timestamp')
def test_pause_and_resume(
self,
get_timestamp_mock,
clear_adb_mock,
open_logcat_mock,
stop_proc_mock,
start_proc_mock,
create_dir_mock,
FastbootProxy,
MockAdbProxy,
):
# The test will pause and resume the log collection twice. The
# resume operation is supposed to request the log messages since the
# log collection got paused.

DATE_COMMAND = ['date', '-Is', r'+%Y-%m-%d\ %H:%M:%S.%N']
Copy link
Collaborator

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?

Copy link
Author

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.

DATE_RESULT_FIRST_PAUSE = '2026-01-14 17:01:02.342'
DATE_RESULT_SECOND_PAUSE = '2026-01-14 17:03:04.789'
state = {'date_result' : ''}

adb_instance = MockAdbProxy.return_value
adb_instance.shell = mock.MagicMock()
adb_instance.has_shell_command = mock.MagicMock(return_value=True)

def adb_shell_logic(cmd, *args, **kwargs):
if cmd == DATE_COMMAND:
return state['date_result'].encode('utf-8')
return b'' # Use empty bytes to match ADB's typical return type
adb_instance.shell.side_effect = adb_shell_logic

# Set up the logcat instance.
mock_serial = '1'
ad = android_device.AndroidDevice(serial=mock_serial)
get_timestamp_mock.return_value = '123'
configs = logcat.Config(clear_log=True)
logcat_service = logcat.Logcat(ad, configs)
logcat_service.start()
expected_log_path = os.path.join(
logging.log_path,
'AndroidDevice%s' % ad.serial,
'logcat,%s,fakemodel,123.txt' % ad.serial,
)

# We expect that the first logcat invocation happens with time "1"
# to include all previously generated log messages.
adb_cmd = ' "adb" -s %s logcat -v threadtime -T "1" >> %s'
start_proc_mock.assert_called_with(
adb_cmd % (ad.serial, '"%s" ' % expected_log_path), shell=True
)

clear_adb_mock.assert_called_once_with()
self.assertTrue(logcat_service.is_alive)

def pause_logcat_service_resume_logcat_service(date):
# Pause the log collection and then resume the log collection.
#
# The passed in `date` is what we expect to receive as an argument
# to the `logcat -T` argument when the logcat serice gets resumed.
# The `date` argument corresponds to the time stamp when the log
# collection got stopped, and we expect that we look back to that
# time stamp when we resume the log collection.

# Pause the logcat collection at time stamp `date`.
state['date_result'] = date
logcat_service.pause()
self.assertFalse(logcat_service.is_alive)
stop_proc_mock.assert_called_with('process')
self.assertIsNone(logcat_service._adb_logcat_process)
clear_adb_mock.reset_mock()

# Resume the logcat collection, which should look back to time
# stamp `date`.
logcat_service.resume()
self.assertTrue(logcat_service.is_alive)
clear_adb_mock.assert_not_called()
adb_cmd = (
' "adb" -s %s logcat -v threadtime -T "' + date + '" >> %s'
)
start_proc_mock.assert_called_with(
adb_cmd % (ad.serial, '"%s" ' % expected_log_path), shell=True
)

# First pause / resume iteration
pause_logcat_service_resume_logcat_service(DATE_RESULT_FIRST_PAUSE)
# self.assertEqual(adb_instance.shell.call_count, 1)

# Second pause / resume iteration
pause_logcat_service_resume_logcat_service(DATE_RESULT_SECOND_PAUSE)

logcat_service.stop()

@mock.patch(
'mobly.controllers.android_device_lib.adb.AdbProxy',
return_value=mock_android_device.MockAdbProxy('1'),
)
@mock.patch(
'mobly.controllers.android_device_lib.fastboot.FastbootProxy',
return_value=mock_android_device.MockFastbootProxy('1'),
)
@mock.patch('mobly.utils.create_dir')
@mock.patch('mobly.utils.start_standing_subprocess', return_value='process')
@mock.patch('mobly.utils.stop_standing_subprocess')
@mock.patch.object(logcat.Logcat, '_open_logcat_file')
@mock.patch(
'mobly.controllers.android_device_lib.services.logcat.Logcat.clear_adb_log',
return_value=mock_android_device.MockAdbProxy('1'),
)
@mock.patch('mobly.logger.get_log_file_timestamp')
def test_pause_and_resume_while_raising_exception_when_getting_pause_time(
self,
get_timestamp_mock,
clear_adb_mock,
open_logcat_mock,
stop_proc_mock,
Expand All @@ -274,20 +385,62 @@ def test_pause_and_resume(
FastbootProxy,
MockAdbProxy,
):
# The test will pause and resume the log collection, but it will
# fail to retrieve a date while pausing the log collection. The log
# collection is expected to resume back from the earliest time stamp
# "1".

DATE_COMMAND = ['date', '-Is', r'+%Y-%m-%d\ %H:%M:%S.%N']

adb_instance = MockAdbProxy.return_value
adb_instance.shell = mock.MagicMock()
adb_instance.has_shell_command = mock.MagicMock(return_value=True)

def adb_shell_logic(cmd, *args, **kwargs):
if cmd == DATE_COMMAND:
raise adb.AdbError(cmd, b'', b'Device is disconnected', 1)
return b'' # Use empty bytes to match ADB's typical return type
adb_instance.shell.side_effect = adb_shell_logic

# Set up the logcat instance.
mock_serial = '1'
ad = android_device.AndroidDevice(serial=mock_serial)
logcat_service = logcat.Logcat(ad, logcat.Config(clear_log=True))
get_timestamp_mock.return_value = '123'
configs = logcat.Config(clear_log=True)
logcat_service = logcat.Logcat(ad, configs)
logcat_service.start()
expected_log_path = os.path.join(
logging.log_path,
'AndroidDevice%s' % ad.serial,
'logcat,%s,fakemodel,123.txt' % ad.serial,
)

# We expect that the first logcat invocation happens with time "1"
# to include all previously generated log messages.
adb_cmd = ' "adb" -s %s logcat -v threadtime -T "1" >> %s'
start_proc_mock.assert_called_with(
adb_cmd % (ad.serial, '"%s" ' % expected_log_path), shell=True
)

clear_adb_mock.assert_called_once_with()
self.assertTrue(logcat_service.is_alive)

# Pause should fail to retrieve a valid date
logcat_service.pause()
self.assertFalse(logcat_service.is_alive)
stop_proc_mock.assert_called_with('process')
self.assertIsNone(logcat_service._adb_logcat_process)
clear_adb_mock.reset_mock()

# Resume should start with the first logcat message again
logcat_service.resume()
self.assertTrue(logcat_service.is_alive)
clear_adb_mock.assert_not_called()
adb_cmd = ' "adb" -s %s logcat -v threadtime -T "1" >> %s'
start_proc_mock.assert_called_with(
adb_cmd % (ad.serial, '"%s" ' % expected_log_path), shell=True
)

logcat_service.stop()

@mock.patch(
Expand Down Expand Up @@ -422,7 +575,7 @@ def test_take_logcat_with_extra_params(
'logcat,%s,fakemodel,123.txt' % ad.serial,
)
create_dir_mock.assert_called_with(os.path.dirname(expected_log_path))
adb_cmd = ' "adb" -s %s logcat -v threadtime -T 1 -b radio >> %s'
adb_cmd = ' "adb" -s %s logcat -v threadtime -T "1" -b radio >> %s'
start_proc_mock.assert_called_with(
adb_cmd % (ad.serial, '"%s" ' % expected_log_path), shell=True
)
Expand Down
Loading