faft: Use existing CmdResult class instead of a separate ShellResult. am: 9cdfd64f03
Change-Id: I45862072ea47e78696209a363695f092fbf8c507
diff --git a/client/common_lib/error.py b/client/common_lib/error.py
index e0dbc36..e284d8c 100644
--- a/client/common_lib/error.py
+++ b/client/common_lib/error.py
@@ -117,7 +117,12 @@
class CmdError(TestError):
- """Indicates that a command failed, is fatal to the test unless caught."""
+ """Indicates that a command failed, is fatal to the test unless caught.
+
+ @type command: str
+ @type result_obj: autotest_lib.client.common_lib.utils.CmdResult
+ @type additional_text: str | None
+ """
def __init__(self, command, result_obj, additional_text=None):
TestError.__init__(self, command, result_obj, additional_text)
self.command = command
diff --git a/client/cros/faft/utils/firmware_updater.py b/client/cros/faft/utils/firmware_updater.py
index 1135986..6efabed 100644
--- a/client/cros/faft/utils/firmware_updater.py
+++ b/client/cros/faft/utils/firmware_updater.py
@@ -9,9 +9,9 @@
import json
import os
+from autotest_lib.client.common_lib import error
from autotest_lib.client.common_lib.cros import chip_utils
-from autotest_lib.client.cros.faft.utils import (flashrom_handler,
- shell_wrapper)
+from autotest_lib.client.cros.faft.utils import flashrom_handler
class FirmwareUpdaterError(Exception):
@@ -611,16 +611,9 @@
extracted from cbfs using cbfs_extract_chip().
The hash data is returned as hexadecimal string.
- Args:
- fw_name:
- Chip firmware name whose hash blob to get.
-
- Returns:
- Boolean success status.
-
- Raises:
- shell_wrapper.ShellError: Underlying remote shell
- operations failed.
+ @param fw_name: Chip firmware name whose hash blob to get.
+ @return: Boolean success status.
+ @raise error.CmdError: Underlying remote shell operations failed.
"""
hexdump_cmd = '%s %s.hash' % (
@@ -635,16 +628,10 @@
bios.bin. All files referenced are expected to be in the
directory set up using cbfs_setup_work_dir().
- Args:
- fw_name: Chip firmware name to be replaced.
- extension: Extension of the name of the cbfs component.
-
- Returns:
- Boolean success status.
-
- Raises:
- shell_wrapper.ShellError: Underlying remote shell
- operations failed.
+ @param fw_name: Chip firmware name to be replaced.
+ @param extension: Extension of the name of the cbfs component.
+ @return: Boolean success status.
+ @raise error.CmdError: Underlying remote shell operations failed.
"""
bios = os.path.join(self._cbfs_work_path, self._bios_path)
@@ -671,7 +658,7 @@
self.os_if.run_shell_command(rm_bin_cmd)
try:
self.os_if.run_shell_command(expand_cmd)
- except shell_wrapper.ShellError:
+ except error.CmdError:
self.os_if.log(
('%s may be too old, '
'continuing without "expand" support') % self.CBFSTOOL)
@@ -680,7 +667,7 @@
self.os_if.run_shell_command(add_bin_cmd)
try:
self.os_if.run_shell_command(truncate_cmd)
- except shell_wrapper.ShellError:
+ except error.CmdError:
self.os_if.log(
('%s may be too old, '
'continuing without "truncate" support') % self.CBFSTOOL)
@@ -708,7 +695,7 @@
try:
self.os_if.run_shell_command(expand_cmd)
- except shell_wrapper.ShellError:
+ except error.CmdError:
self.os_if.log(
'%s may be too old, continuing without "expand" support'
% self.CBFSTOOL)
@@ -717,7 +704,7 @@
try:
self.os_if.run_shell_command(truncate_cmd)
- except shell_wrapper.ShellError:
+ except error.CmdError:
self.os_if.log(
'%s may be too old, continuing without "truncate" support'
% self.CBFSTOOL)
diff --git a/client/cros/faft/utils/flashrom_handler.py b/client/cros/faft/utils/flashrom_handler.py
index 5ea26a7..0b663ed 100644
--- a/client/cros/faft/utils/flashrom_handler.py
+++ b/client/cros/faft/utils/flashrom_handler.py
@@ -15,9 +15,9 @@
import struct
import tempfile
+from autotest_lib.client.common_lib import error
from autotest_lib.client.common_lib.cros import chip_utils
-from autotest_lib.client.cros.faft.utils import (saft_flashrom_util,
- shell_wrapper)
+from autotest_lib.client.cros.faft.utils import saft_flashrom_util
class FvSection(object):
@@ -185,9 +185,10 @@
try:
self.fum.check_target()
self._available = True
- except shell_wrapper.ShellError as e:
+ except error.CmdError as e:
+ # First line: "Command <flashrom -p host> failed, rc=2"
+ self._unavailable_err = str(e).split('\n', 1)[0]
self._available = False
- self._unavailable_err = str(e).capitalize()
return self._available
def section_file(self, *paths):
diff --git a/client/cros/faft/utils/os_interface.py b/client/cros/faft/utils/os_interface.py
index 68cc264..a891bbb 100644
--- a/client/cros/faft/utils/os_interface.py
+++ b/client/cros/faft/utils/os_interface.py
@@ -92,11 +92,15 @@
the command, but don't actually run it.
This should be set for RPC commands that alter
the OS or firmware in some persistent way.
+
+ @return: the result of the command, or None if skipped for test mode.
+ @rtype: autotest_lib.client.common_lib.utils.CmdResult | None
+ @raise autotest_lib.client.common_lib.error.CmdError: if command fails
"""
if self.test_mode and modifies_device:
self.log('[SKIPPED] %s' % cmd)
else:
- self.shell.run_command(cmd)
+ return self.shell.run_command(cmd)
def run_shell_command_check_output(self, cmd, success_token):
"""Run shell command and check its stdout for a string."""
diff --git a/client/cros/faft/utils/saft_flashrom_util.py b/client/cros/faft/utils/saft_flashrom_util.py
index 871c5c9..6b54c0a 100644
--- a/client/cros/faft/utils/saft_flashrom_util.py
+++ b/client/cros/faft/utils/saft_flashrom_util.py
@@ -275,7 +275,7 @@
The command executed is just 'flashrom -p <target>'.
@return: True if flashrom completed successfully
- @raise ShellError: if flashrom exited with an error code
+ @raise autotest_lib.client.common_lib.error.CmdError: if flashrom failed
"""
cmd = 'flashrom %s' % self._target_command
self.os_if.run_shell_command(cmd)
diff --git a/client/cros/faft/utils/saft_flashrom_util_unittest.py b/client/cros/faft/utils/saft_flashrom_util_unittest.py
index 3c97bc2..2f78b27 100644
--- a/client/cros/faft/utils/saft_flashrom_util_unittest.py
+++ b/client/cros/faft/utils/saft_flashrom_util_unittest.py
@@ -5,9 +5,9 @@
import unittest
from autotest_lib.client.common_lib import autotemp
+from autotest_lib.client.common_lib import error
from autotest_lib.client.cros.faft.utils import (os_interface,
- saft_flashrom_util,
- shell_wrapper)
+ saft_flashrom_util)
class TestFlashromUtil(unittest.TestCase):
@@ -23,7 +23,7 @@
@mock.patch('subprocess.Popen')
def testTargetIsBroken(self, mock_subproc_popen):
- """check_target should raise ShellError if flashrom is broken"""
+ """check_target should raise error.CmdError if flashrom is broken"""
bad_flashrom = mock.Mock()
bad_flashrom.stdout = StringIO.StringIO('broken flashrom stdout')
@@ -31,7 +31,7 @@
bad_flashrom.returncode = 1
mock_subproc_popen.return_value = bad_flashrom
- with self.assertRaises(shell_wrapper.ShellError):
+ with self.assertRaises(error.CmdError):
self.flashrom_util.check_target()
@mock.patch('subprocess.Popen')
diff --git a/client/cros/faft/utils/shell_wrapper.py b/client/cros/faft/utils/shell_wrapper.py
index 7d55c0b..fe7000d 100644
--- a/client/cros/faft/utils/shell_wrapper.py
+++ b/client/cros/faft/utils/shell_wrapper.py
@@ -5,10 +5,10 @@
import subprocess
+import time
-class ShellError(Exception):
- """Shell specific exception."""
- pass
+from autotest_lib.client.common_lib import error
+from autotest_lib.client.common_lib import utils
class UnsupportedSuccessToken(Exception):
@@ -30,7 +30,7 @@
output in case command succeeded. If block=False, will not wait for
process to return before returning.
"""
- self._os_if.log('Executing %s' % cmd)
+ self._os_if.log('Executing: %s' % cmd)
process = subprocess.Popen(
cmd,
shell=True,
@@ -47,19 +47,24 @@
outputs on the console and dump them into the log. Otherwise suppress
all output.
- In case of command error raise an ShellError exception.
+ @raise error.CmdError: if block is True and command fails (rc!=0)
+ @return: the result, if block is True
+
+ @rtype: utils.CmdResult | None
"""
- process = self._run_command(cmd, block)
- if process.returncode:
- err = ['Failed running: %s' % cmd]
- err.append('stdout:')
- err.append(process.stdout.read())
- err.append('stderr:')
- err.append(process.stderr.read())
- text = '\n'.join(err)
- self._os_if.log(text)
- raise ShellError(
- 'command %s failed (code: %d)' % (cmd, process.returncode))
+ start_time = time.time()
+ process = self._run_command(cmd)
+ if block:
+ returncode = process.returncode
+ stdout = process.stdout.read()
+ stderr = process.stderr.read()
+ duration = time.time() - start_time
+ result = utils.CmdResult(cmd, stdout, stderr, returncode, duration)
+ if returncode:
+ self._os_if.log('Command failed.\n%s' % result)
+ raise error.CmdError(cmd, result)
+ else:
+ return result
def run_command_check_output(self, cmd, success_token):
"""Run a command and check whether standard output contains some string.
diff --git a/server/site_tests/firmware_FAFTRPC/config.py b/server/site_tests/firmware_FAFTRPC/config.py
index 71b4ba2..8f67e25 100644
--- a/server/site_tests/firmware_FAFTRPC/config.py
+++ b/server/site_tests/firmware_FAFTRPC/config.py
@@ -115,6 +115,18 @@
],
},
{
+ "method_name": "RunShellCommandGetStatus",
+ "passing_args": [
+ ("ls ''",),
+ ],
+ },
+ {
+ "method_name": "RunShellCommand",
+ "failing_args": [
+ ("ls ''",),
+ ],
+ },
+ {
"method_name": "RunShellCommandCheckOutput",
"passing_args": [
("ls -l", "total"),
@@ -409,7 +421,7 @@
"method_name": "RebootToSwitchSlot",
"passing_args": [NO_ARGS],
"failing_args": [ONE_INT_ARG, ONE_STR_ARG],
- "allow_error_msg": "ShellError",
+ "allow_error_msg": "CmdError",
},
],
},