From dfdf19f2254a65a6f85924e6b2cabb0d3c872de8 Mon Sep 17 00:00:00 2001 From: Zhuoyao Zhang Date: Mon, 20 May 2024 18:31:12 +0000 Subject: [PATCH] Fix a bug in run_tool_with_logging script Fix the case when call a tool with run_tool_with_logging with a single arg start with -- (e.g. adb --help), make sure when we pass that arg to the logger, it treat it as the value for the --tool_args option instead of a separate option Test: atest run_tool_with_logging_test Test: manually run source build/envsetup.sh and run adb --help and check the event log is sent to clearcut. Tested with both bash and zsh Bug: 341382247 Change-Id: I1e09907f267b453cb62876e171064daa021e3d91 --- envsetup.sh | 10 +++++----- tests/run_tool_with_logging_test.py | 21 +++++++++++++++++---- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/envsetup.sh b/envsetup.sh index b20c83fed3..640ed149e6 100644 --- a/envsetup.sh +++ b/envsetup.sh @@ -847,11 +847,11 @@ function run_tool_with_logging() { # Remove the trap to prevent duplicate log. trap - EXIT; "${logger}" \ - --tool_tag "${tool_tag}" \ - --start_timestamp "${start_time}" \ - --end_timestamp "$(date +%s.%N)" \ - --tool_args "$*" \ - --exit_code "${exit_code}" \ + --tool_tag="${tool_tag}" \ + --start_timestamp="${start_time}" \ + --end_timestamp="$(date +%s.%N)" \ + --tool_args="$*" \ + --exit_code="${exit_code}" \ ${ANDROID_TOOL_LOGGER_EXTRA_ARGS} \ > /dev/null 2>&1 & exit ${exit_code} diff --git a/tests/run_tool_with_logging_test.py b/tests/run_tool_with_logging_test.py index 18abd8e54f..6f9b59c5c8 100644 --- a/tests/run_tool_with_logging_test.py +++ b/tests/run_tool_with_logging_test.py @@ -90,8 +90,8 @@ class RunToolWithLoggingTest(unittest.TestCase): test_tool.assert_called_once_with_args("arg1 arg2") expected_logger_args = ( - "--tool_tag FAKE_TOOL --start_timestamp \d+\.\d+ --end_timestamp" - " \d+\.\d+ --tool_args arg1 arg2 --exit_code 0" + "--tool_tag=FAKE_TOOL --start_timestamp=\d+\.\d+ --end_timestamp=" + "\d+\.\d+ --tool_args=arg1 arg2 --exit_code=0" ) test_logger.assert_called_once_with_args(expected_logger_args) @@ -163,8 +163,8 @@ class RunToolWithLoggingTest(unittest.TestCase): self.assertEqual(returncode, INTERRUPTED_RETURN_CODE) expected_logger_args = ( - "--tool_tag FAKE_TOOL --start_timestamp \d+\.\d+ --end_timestamp" - " \d+\.\d+ --tool_args arg1 arg2 --exit_code 130" + "--tool_tag=FAKE_TOOL --start_timestamp=\d+\.\d+ --end_timestamp=" + "\d+\.\d+ --tool_args=arg1 arg2 --exit_code=130" ) test_logger.assert_called_once_with_args(expected_logger_args) @@ -205,6 +205,19 @@ class RunToolWithLoggingTest(unittest.TestCase): self._assert_logger_dry_run() + def test_tool_args_do_not_fail_logger(self): + test_tool = TestScript.create(self.working_dir) + logger_path = self._import_logger() + + self._run_script_and_wait(f""" + TMPDIR="{self.working_dir.name}" + ANDROID_TOOL_LOGGER="{logger_path}" + ANDROID_TOOL_LOGGER_EXTRA_ARGS="--dry_run" + run_tool_with_logging "FAKE_TOOL" {test_tool.executable} --tool-arg1 + """) + + self._assert_logger_dry_run() + def _import_logger(self) -> Path: logger = "tool_event_logger" logger_path = Path(self.working_dir.name).joinpath(logger)