From 46fc5ccf45924dc368edc42db7fc87149388767e Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Tue, 7 May 2019 06:51:42 -0400 Subject: [PATCH 1/3] Add failing test. --- tests/lib/test_lib.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/lib/test_lib.py b/tests/lib/test_lib.py index 4b7dc0936c2..81b50baec73 100644 --- a/tests/lib/test_lib.py +++ b/tests/lib/test_lib.py @@ -80,7 +80,7 @@ def test_as_import(script): class TestPipTestEnvironment: - def run_with_log_command(self, script, sub_string): + def run_with_log_command(self, script, sub_string, **kwargs): """ Call run() on a command that logs a "%"-style format string using the given substring as the string's replacement field. @@ -90,7 +90,7 @@ def run_with_log_command(self, script, sub_string): "logging.getLogger().info('sub: {}', 'foo')" ).format(sub_string) args = [sys.executable, '-c', command] - script.run(*args) + script.run(*args, **kwargs) def run_stderr_with_prefix(self, script, prefix, **kwargs): """ @@ -162,8 +162,13 @@ def test_run__logging_error(self, script): expected_start = 'stderr has a logging error, which is never allowed' with assert_error_startswith(RuntimeError, expected_start): - # Pass a bad substitution string. - self.run_with_log_command(script, sub_string='{!r}') + # Pass a bad substitution string. Also, pass + # allow_stderr_error=True to check that the RuntimeError occurs + # even under the stricter test condition of when we are allowing + # other types of errors. + self.run_with_log_command( + script, sub_string='{!r}', allow_stderr_error=True, + ) def test_run__allow_stderr_error_false_error_with_expect_error( self, script, From 8a9e36ee154c915dda57a784d08b1406f9637172 Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Tue, 7 May 2019 06:58:38 -0400 Subject: [PATCH 2/3] Change to never allow logging errors during tests. --- tests/lib/__init__.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/tests/lib/__init__.py b/tests/lib/__init__.py index 6ee0808d094..b57f49d7bb7 100644 --- a/tests/lib/__init__.py +++ b/tests/lib/__init__.py @@ -294,26 +294,28 @@ def check_stderr( if allow_stderr_warning is None: allow_stderr_warning = allow_stderr_error - if allow_stderr_error: - # Then any stderr is acceptable. - if not allow_stderr_warning: - raise RuntimeError( - 'cannot pass allow_stderr_warning=False with ' - 'allow_stderr_error=True' - ) - return + if allow_stderr_error and not allow_stderr_warning: + raise RuntimeError( + 'cannot pass allow_stderr_warning=False with ' + 'allow_stderr_error=True' + ) lines = stderr.splitlines() for line in lines: - # First check for logging errors which are sent directly to stderr - # and so bypass any configured log formatter. The - # "--- Logging error ---" string is used in Python 3.4+, and + # First check for logging errors, which we don't allow during + # tests even if allow_stderr_error=True (since a logging error + # would signal a bug in pip's code). + # Unlike errors logged with logger.error(), these errors are + # sent directly to stderr and so bypass any configured log formatter. + # The "--- Logging error ---" string is used in Python 3.4+, and # "Logged from file " is used in Python 2. if (line.startswith('--- Logging error ---') or line.startswith('Logged from file ')): reason = 'stderr has a logging error, which is never allowed' msg = make_check_stderr_message(stderr, line=line, reason=reason) raise RuntimeError(msg) + if allow_stderr_error: + continue if line.startswith('ERROR: '): reason = ( From 0b8377d53f7de097c40736108d13564e748bf328 Mon Sep 17 00:00:00 2001 From: Chris Jerdonek Date: Tue, 7 May 2019 06:59:25 -0400 Subject: [PATCH 3/3] Move run_with_log_command() after run_stderr_with_prefix(). We do this to better parallel the test order in TestPipTestEnvironment, since the tests using run_with_log_command() occur later. --- tests/lib/test_lib.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/lib/test_lib.py b/tests/lib/test_lib.py index 81b50baec73..dc50196bdc0 100644 --- a/tests/lib/test_lib.py +++ b/tests/lib/test_lib.py @@ -80,6 +80,15 @@ def test_as_import(script): class TestPipTestEnvironment: + def run_stderr_with_prefix(self, script, prefix, **kwargs): + """ + Call run() that prints stderr with the given prefix. + """ + text = '{}: hello, world\\n'.format(prefix) + command = 'import sys; sys.stderr.write("{}")'.format(text) + args = [sys.executable, '-c', command] + script.run(*args, **kwargs) + def run_with_log_command(self, script, sub_string, **kwargs): """ Call run() on a command that logs a "%"-style format string using @@ -92,15 +101,6 @@ def run_with_log_command(self, script, sub_string, **kwargs): args = [sys.executable, '-c', command] script.run(*args, **kwargs) - def run_stderr_with_prefix(self, script, prefix, **kwargs): - """ - Call run() that prints stderr with the given prefix. - """ - text = '{}: hello, world\\n'.format(prefix) - command = 'import sys; sys.stderr.write("{}")'.format(text) - args = [sys.executable, '-c', command] - script.run(*args, **kwargs) - @pytest.mark.parametrize('prefix', ( 'DEBUG', 'INFO',