Change output_of to execute commands with shell_tools.run

dev_tools/git_env_tools.py

@@ -22,7 +22,7 @@
 
 def get_repo_root() -> str:
     """Get the root of the git repository the cwd is within."""
-    return shell_tools.output_of('git', 'rev-parse', '--show-toplevel')
+    return shell_tools.output_of(['git', 'rev-parse', '--show-toplevel'])
 
 
 def _git_fetch_for_comparison(
@@ -60,7 +60,7 @@ def _git_fetch_for_comparison(
             depth_str,
             log_run_to_stderr=verbose,
         )
-        actual_id = shell_tools.output_of('git', 'rev-parse', 'FETCH_HEAD')
+        actual_id = shell_tools.output_of(['git', 'rev-parse', 'FETCH_HEAD'])
 
         shell_tools.run_cmd(
             'git',
@@ -71,10 +71,10 @@ def _git_fetch_for_comparison(
             depth_str,
             log_run_to_stderr=verbose,
         )
-        base_id = shell_tools.output_of('git', 'rev-parse', 'FETCH_HEAD')
+        base_id = shell_tools.output_of(['git', 'rev-parse', 'FETCH_HEAD'])
 
         try:
-            base_id = shell_tools.output_of('git', 'merge-base', actual_id, base_id)
+            base_id = shell_tools.output_of(['git', 'merge-base', actual_id, base_id])
             break
         except subprocess.CalledProcessError:
             # No common ancestor. We need to dig deeper.
@@ -170,7 +170,7 @@ def fetch_local_files(destination_directory: str, verbose: bool) -> prepared_env
             log_run_to_stderr=verbose,
         )
 
-        cur_commit = shell_tools.output_of('git', 'rev-parse', 'HEAD')
+        cur_commit = shell_tools.output_of(['git', 'rev-parse', 'HEAD'])
 
         os.chdir(destination_directory)
         if verbose:

dev_tools/incremental_coverage.py

@@ -144,8 +144,9 @@ def get_incremental_uncovered_lines(
     if not os.path.isfile(abs_path):
         return []
 
+    optional_actual_commit = [] if actual_commit is None else [actual_commit]
     unified_diff_lines_str = shell_tools.output_of(
-        'git', 'diff', '--unified=0', base_commit, actual_commit, '--', abs_path
+        ['git', 'diff', '--unified=0', base_commit, *optional_actual_commit, '--', abs_path]
     )
     unified_diff_lines = [e for e in unified_diff_lines_str.split('\n') if e.strip()]
 

dev_tools/prepared_env.py

@@ -121,13 +121,16 @@ def get_changed_files(self) -> List[str]:
             List[str]: File paths of changed files, relative to the git repo
                 root.
         """
+        optional_actual_commit_id = [] if self.actual_commit_id is None else [self.actual_commit_id]
         out = shell_tools.output_of(
-            'git',
-            'diff',
-            '--name-only',
-            self.compare_commit_id,
-            self.actual_commit_id,
-            '--',
+            [
+                'git',
+                'diff',
+                '--name-only',
+                self.compare_commit_id,
+                *optional_actual_commit_id,
+                '--',
+            ],
             cwd=self.destination_directory,
         )
         return [e for e in out.split('\n') if e.strip()]

dev_tools/shell_tools.py

@@ -285,24 +285,26 @@ def run_shell(
     return result
 
 
-def output_of(*cmd: Optional[str], **kwargs) -> str:
+def output_of(args: Union[str, List[str]], **kwargs) -> str:
     """Invokes a subprocess and returns its output as a string.
 
     Args:
-        *cmd: Components of the command to execute, e.g. ["echo", "dog"].
-        **kwargs: Extra arguments for asyncio.create_subprocess_shell, such as
+        args: The arguments for launching the process.  This may be a list
+            or a string, for example, ["echo", "dog"] or "pwd".  The string
+            type may need to be used with ``shell=True`` to allow invocation
+            as a shell command, otherwise the string is used as a command name
+            with no arguments.
+        **kwargs: Extra arguments for the shell_tools.run function, such as
             a cwd (current working directory) argument.
 
     Returns:
-        A (captured output, captured error output, return code) triplet. The
-        captured outputs will be None if the out or err parameters were not set
-        to an instance of TeeCapture.
+        str: The standard output of the command with the last newline removed.
 
     Raises:
          subprocess.CalledProcessError: The process returned a non-zero error
-            code and raise_on_fail was set.
+            code and the `check` flag was True (default).
     """
-    result = cast(str, run_cmd(*cmd, log_run_to_stderr=False, out=TeeCapture(), **kwargs).out)
+    result = run(args, log_run_to_stderr=False, stdout=subprocess.PIPE, **kwargs).stdout
 
     # Strip final newline.
     if result.endswith('\n'):

dev_tools/shell_tools_test.py

@@ -110,5 +110,8 @@ def test_output_of():
     assert shell_tools.output_of('true') == ''
     with pytest.raises(subprocess.CalledProcessError):
         _ = shell_tools.output_of('false')
-    assert shell_tools.output_of('echo', 'test') == 'test'
+    assert shell_tools.output_of(['echo', 'test']) == 'test'
+    # filtering of the None arguments was removed.  check this now fails
+    with pytest.raises(TypeError):
+        _ = shell_tools.output_of(['echo', 'test', None, 'duck'])
     assert shell_tools.output_of('pwd', cwd='/tmp') in ['/tmp', '/private/tmp']