crosperf: RunCommand should return one type of object.

Cleaned up the interfaces for the RunCommand routines.
These were returning different types (int or tuple) depending on
the value of the return_ouput parameter.
Returning different unrelated types from a routine is bad practice.
Linter complains about this with several warnings like this:

"Attempting to unpack a non-sequence defined at line XY of
utils.command_executer"

BUG=chromium:566256
TEST=ran crosperf with a example experiment file
     Ran run_tests.

Change-Id: Ibb83ab9322c87558077fc4937ef5c0686bbe5417
Reviewed-on: https://chrome-internal-review.googlesource.com/241459
Commit-Ready: Luis Lozano <llozano@chromium.org>
Tested-by: Luis Lozano <llozano@chromium.org>
Reviewed-by: Han Shen <shenhan@google.com>

4 files changed, 145 insertions, 64 deletions

diff --git a/utils/buildbot_utils.py b/utils/buildbot_utils.py
index 135347a3..cff30073 100644
--- a/utils/buildbot_utils.py
+++ b/utils/buildbot_utils.py
@@ -123,8 +123,7 @@ def GetBuildInfo(file_dir, builder):
         commands += " -b release"
       else:
         commands += " -b %s" % builder
-    _, buildinfo, _ = ce.RunCommand(commands, return_output=True,
-                                    print_to_console=False)
+    _, buildinfo, _ = ce.RunCommandWOutput(commands, print_to_console=False)
     build_log = buildinfo.splitlines()
     return build_log
 
@@ -138,9 +137,8 @@ def FindArchiveImage(chromeos_root, build, build_id):
     ce = command_executer.GetCommandExecuter()
     command = ("gsutil ls gs://chromeos-image-archive/trybot-%s/*b%s"
                "/chromiumos_test_image.tar.xz" % (build, build_id))
-    retval, out, err = ce.ChrootRunCommand(chromeos_root, command,
-                                           return_output=True,
-                                           print_to_console=False)
+    retval, out, err = ce.ChrootRunCommandWOutput(chromeos_root, command,
+                                                  print_to_console=False)
     #
     # If build_id is not unique, there may be multiple archive images
     # to choose from; sort them & pick the first (newest).
@@ -214,7 +212,7 @@ def GetTrybotImage(chromeos_root, buildbot_name, patch_list, build_tag,
     command = ("./cbuildbot --remote --nochromesdk --notests"
                " --remote-description=%s %s %s %s"
                % (description, toolchain_flags, patch_arg, build))
-    _, out, _ = ce.RunCommand(command, return_output=True)
+    _, out, _ = ce.RunCommandWOutput(command)
     if "Tryjob submitted!" not in out:
       logger.GetLogger().LogFatal("Error occurred while launching trybot job: "
                                   "%s" % command)
diff --git a/utils/command_executer.py b/utils/command_executer.py
index 5e86d6f1..e13fb3b0 100644
--- a/utils/command_executer.py
+++ b/utils/command_executer.py
@@ -54,12 +54,15 @@ class CommandExecuter(object):
   def SetLogLevel(self, log_level):
     self.log_level = log_level
 
-  def RunCommand(self, cmd, return_output=False, machine=None,
-                 username=None, command_terminator=None,
-                 command_timeout=None,
-                 terminated_timeout=10,
-                 print_to_console=True):
-    """Run a command."""
+  def RunCommandGeneric(self, cmd, return_output=False, machine=None,
+                        username=None, command_terminator=None,
+                        command_timeout=None,
+                        terminated_timeout=10,
+                        print_to_console=True):
+    """Run a command.
+
+    Returns triplet (returncode, stdout, stderr).
+    """
 
     cmd = str(cmd)
 
@@ -73,10 +76,7 @@ class CommandExecuter(object):
     if command_terminator and command_terminator.IsTerminated():
       if self.logger:
         self.logger.LogError("Command was terminated!", print_to_console)
-      if return_output:
-        return [1, "", ""]
-      else:
-        return 1
+      return (1, "", "")
 
     if machine is not None:
       user = ""
@@ -162,7 +162,31 @@ class CommandExecuter(object):
     p.wait()
     if return_output:
       return (p.returncode, full_stdout, full_stderr)
-    return p.returncode
+    return (p.returncode, "", "")
+
+  def RunCommand(self, *args, **kwargs):
+    """Run a command.
+
+    Takes the same arguments as RunCommandGeneric except for return_output.
+    Returns a single value returncode.
+    """
+    # Make sure that args does not overwrite 'return_output'
+    assert len(args) <= 1
+    assert 'return_output' not in kwargs
+    kwargs['return_output'] = False
+    return self.RunCommandGeneric(*args, **kwargs)[0]
+
+  def RunCommandWOutput(self, *args, **kwargs):
+    """Run a command.
+
+    Takes the same arguments as RunCommandGeneric except for return_output.
+    Returns a triplet (returncode, stdout, stderr).
+    """
+    # Make sure that args does not overwrite 'return_output'
+    assert len(args) <= 1
+    assert 'return_output' not in kwargs
+    kwargs['return_output'] = True
+    return self.RunCommandGeneric(*args, **kwargs)
 
   def RemoteAccessInitCommand(self, chromeos_root, machine):
     command = ""
@@ -187,18 +211,21 @@ class CommandExecuter(object):
     command = self.RemoteAccessInitCommand(chromeos_root, machine)
     command += "\nlearn_board"
     command += "\necho ${FLAGS_board}"
-    retval, output, _ = self.RunCommand(command, True)
+    retval, output, _ = self.RunCommandWOutput(command)
     if self.logger:
       self.logger.LogFatalIf(retval, "learn_board command failed")
     elif retval:
       sys.exit(1)
     return output.split()[-1]
 
-  def CrosRunCommand(self, cmd, return_output=False, machine=None,
-                     command_terminator=None,
-                     chromeos_root=None, command_timeout=None,
-                     terminated_timeout=10, print_to_console=True):
-    """Run a command on a chromeos box"""
+  def CrosRunCommandGeneric(self, cmd, return_output=False, machine=None,
+                            command_terminator=None,
+                            chromeos_root=None, command_timeout=None,
+                            terminated_timeout=10, print_to_console=True):
+    """Run a command on a ChromeOS box.
+
+    Returns triplet (returncode, stdout, stderr).
+    """
 
     if self.log_level != "verbose":
       print_to_console = False
@@ -225,16 +252,16 @@ class CommandExecuter(object):
       if self.logger:
         self.logger.LogError("Could not run remote command on machine."
                              " Is the machine up?")
-      return retval
+      return (retval, "", "")
 
     command = self.RemoteAccessInitCommand(chromeos_root, machine)
     command += "\nremote_sh bash %s" % command_file
     command += "\nl_retval=$?; echo \"$REMOTE_OUT\"; exit $l_retval"
-    retval = self.RunCommand(command, return_output,
-                             command_terminator=command_terminator,
-                             command_timeout=command_timeout,
-                             terminated_timeout=terminated_timeout,
-                             print_to_console=print_to_console)
+    retval = self.RunCommandGeneric(command, return_output,
+                                    command_terminator=command_terminator,
+                                    command_timeout=command_timeout,
+                                    terminated_timeout=terminated_timeout,
+                                    print_to_console=print_to_console)
     if return_output:
       connect_signature = ("Initiating first contact with remote host\n" +
                            "Connection OK\n")
@@ -244,11 +271,39 @@ class CommandExecuter(object):
       return modded_retval
     return retval
 
-  def ChrootRunCommand(self, chromeos_root, command, return_output=False,
-                       command_terminator=None, command_timeout=None,
-                       terminated_timeout=10,
-                       print_to_console=True,
-                       cros_sdk_options=""):
+  def CrosRunCommand(self, *args, **kwargs):
+    """Run a command on a ChromeOS box.
+
+    Takes the same arguments as CrosRunCommandGeneric except for return_output.
+    Returns a single value returncode.
+    """
+    # Make sure that args does not overwrite 'return_output'
+    assert len(args) <= 1
+    assert 'return_output' not in kwargs
+    kwargs['return_output'] = False
+    return self.CrosRunCommandGeneric(*args, **kwargs)[0]
+
+  def CrosRunCommandWOutput(self, *args, **kwargs):
+    """Run a command on a ChromeOS box.
+
+    Takes the same arguments as CrosRunCommandGeneric except for return_output.
+    Returns a triplet (returncode, stdout, stderr).
+    """
+    # Make sure that args does not overwrite 'return_output'
+    assert len(args) <= 1
+    assert 'return_output' not in kwargs
+    kwargs['return_output'] = True
+    return self.CrosRunCommandGeneric(*args, **kwargs)
+
+  def ChrootRunCommandGeneric(self, chromeos_root, command, return_output=False,
+                              command_terminator=None, command_timeout=None,
+                              terminated_timeout=10, print_to_console=True,
+                              cros_sdk_options=""):
+    """Runs a command within the chroot.
+
+    Returns triplet (returncode, stdout, stderr).
+    """
+
 
     if self.log_level != "verbose":
       print_to_console = False
@@ -283,20 +338,44 @@ class CommandExecuter(object):
                 cros_sdk_options,
                 misc.CHROMEOS_SCRIPTS_DIR,
                 os.path.basename(command_file)))
-    ret = self.RunCommand(command, return_output,
-                          command_terminator=command_terminator,
-                          command_timeout=command_timeout,
-                          terminated_timeout=terminated_timeout,
-                          print_to_console=print_to_console)
+    ret = self.RunCommandGeneric(command, return_output,
+                                 command_terminator=command_terminator,
+                                 command_timeout=command_timeout,
+                                 terminated_timeout=terminated_timeout,
+                                 print_to_console=print_to_console)
     os.remove(command_file)
     return ret
 
+  def ChrootRunCommand(self, *args, **kwargs):
+    """Runs a command within the chroot.
 
-  def RunCommands(self, cmdlist, return_output=False, machine=None,
+    Takes the same arguments as ChrootRunCommandGeneric except for
+    return_output.
+    Returns a single value returncode.
+    """
+    # Make sure that args does not overwrite 'return_output'
+    assert len(args) <= 2
+    assert 'return_output' not in kwargs
+    kwargs['return_output'] = False
+    return self.ChrootRunCommandGeneric(*args, **kwargs)[0]
+
+  def ChrootRunCommandWOutput(self, *args, **kwargs):
+    """Runs a command within the chroot.
+
+    Takes the same arguments as ChrootRunCommandGeneric except for
+    return_output.
+    Returns a triplet (returncode, stdout, stderr).
+    """
+    # Make sure that args does not overwrite 'return_output'
+    assert len(args) <= 2
+    assert 'return_output' not in kwargs
+    kwargs['return_output'] = True
+    return self.ChrootRunCommandGeneric(*args, **kwargs)
+
+  def RunCommands(self, cmdlist, machine=None,
                   username=None, command_terminator=None):
     cmd = " ;\n" .join(cmdlist)
-    return self.RunCommand(cmd, return_output, machine, username,
-                           command_terminator)
+    return self.RunCommand(cmd, machine, username, command_terminator)
 
   def CopyFiles(self, src, dest, src_machine=None, dest_machine=None,
                 src_user=None, dest_user=None, recursive=True,
@@ -484,10 +563,10 @@ class MockCommandExecuter(CommandExecuter):
   def __init__(self, log_level, logger_to_set=None):
     super(MockCommandExecuter, self).__init__(log_level, logger_to_set)
 
-  def RunCommand(self, cmd, return_output=False, machine=None,
-                 username=None, command_terminator=None,
-                 command_timeout=None, terminated_timeout=10,
-                 print_to_console=True):
+  def RunCommandGeneric(self, cmd, return_output=False, machine=None,
+                        username=None, command_terminator=None,
+                        command_timeout=None, terminated_timeout=10,
+                        print_to_console=True):
     assert not command_timeout
     cmd = str(cmd)
     if machine is None:
@@ -496,8 +575,17 @@ class MockCommandExecuter(CommandExecuter):
       username = "current"
     logger.GetLogger().LogCmd("(Mock) " + cmd, machine,
                               username, print_to_console)
-    return 0
+    return (0, "", "")
+
+  def RunCommand(self, *args, **kwargs):
+    assert 'return_output' not in kwargs
+    kwargs['return_output'] = False
+    return self.RunCommandGeneric(*args, **kwargs)[0]
 
+  def RunCommandWOutput(self, *args, **kwargs):
+    assert 'return_output' not in kwargs
+    kwargs['return_output'] = True
+    return self.RunCommandGeneric(*args, **kwargs)
 
 class CommandTerminator(object):
   """Object to request termination of a command in execution."""
diff --git a/utils/file_utils.py b/utils/file_utils.py
index 74573344..bb431f19 100644
--- a/utils/file_utils.py
+++ b/utils/file_utils.py
@@ -31,7 +31,7 @@ class FileUtils(object):
   def Md5File(self, filename, log_level="verbose", block_size=2 ** 10):
     command = "md5sum %s" % filename
     ce = command_executer.GetCommandExecuter(log_level=log_level)
-    ret, out, err = ce.RunCommand(command, return_output=True)
+    ret, out, err = ce.RunCommandWOutput(command)
     if ret:
       raise Exception("Could not run md5sum on: %s" % filename)
 
diff --git a/utils/misc.py b/utils/misc.py
index 05cfffa8..b4f7d4b4 100644
--- a/utils/misc.py
+++ b/utils/misc.py
@@ -29,8 +29,7 @@ def GetChromeOSVersionFromLSBVersion(lsb_version):
   ce = command_executer.GetCommandExecuter()
   command = ("git ls-remote "
              "https://chromium.googlesource.com/chromiumos/manifest.git")
-  ret, out, _ = ce.RunCommand(command, return_output=True,
-                              print_to_console=False)
+  ret, out, _ = ce.RunCommandWOutput(command, print_to_console=False)
   assert ret == 0, "Command %s failed" % command
   lower = []
   for line in out.splitlines():
@@ -219,9 +218,7 @@ def GetCtargetFromBoard(board, chromeos_root):
   command = ("source %s; get_ctarget_from_board %s" %
              (TOOLCHAIN_UTILS_PATH, base_board))
   ce = command_executer.GetCommandExecuter()
-  ret, out, _ = ce.ChrootRunCommand(chromeos_root,
-                                    command,
-                                    return_output=True)
+  ret, out, _ = ce.ChrootRunCommandWOutput(chromeos_root, command)
   if ret != 0:
     raise ValueError("Board %s is invalid!" % board)
   # Remove ANSI escape sequences.
@@ -235,9 +232,7 @@ def GetArchFromBoard(board, chromeos_root):
   command = ("source %s; get_board_arch %s" %
              (TOOLCHAIN_UTILS_PATH, base_board))
   ce = command_executer.GetCommandExecuter()
-  ret, out, _ = ce.ChrootRunCommand(chromeos_root,
-                                    command,
-                                    return_output=True)
+  ret, out, _ = ce.ChrootRunCommandWOutput(chromeos_root, command)
   if ret != 0:
     raise ValueError("Board %s is invalid!" % board)
   # Remove ANSI escape sequences.
@@ -285,7 +280,7 @@ def MergeEnvStringWithDict(env_string, env_dict, prepend=True):
     else:
       new_env = "%s=\"$%s %s\"" % (k, k, v)
     command = "; ".join([env_string, new_env, "echo $%s" % k])
-    ret, out, _ = ce.RunCommand(command, return_output=True)
+    ret, out, _ = ce.RunCommandWOutput(command)
     override_env_list.append("%s=%r" % (k, out.strip()))
   ret = env_string + " " + " ".join(override_env_list)
   return ret.strip()
@@ -295,7 +290,7 @@ def GetAllImages(chromeos_root, board):
   ce = command_executer.GetCommandExecuter()
   command = ("find %s/src/build/images/%s -name chromiumos_test_image.bin" %
              (chromeos_root, board))
-  ret, out, _ = ce.RunCommand(command, return_output=True)
+  ret, out, _ = ce.RunCommandWOutput(command)
   assert ret == 0, "Could not run command: %s" % command
   return out.splitlines()
 
@@ -380,8 +375,8 @@ def GitGetCommitHash(git_dir, commit_symbolic_name):
 
   command = ('cd {0} && git log -n 1 --pretty="format:%H" {1}').format(
       git_dir, commit_symbolic_name)
-  rv, out, _ = command_executer.GetCommandExecuter().RunCommand(
-      command, return_output=True, print_to_console=False)
+  rv, out, _ = command_executer.GetCommandExecuter().RunCommandWOutput(
+      command, print_to_console=False)
   if rv == 0:
     return out.strip()
   return None
@@ -422,8 +417,8 @@ def GetGitChangesAsList(git_dir, path=None, staged=False):
     command += " --cached"
   if path:
     command += " -- " + path
-  _, out, _ = command_executer.GetCommandExecuter().RunCommand(
-      command, return_output=True, print_to_console=False)
+  _, out, _ = command_executer.GetCommandExecuter().RunCommandWOutput(
+      command, print_to_console=False)
   rv = []
   for line in out.splitlines():
     rv.append(line)
@@ -457,7 +452,7 @@ def DeleteChromeOsTree(chromeos_root, dry_run=False):
     print cmd0
   else:
     if command_executer.GetCommandExecuter().RunCommand(
-        cmd0, return_output=False, print_to_console=True) != 0:
+        cmd0, print_to_console=True) != 0:
       return False
 
   cmd1 = ('export CHROMEOSDIRNAME="$(dirname $(cd {0} && pwd))" && '
@@ -469,7 +464,7 @@ def DeleteChromeOsTree(chromeos_root, dry_run=False):
     return True
 
   return command_executer.GetCommandExecuter().RunCommand(
-      cmd1, return_output=False, print_to_console=True) == 0
+      cmd1, print_to_console=True) == 0
 
 
 def ApplyGerritPatches(chromeos_root,