[Infra] Update execute function in utils.py (#3319)

infra/build_specified_commit.py

@@ -88,7 +88,7 @@ def detect_main_repo(project_name, repo_name=None, commit=None):
     command_to_run.extend(['--repo_name', repo_name])
   else:
     command_to_run.extend(['--example_commit', commit])
-  out, _ = utils.execute(command_to_run)
+  out, _, _ = utils.execute(command_to_run)
   match = re.search(r'\bDetected repo: ([^ ]+) ([^ ]+)', out.rstrip())
   if match and match.group(1) and match.group(2):
     return match.group(1), match.group(2)

infra/repo_manager.py

@@ -63,8 +63,8 @@ class RepoManager:
     if not os.path.exists(self.base_dir):
       os.makedirs(self.base_dir)
     self.remove_repo()
-    out, err = utils.execute(['git', 'clone', self.repo_url, self.repo_name],
-                             location=self.base_dir)
+    out, _, _ = utils.execute(['git', 'clone', self.repo_url, self.repo_name],
+                              location=self.base_dir)
     if not self._is_git_repo():
       raise ValueError('%s is not a git repo' % self.repo_url)
 
@@ -89,8 +89,8 @@ class RepoManager:
     if not commit.rstrip():
       return False
 
-    _, err_code = utils.execute(['git', 'cat-file', '-e', commit],
-                                self.repo_dir)
+    _, _, err_code = utils.execute(['git', 'cat-file', '-e', commit],
+                                   self.repo_dir)
     return not err_code
 
   def get_current_commit(self):
@@ -99,9 +99,9 @@ class RepoManager:
     Returns:
       The current active commit SHA.
     """
-    out, _ = utils.execute(['git', 'rev-parse', 'HEAD'],
-                           self.repo_dir,
-                           check_result=True)
+    out, _, _ = utils.execute(['git', 'rev-parse', 'HEAD'],
+                              self.repo_dir,
+                              check_result=True)
     return out.strip('\n')
 
   def get_commit_list(self, old_commit, new_commit):
@@ -125,7 +125,7 @@ class RepoManager:
       raise ValueError('The new commit %s does not exist' % new_commit)
     if old_commit == new_commit:
       return [old_commit]
-    out, err_code = utils.execute(
+    out, _, err_code = utils.execute(
         ['git', 'rev-list', old_commit + '..' + new_commit], self.repo_dir)
     commits = out.split('\n')
     commits = [commit for commit in commits if commit]

infra/utils.py

@@ -13,6 +13,7 @@
 # limitations under the License.
 """Utilities for OSS-Fuzz infrastructure."""
 
+import logging
 import os
 import re
 import stat
@@ -41,7 +42,7 @@ def execute(command, location=None, check_result=False):
     check_result: Should an exception be thrown on failed command.
 
   Returns:
-    The stdout of the command, the error code.
+    stdout, stderr, error code.
 
   Raises:
     RuntimeError: running a command resulted in an error.
@@ -49,14 +50,20 @@ def execute(command, location=None, check_result=False):
 
   if not location:
     location = os.getcwd()
-  process = subprocess.Popen(command, stdout=subprocess.PIPE, cwd=location)
+  process = subprocess.Popen(command,
+                             stdout=subprocess.PIPE,
+                             stderr=subprocess.PIPE,
+                             cwd=location)
   out, err = process.communicate()
-  if check_result and (process.returncode or err):
-    raise RuntimeError('Error: %s\n Command: %s\n Return code: %s\n Out: %s' %
-                       (err, command, process.returncode, out))
-  if out is not None:
-    out = out.decode('ascii').rstrip()
-  return out, process.returncode
+  out = out.decode('ascii')
+  err = err.decode('ascii')
+  if err:
+    logging.debug('Stderr of command \'%s\' is %s.', ' '.join(command), err)
+  if check_result and process.returncode:
+    raise RuntimeError(
+        'Executing command \'{0}\' failed with error: {1}.'.format(
+            ' '.join(command), err))
+  return out, err, process.returncode
 
 
 def get_fuzz_targets(path):

infra/utils_test.py

@@ -11,13 +11,10 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-"""Test the functionality of the utils module's functions:
-1. is_fuzz_target_local
-2. get_fuzz_targets
-3. get_env_var
-"""
+"""Test the functionality of the utils module's functions"""
 
 import os
+import tempfile
 import unittest
 
 import utils
@@ -98,5 +95,45 @@ class GetFuzzTargetsUnitTest(unittest.TestCase):
     self.assertFalse(fuzz_targets)
 
 
+class ExecuteUnitTest(unittest.TestCase):
+  """Test execute function in the utils module."""
+
+  def test_valid_command(self):
+    """Tests that execute can produce valid output."""
+    with tempfile.TemporaryDirectory() as tmp_dir:
+      out, err, err_code = utils.execute(['ls', '.'],
+                                         location=tmp_dir,
+                                         check_result=False)
+      self.assertEqual(err_code, 0)
+      self.assertEqual(err, '')
+      self.assertEqual(out, '')
+      out, err, err_code = utils.execute(['mkdir', 'tmp'],
+                                         location=tmp_dir,
+                                         check_result=False)
+      self.assertEqual(err_code, 0)
+      self.assertEqual(err, '')
+      self.assertEqual(out, '')
+      out, err, err_code = utils.execute(['ls', '.'],
+                                         location=tmp_dir,
+                                         check_result=False)
+      self.assertEqual(err_code, 0)
+      self.assertEqual(err, '')
+      self.assertEqual(out, 'tmp\n')
+
+  def test_error_command(self):
+    """Tests that execute can correctly surface errors."""
+    with tempfile.TemporaryDirectory() as tmp_dir:
+      out, err, err_code = utils.execute(['ls', 'notarealdir'],
+                                         location=tmp_dir,
+                                         check_result=False)
+      self.assertEqual(err_code, 2)
+      self.assertIsNotNone(err)
+      self.assertEqual(out, '')
+      with self.assertRaises(RuntimeError):
+        out, err, err_code = utils.execute(['ls', 'notarealdir'],
+                                           location=tmp_dir,
+                                           check_result=True)
+
+
 if __name__ == '__main__':
   unittest.main()