diff --git a/Lib/test/regrtest.py b/Lib/test/regrtest.py index a544d679cf4..e808581ba83 100755 --- a/Lib/test/regrtest.py +++ b/Lib/test/regrtest.py @@ -353,6 +353,7 @@ def main(tests=None, testdir=None, verbose=0, quiet=False, bad = [] skipped = [] resource_denieds = [] + environment_changed = [] if findleaks: try: @@ -429,11 +430,13 @@ def main(tests=None, testdir=None, verbose=0, quiet=False, test_times.append((test_time, test)) if ok > 0: good.append(test) - elif ok == 0: + elif -2 < ok <= 0: bad.append(test) + if ok == -1: + environment_changed.append(test) else: skipped.append(test) - if ok == -2: + if ok == -3: resource_denieds.append(test) if use_mp: @@ -539,6 +542,7 @@ def main(tests=None, testdir=None, verbose=0, quiet=False, good.sort() bad.sort() skipped.sort() + environment_changed.sort() if good and not quiet: if not bad and not skipped and len(good) > 1: @@ -553,8 +557,14 @@ def main(tests=None, testdir=None, verbose=0, quiet=False, for time, test in test_times[:10]: print("%s: %.1fs" % (test, time)) if bad: - print(count(len(bad), "test"), "failed:") - printlist(bad) + bad = sorted(set(bad) - set(environment_changed)) + if bad: + print(count(len(bad), "test"), "failed:") + printlist(bad) + if environment_changed: + print("{} altered the execution environment:".format( + count(len(environment_changed), "test"))) + printlist(environment_changed) if skipped and not quiet: print(count(len(skipped), "test"), "skipped:") printlist(skipped) @@ -657,8 +667,10 @@ def runtest(test, verbose, quiet, debug -- if true, print tracebacks for failed tests regardless of verbose setting Return: - -2 test skipped because resource denied - -1 test skipped for some other reason + -4 KeyboardInterrupt when run under -j + -3 test skipped because resource denied + -2 test skipped for some other reason + -1 test failed because it changed the execution environment 0 test failed 1 test passed """ @@ -672,6 +684,118 @@ def runtest(test, verbose, quiet, finally: cleanup_test_droppings(test, verbose) +# Unit tests are supposed to leave the execution environment unchanged +# once they complete. But sometimes tests have bugs, especially when +# tests fail, and the changes to environment go on to mess up other +# tests. This can cause issues with buildbot stability, since tests +# are run in random order and so problems may appear to come and go. +# There are a few things we can save and restore to mitigate this, and +# the following context manager handles this task. + +class saved_test_environment: + """Save bits of the test environment and restore them at block exit. + + with saved_test_environment(testname, verbose, quiet): + #stuff + + Unless quiet is True, a warning is printed to stderr if any of + the saved items was changed by the test. The attribute 'changed' + is initially False, but is set to True if a change is detected. + + If verbose is more than 1, the before and after state of changed + items is also printed. + """ + + changed = False + + def __init__(self, testname, verbose=0, quiet=False): + self.testname = testname + self.verbose = verbose + self.quiet = quiet + + # To add things to save and restore, add a name XXX to the resources list + # and add corresponding get_XXX/restore_XXX functions. get_XXX should + # return the value to be saved and compared against a second call to the + # get function when test execution completes. restore_XXX should accept + # the saved value and restore the resource using it. It will be called if + # and only if a change in the value is detected. + # + # Note: XXX will have any '.' replaced with '_' characters when determining + # the corresponding method names. + + resources = ('sys.argv', 'cwd', 'sys.stdin', 'sys.stdout', 'sys.stderr', + 'os.environ', 'sys.path') + + def get_sys_argv(self): + return id(sys.argv), sys.argv, sys.argv[:] + def restore_sys_argv(self, saved_argv): + sys.argv = saved_argv[1] + sys.argv[:] = saved_argv[2] + + def get_cwd(self): + return os.getcwd() + def restore_cwd(self, saved_cwd): + os.chdir(saved_cwd) + + def get_sys_stdout(self): + return sys.stdout + def restore_sys_stdout(self, saved_stdout): + sys.stdout = saved_stdout + + def get_sys_stderr(self): + return sys.stderr + def restore_sys_stderr(self, saved_stderr): + sys.stderr = saved_stderr + + def get_sys_stdin(self): + return sys.stdin + def restore_sys_stdin(self, saved_stdin): + sys.stdin = saved_stdin + + def get_os_environ(self): + return id(os.environ), os.environ, dict(os.environ) + def restore_os_environ(self, saved_environ): + os.environ = saved_environ[1] + os.environ.clear() + os.environ.update(saved_environ[2]) + + def get_sys_path(self): + return id(sys.path), sys.path, sys.path[:] + def restore_sys_path(self, saved_path): + sys.path = saved_path[1] + sys.path[:] = saved_path[2] + + def resource_info(self): + for name in self.resources: + method_suffix = name.replace('.', '_') + get_name = 'get_' + method_suffix + restore_name = 'restore_' + method_suffix + yield name, getattr(self, get_name), getattr(self, restore_name) + + def __enter__(self): + self.saved_values = dict((name, get()) for name, get, restore + in self.resource_info()) + return self + + def __exit__(self, exc_type, exc_val, exc_tb): + for name, get, restore in self.resource_info(): + current = get() + original = self.saved_values[name] + # Check for changes to the resource's value + if current != original: + self.changed = True + restore(original) + if not self.quiet: + print("Warning -- {} was modified by {}".format( + name, self.testname), + file=sys.stderr) + if self.verbose > 1: + print(" Before: {}\n After: {} ".format( + original, current), + file=sys.stderr) + return False + + def runtest_inner(test, verbose, quiet, testdir=None, huntrleaks=False, debug=False): support.unload(test) @@ -692,18 +816,20 @@ def runtest_inner(test, verbose, quiet, else: # Always import it from the test package abstest = 'test.' + test - start_time = time.time() - the_package = __import__(abstest, globals(), locals(), []) - the_module = getattr(the_package, test) - # Old tests run to completion simply as a side-effect of - # being imported. For tests based on unittest or doctest, - # explicitly invoke their test_main() function (if it exists). - indirect_test = getattr(the_module, "test_main", None) - if indirect_test is not None: - indirect_test() - if huntrleaks: - refleak = dash_R(the_module, test, indirect_test, huntrleaks) - test_time = time.time() - start_time + with saved_test_environment(test, verbose, quiet) as environment: + start_time = time.time() + the_package = __import__(abstest, globals(), locals(), []) + the_module = getattr(the_package, test) + # Old tests run to completion simply as a side-effect of + # being imported. For tests based on unittest or doctest, + # explicitly invoke their test_main() function (if it exists). + indirect_test = getattr(the_module, "test_main", None) + if indirect_test is not None: + indirect_test() + if huntrleaks: + refleak = dash_R(the_module, test, indirect_test, + huntrleaks) + test_time = time.time() - start_time finally: sys.stdout = save_stdout # Restore what we saved if needed, but also complain if the test @@ -721,12 +847,12 @@ def runtest_inner(test, verbose, quiet, if not quiet: print(test, "skipped --", msg) sys.stdout.flush() - return -2, test_time + return -3, test_time except unittest.SkipTest as msg: if not quiet: print(test, "skipped --", msg) sys.stdout.flush() - return -1, test_time + return -2, test_time except KeyboardInterrupt: raise except support.TestFailed as msg: @@ -744,6 +870,8 @@ def runtest_inner(test, verbose, quiet, else: if refleak: return 0, test_time + if environment.changed: + return -1, test_time return 1, test_time def cleanup_test_droppings(testname, verbose): diff --git a/Lib/test/support.py b/Lib/test/support.py index 04964a10958..611d48fe9b1 100644 --- a/Lib/test/support.py +++ b/Lib/test/support.py @@ -567,6 +567,32 @@ class EnvironmentVarGuard(collections.MutableMapping): del self._environ[k] else: self._environ[k] = v + os.environ = self._environ + + +class DirsOnSysPath(object): + """Context manager to temporarily add directories to sys.path. + + This makes a copy of sys.path, appends any directories given + as positional arguments, then reverts sys.path to the copied + settings when the context ends. + + Note that *all* sys.path modifications in the body of the + context manager, including replacement of the object, + will be reverted at the end of the block. + """ + + def __init__(self, *paths): + self.original_value = sys.path[:] + self.original_object = sys.path + sys.path.extend(paths) + + def __enter__(self): + return self + + def __exit__(self, *ignore_exc): + sys.path = self.original_object + sys.path[:] = self.original_value class TransientResource(object): @@ -623,6 +649,9 @@ def captured_output(stream_name): def captured_stdout(): return captured_output("stdout") +def captured_stdin(): + return captured_output("stdin") + def gc_collect(): """Force as many objects as possible to be collected. diff --git a/Lib/test/test_cmd_line.py b/Lib/test/test_cmd_line.py index 87ae2b66199..b4a1f9f1005 100644 --- a/Lib/test/test_cmd_line.py +++ b/Lib/test/test_cmd_line.py @@ -65,16 +65,6 @@ class CmdLineTest(unittest.TestCase): self.verify_valid_flag('-Qwarnall') def test_site_flag(self): - if os.name == 'posix': - # Workaround bug #586680 by adding the extension dir to PYTHONPATH - from distutils.util import get_platform - s = "./build/lib.%s-%.3s" % (get_platform(), sys.version) - if hasattr(sys, 'gettotalrefcount'): - s += '-pydebug' - p = os.environ.get('PYTHONPATH', '') - if p: - p += ':' - os.environ['PYTHONPATH'] = p + s self.verify_valid_flag('-S') def test_usage(self): diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py index b6ef06d6ef0..0a51cd01e60 100644 --- a/Lib/test/test_descr.py +++ b/Lib/test/test_descr.py @@ -4001,6 +4001,7 @@ order (MRO) for bases """ def test_file_fault(self): # Testing sys.stdout is changed in getattr... import sys + test_stdout = sys.stdout class StdoutGuard: def __getattr__(self, attr): sys.stdout = sys.__stdout__ @@ -4010,6 +4011,8 @@ order (MRO) for bases """ print("Oops!") except RuntimeError: pass + finally: + sys.stdout = test_stdout def test_vicious_descriptor_nonsense(self): # Testing vicious_descriptor_nonsense... diff --git a/Lib/test/test_httpservers.py b/Lib/test/test_httpservers.py index 13e4030fea7..9a3d2049594 100644 --- a/Lib/test/test_httpservers.py +++ b/Lib/test/test_httpservers.py @@ -51,6 +51,7 @@ class TestServerThread(threading.Thread): class BaseTestCase(unittest.TestCase): def setUp(self): + os.environ = support.EnvironmentVarGuard() self.lock = threading.Lock() self.thread = TestServerThread(self, self.request_handler) self.thread.start() @@ -59,6 +60,7 @@ class BaseTestCase(unittest.TestCase): def tearDown(self): self.lock.release() self.thread.stop() + os.environ.__exit__() def request(self, uri, method='GET', body=None, headers={}): self.connection = http.client.HTTPConnection('localhost', self.PORT) diff --git a/Lib/test/test_imp.py b/Lib/test/test_imp.py index 70169e0698c..80df6e7dfe6 100644 --- a/Lib/test/test_imp.py +++ b/Lib/test/test_imp.py @@ -146,18 +146,38 @@ class ImportTests(unittest.TestCase): support.rmtree(test_package_name) - def test_reload(self): - import marshal - imp.reload(marshal) - import string - imp.reload(string) - ## import sys - ## self.assertRaises(ImportError, reload, sys) +class ReloadTests(unittest.TestCase): + + """Very basic tests to make sure that imp.reload() operates just like + reload().""" + + def test_source(self): + # XXX (ncoghlan): It would be nice to use test_support.CleanImport + # here, but that breaks because the os module registers some + # handlers in copy_reg on import. Since CleanImport doesn't + # revert that registration, the module is left in a broken + # state after reversion. Reinitialising the module contents + # and just reverting os.environ to its previous state is an OK + # workaround + with support.EnvironmentVarGuard(): + import os + imp.reload(os) + + def test_extension(self): + with support.CleanImport('time'): + import time + imp.reload(time) + + def test_builtin(self): + with support.CleanImport('marshal'): + import marshal + imp.reload(marshal) def test_main(): tests = [ ImportTests, + ReloadTests, ] try: import _thread diff --git a/Lib/test/test_import.py b/Lib/test/test_import.py index 20c2d8826b3..23967cb4e60 100644 --- a/Lib/test/test_import.py +++ b/Lib/test/test_import.py @@ -8,7 +8,8 @@ import py_compile import warnings import imp import marshal -from test.support import unlink, TESTFN, unload, run_unittest, TestFailed +from test.support import (unlink, TESTFN, unload, run_unittest, + TestFailed, EnvironmentVarGuard) def remove_files(name): @@ -109,9 +110,22 @@ class ImportTest(unittest.TestCase): def testImpModule(self): # Verify that the imp module can correctly load and find .py files - import imp - x = imp.find_module("os") - os = imp.load_module("os", *x) + import imp, os + # XXX (ncoghlan): It would be nice to use test_support.CleanImport + # here, but that breaks because the os module registers some + # handlers in copy_reg on import. Since CleanImport doesn't + # revert that registration, the module is left in a broken + # state after reversion. Reinitialising the module contents + # and just reverting os.environ to its previous state is an OK + # workaround + orig_path = os.path + orig_getenv = os.getenv + with EnvironmentVarGuard(): + x = imp.find_module("os") + new_os = imp.load_module("os", *x) + self.assertIs(os, new_os) + self.assertIs(orig_path, new_os.path) + self.assertIsNot(orig_getenv, new_os.getenv) def test_module_with_large_stack(self, module='longlist'): # create module w/list of 65000 elements to test bug #561858 @@ -360,7 +374,7 @@ class PathsTests(unittest.TestCase): def tearDown(self): shutil.rmtree(self.path) - sys.path = self.syspath + sys.path[:] = self.syspath # http://bugs.python.org/issue1293 def test_trailing_slash(self): diff --git a/Lib/test/test_site.py b/Lib/test/test_site.py index 20444b76247..6935798a3fe 100644 --- a/Lib/test/test_site.py +++ b/Lib/test/test_site.py @@ -41,7 +41,7 @@ class HelperFunctionsTests(unittest.TestCase): def tearDown(self): """Restore sys.path""" - sys.path = self.sys_path + sys.path[:] = self.sys_path site.USER_BASE = self.old_base site.USER_SITE = self.old_site site.PREFIXES = self.old_prefixes @@ -247,7 +247,7 @@ class ImportSideEffectTests(unittest.TestCase): def tearDown(self): """Restore sys.path""" - sys.path = self.sys_path + sys.path[:] = self.sys_path def test_abs__file__(self): # Make sure all imported modules have their __file__ attribute diff --git a/Lib/test/test_unittest.py b/Lib/test/test_unittest.py index 040880ad028..b6100b15147 100644 --- a/Lib/test/test_unittest.py +++ b/Lib/test/test_unittest.py @@ -3572,12 +3572,12 @@ class TestDiscovery(TestCase): os.path.isfile = lambda path: False self.addCleanup(restore_isfile) - full_path = os.path.abspath(os.path.normpath('/foo')) - def clean_path(): - if sys.path[-1] == full_path: - sys.path.pop(-1) - self.addCleanup(clean_path) + orig_sys_path = sys.path[:] + def restore_path(): + sys.path[:] = orig_sys_path + self.addCleanup(restore_path) + full_path = os.path.abspath(os.path.normpath('/foo')) with self.assertRaises(ImportError): loader.discover('/foo/bar', top_level_dir='/foo') @@ -3599,6 +3599,7 @@ class TestDiscovery(TestCase): self.assertEqual(suite, "['tests']") self.assertEqual(loader._top_level_dir, top_level_dir) self.assertEqual(_find_tests_args, [(start_dir, 'pattern')]) + self.assertIn(top_level_dir, sys.path) def test_discover_with_modules_that_fail_to_import(self): loader = unittest.TestLoader() @@ -3607,12 +3608,15 @@ class TestDiscovery(TestCase): os.listdir = lambda _: ['test_this_does_not_exist.py'] isfile = os.path.isfile os.path.isfile = lambda _: True + orig_sys_path = sys.path[:] def restore(): os.path.isfile = isfile os.listdir = listdir + sys.path[:] = orig_sys_path self.addCleanup(restore) suite = loader.discover('.') + self.assertIn(os.getcwd(), sys.path) self.assertEqual(suite.countTestCases(), 1) test = list(list(suite)[0])[0] # extract test from suite diff --git a/Lib/test/test_xmlrpc.py b/Lib/test/test_xmlrpc.py index 3ea2e1b542c..1542cd4063c 100644 --- a/Lib/test/test_xmlrpc.py +++ b/Lib/test/test_xmlrpc.py @@ -725,54 +725,45 @@ class CGIHandlerTestCase(unittest.TestCase): env['REQUEST_METHOD'] = 'GET' # if the method is GET and no request_text is given, it runs handle_get # get sysout output - tmp = sys.stdout - sys.stdout = open(support.TESTFN, "w") - self.cgi.handle_request() - sys.stdout.close() - sys.stdout = tmp + with support.captured_stdout() as data_out: + self.cgi.handle_request() # parse Status header - handle = open(support.TESTFN, "r").read() + data_out.seek(0) + handle = data_out.read() status = handle.split()[1] message = ' '.join(handle.split()[2:4]) self.assertEqual(status, '400') self.assertEqual(message, 'Bad Request') - os.remove(support.TESTFN) def test_cgi_xmlrpc_response(self): data = """ - - test_method - - - foo - - - bar - - - -""" - open("xmldata.txt", "w").write(data) - tmp1 = sys.stdin - tmp2 = sys.stdout + + test_method + + + foo + + + bar + + + + """ - sys.stdin = open("xmldata.txt", "r") - sys.stdout = open(support.TESTFN, "w") - - with support.EnvironmentVarGuard() as env: + with support.EnvironmentVarGuard() as env, \ + support.captured_stdout() as data_out, \ + support.captured_stdin() as data_in: + data_in.write(data) + data_in.seek(0) env['CONTENT_LENGTH'] = str(len(data)) self.cgi.handle_request() - - sys.stdin.close() - sys.stdout.close() - sys.stdin = tmp1 - sys.stdout = tmp2 + data_out.seek(0) # will respond exception, if so, our goal is achieved ;) - handle = open(support.TESTFN, "r").read() + handle = data_out.read() # start with 44th char so as not to get http header, we just # need only xml @@ -789,9 +780,6 @@ class CGIHandlerTestCase(unittest.TestCase): int(re.search('Content-Length: (\d+)', handle).group(1)), len(content)) - os.remove("xmldata.txt") - os.remove(support.TESTFN) - def test_main(): xmlrpc_tests = [XMLRPCTestCase, HelperTestCase, DateTimeTestCase,