diff --git a/PRESUBMIT.py b/PRESUBMIT.py index 127cc906c..ebe5061de 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -85,7 +85,8 @@ def RunGitClTests(input_api, output_api): [input_api.os_path.join(test_path, test)], cwd=test_path) else: input_api.subprocess.check_output( - [input_api.os_path.join(test_path, test)], cwd=test_path) + [input_api.os_path.join(test_path, test)], cwd=test_path, + stderr=input_api.subprocess.STDOUT) except (OSError, input_api.subprocess.CalledProcessError), e: results.append(output_api.PresubmitError('%s failed\n%s' % (test, e))) except local_rietveld.Failure, e: diff --git a/checkout.py b/checkout.py index a8fdcd2fc..5f61d6f16 100644 --- a/checkout.py +++ b/checkout.py @@ -140,6 +140,7 @@ class RawCheckout(CheckoutBase): stdout = subprocess2.check_output( ['patch', '-p%s' % p.patchlevel], stdin=p.get(), + stderr=subprocess2.STDOUT, cwd=self.project_path) elif p.is_new and not os.path.exists(filepath): # There is only a header. Just create the file. @@ -218,7 +219,9 @@ class SvnMixIn(object): """ kwargs.setdefault('cwd', self.project_path) return subprocess2.check_output( - self._add_svn_flags(args, True, credentials), **kwargs) + self._add_svn_flags(args, True, credentials), + stderr=subprocess2.STDOUT, + **kwargs) @staticmethod def _parse_svn_info(output, key): @@ -496,7 +499,8 @@ class GitCheckoutBase(CheckoutBase): def _check_output_git(self, args, **kwargs): kwargs.setdefault('cwd', self.project_path) - return subprocess2.check_output(['git'] + args, **kwargs) + return subprocess2.check_output( + ['git'] + args, stderr=subprocess2.STDOUT, **kwargs) def _branches(self): """Returns the list of branches and the active one.""" diff --git a/gclient_scm.py b/gclient_scm.py index 15e9eb9b2..4500dffc8 100644 --- a/gclient_scm.py +++ b/gclient_scm.py @@ -665,7 +665,9 @@ class GitWrapper(SCMWrapper): def _Capture(self, args): return subprocess2.check_output( - ['git'] + args, cwd=self.checkout_path).strip() + ['git'] + args, + stderr=subprocess2.PIPE, + cwd=self.checkout_path).strip() def _Run(self, args, options, **kwargs): kwargs.setdefault('cwd', self.checkout_path) diff --git a/presubmit_canned_checks.py b/presubmit_canned_checks.py index 0b79fc00b..aeb271145 100644 --- a/presubmit_canned_checks.py +++ b/presubmit_canned_checks.py @@ -515,7 +515,9 @@ def RunUnitTests(input_api, output_api, unit_tests): input_api.subprocess.check_call(cmd, cwd=input_api.PresubmitLocalPath()) else: input_api.subprocess.check_output( - cmd, cwd=input_api.PresubmitLocalPath()) + cmd, + stderr=input_api.subprocess.STDOUT, + cwd=input_api.PresubmitLocalPath()) except (OSError, input_api.subprocess.CalledProcessError), e: results.append(message_type('%s failed!\n%s' % (unit_test, e))) return results @@ -558,7 +560,8 @@ def RunPythonUnitTests(input_api, output_api, unit_tests): env['PYTHONPATH'] = input_api.os_path.pathsep.join((backpath)) cmd = [input_api.python_executable, '-m', '%s' % unit_test] try: - input_api.subprocess.check_output(cmd, cwd=cwd, env=env) + input_api.subprocess.check_output( + cmd, stderr=input_api.subprocess.STDOUT, cwd=cwd, env=env) except (OSError, input_api.subprocess.CalledProcessError), e: results.append(message_type('%s failed!\n%s' % (unit_test_name, e))) return results diff --git a/scm.py b/scm.py index 80e5fc4a5..446c94add 100644 --- a/scm.py +++ b/scm.py @@ -79,6 +79,7 @@ def determine_scm(root): subprocess2.check_output( ['git', 'rev-parse', '--show-cdup'], stdout=subprocess2.VOID, + stderr=subprocess2.VOID, cwd=root) return 'git' except (OSError, subprocess2.CalledProcessError): diff --git a/subprocess2.py b/subprocess2.py index 5678127af..034463fd2 100644 --- a/subprocess2.py +++ b/subprocess2.py @@ -249,6 +249,8 @@ def call(args, **kwargs): """Emulates subprocess.call(). Automatically convert stdout=PIPE or stderr=PIPE to VOID. + In no case they can be returned since no code path raises + subprocess2.CalledProcessError. """ if kwargs.get('stdout') == PIPE: kwargs['stdout'] = VOID @@ -281,16 +283,12 @@ def capture(args, **kwargs): Returns stdout. - Discards returncode. - - Discards stderr. By default sets stderr=STDOUT. - - Blocks stdin by default since no output will be visible. + - Blocks stdin by default if not specified since no output will be visible. """ - if kwargs.get('stdin') is None: - kwargs['stdin'] = VOID - if kwargs.get('stdout') is None: - kwargs['stdout'] = PIPE - if kwargs.get('stderr') is None: - kwargs['stderr'] = STDOUT - return communicate(args, **kwargs)[0][0] + kwargs.setdefault('stdin', VOID) + + # Like check_output, deny the caller from using stdout arg. + return communicate(args, stdout=PIPE, **kwargs)[0][0] def check_output(args, **kwargs): @@ -298,15 +296,10 @@ def check_output(args, **kwargs): Captures stdout of a process call and returns stdout only. - - Discards stderr. By default sets stderr=STDOUT. - Throws if return code is not 0. - Works even prior to python 2.7. - - Blocks stdin by default since no output will be visible. + - Blocks stdin by default if not specified since no output will be visible. + - As per doc, "The stdout argument is not allowed as it is used internally." """ - if kwargs.get('stdin') is None: - kwargs['stdin'] = VOID - if kwargs.get('stdout') is None: - kwargs['stdout'] = PIPE - if kwargs.get('stderr') is None: - kwargs['stderr'] = STDOUT - return check_call_out(args, **kwargs)[0] + kwargs.setdefault('stdin', VOID) + return check_call_out(args, stdout=PIPE, **kwargs)[0] diff --git a/tests/subprocess2_test.py b/tests/subprocess2_test.py index 031b6d984..d565cc0a7 100755 --- a/tests/subprocess2_test.py +++ b/tests/subprocess2_test.py @@ -34,6 +34,8 @@ class Subprocess2Test(unittest.TestCase): for module, names in self.TO_SAVE.iteritems(): self.saved[module] = dict( (name, getattr(module, name)) for name in names) + # TODO(maruel): Do a reopen() on sys.__stdout__ and sys.__stderr__ so they + # can be trapped in the child process for better coverage. def tearDown(self): for module, saved in self.saved.iteritems(): @@ -89,6 +91,18 @@ class Subprocess2Test(unittest.TestCase): } self.assertEquals(expected, results) + def test_capture_defaults(self): + results = self._fake_communicate() + self.assertEquals( + 'stdout', subprocess2.capture(['foo'], a=True)) + expected = { + 'args': ['foo'], + 'a':True, + 'stdin': subprocess2.VOID, + 'stdout': subprocess2.PIPE, + } + self.assertEquals(expected, results) + def test_communicate_defaults(self): results = self._fake_Popen() self.assertEquals( @@ -129,7 +143,6 @@ class Subprocess2Test(unittest.TestCase): 'a':True, 'stdin': subprocess2.VOID, 'stdout': subprocess2.PIPE, - 'stderr': subprocess2.STDOUT, } self.assertEquals(expected, results) @@ -142,22 +155,69 @@ class Subprocess2Test(unittest.TestCase): self.assertEquals(subprocess2.TIMED_OUT, returncode) self.assertEquals(['', None], out) - def test_void(self): - out = subprocess2.check_output( + def test_check_output_no_stdout(self): + try: + subprocess2.check_output(self.exe, stdout=subprocess2.PIPE) + self.fail() + except TypeError: + pass + + def test_stdout_void(self): + (out, err), code = subprocess2.communicate( self.exe + ['--stdout', '--stderr'], - stdout=subprocess2.VOID) + stdout=subprocess2.VOID, + stderr=subprocess2.PIPE) self.assertEquals(None, out) - out = subprocess2.check_output( + self.assertEquals('a\nbb\nccc\n', err) + self.assertEquals(0, code) + + def test_stderr_void(self): + (out, err), code = subprocess2.communicate( self.exe + ['--stdout', '--stderr'], - universal_newlines=True, + universal_newlines=True, + stdout=subprocess2.PIPE, stderr=subprocess2.VOID) self.assertEquals('A\nBB\nCCC\n', out) + self.assertEquals(None, err) + self.assertEquals(0, code) + + def test_check_output_throw_stdout(self): + try: + subprocess2.check_output( + self.exe + ['--fail', '--stdout'], universal_newlines=True) + self.fail() + except subprocess2.CalledProcessError, e: + self.assertEquals('A\nBB\nCCC\n', e.stdout) + self.assertEquals(None, e.stderr) + self.assertEquals(64, e.returncode) - def test_check_output_throw(self): + def test_check_output_throw_no_stderr(self): try: subprocess2.check_output( self.exe + ['--fail', '--stderr'], universal_newlines=True) self.fail() + except subprocess2.CalledProcessError, e: + self.assertEquals('', e.stdout) + self.assertEquals(None, e.stderr) + self.assertEquals(64, e.returncode) + + def test_check_output_throw_stderr(self): + try: + subprocess2.check_output( + self.exe + ['--fail', '--stderr'], stderr=subprocess2.PIPE, + universal_newlines=True) + self.fail() + except subprocess2.CalledProcessError, e: + self.assertEquals('', e.stdout) + self.assertEquals('a\nbb\nccc\n', e.stderr) + self.assertEquals(64, e.returncode) + + def test_check_output_throw_stderr_stdout(self): + try: + subprocess2.check_output( + self.exe + ['--fail', '--stderr'], stderr=subprocess2.STDOUT, + universal_newlines=True) + self.fail() except subprocess2.CalledProcessError, e: self.assertEquals('a\nbb\nccc\n', e.stdout) self.assertEquals(None, e.stderr)