From 6ead552b47ecc4e7d85a03b3b662c6d46e59bec3 Mon Sep 17 00:00:00 2001 From: Nick Coghlan Date: Sun, 18 Oct 2009 13:19:33 +0000 Subject: Note that a number of the changes listed below were not applicable to the Py3k branch, and hence the corresponding files are unchanged in this checkin. This checkin is also the first time the environment checking in regrtest has been forward ported to the Py3k branch. This checkin causes test_xmlrpc to fail - see issue 7165 (it's a bug in the 3.x version of xmlrpc.server) I am also getting a failure in test_telnetlib, but it isn't clear yet if that is due to these changes. Recorded merge of revisions 75400-75401,75404,75406,75414,75416,75422,75425-75428,75435,75439,75441-75444,75447-75449,75451-75453,75455-75458,75460-75469,75471-75473,75475-75477,75479-75481,75483,75486-75489 via svnmerge from svn+ssh://pythondev@svn.python.org/python/trunk ........ r75400 | r.david.murray | 2009-10-14 23:58:07 +1000 (Wed, 14 Oct 2009) | 6 lines Enhanced Issue 7058 patch, which will not be backported. Refactors the code, adds checks for stdin/out/err, cwd, and sys.path, and adds a new section in the summary for tests that modify the environment (thanks to Ezio Melotti for that suggestion). ........ r75453 | nick.coghlan | 2009-10-17 16:33:05 +1000 (Sat, 17 Oct 2009) | 1 line Correctly restore sys.stdout in test_descr ........ r75456 | nick.coghlan | 2009-10-17 17:30:40 +1000 (Sat, 17 Oct 2009) | 1 line Enhancement to the new environment checking code to print the changed items under -vv. Also includes a small tweak to allow underscores in the names of resources. ........ r75457 | nick.coghlan | 2009-10-17 17:34:27 +1000 (Sat, 17 Oct 2009) | 1 line Formatting tweak so that before and after values are vertically aligned ........ r75458 | nick.coghlan | 2009-10-17 18:21:21 +1000 (Sat, 17 Oct 2009) | 1 line Check and revert expected sys.path alterations ........ r75461 | nick.coghlan | 2009-10-18 00:40:54 +1000 (Sun, 18 Oct 2009) | 1 line Restore original sys.path when running TTK tests ........ r75462 | nick.coghlan | 2009-10-18 01:09:41 +1000 (Sun, 18 Oct 2009) | 1 line Don't invoke reload(sys) and use StringIO objects instead of real files to capture stdin and stdout when needed (ensures all sys attributes remain unmodified after test_xmlrpc runs) ........ r75463 | nick.coghlan | 2009-10-18 01:23:08 +1000 (Sun, 18 Oct 2009) | 1 line Revert changes made to environment in test_httpservers ........ r75465 | nick.coghlan | 2009-10-18 01:45:52 +1000 (Sun, 18 Oct 2009) | 1 line Move restoration of the os.environ object into the context manager where it belongs ........ r75466 | nick.coghlan | 2009-10-18 01:48:16 +1000 (Sun, 18 Oct 2009) | 1 line Also check and restore identity of sys.path, sys.argv and os.environ rather than just their values (this picked up a few more misbehaving tests) ........ r75467 | nick.coghlan | 2009-10-18 01:57:42 +1000 (Sun, 18 Oct 2009) | 1 line Avoid replacing existing modules and sys.path in import tests ........ r75468 | nick.coghlan | 2009-10-18 02:19:51 +1000 (Sun, 18 Oct 2009) | 1 line Don't replace sys.path in test_site ........ r75481 | nick.coghlan | 2009-10-18 15:38:48 +1000 (Sun, 18 Oct 2009) | 1 line Using CleanImport to revert a reload of the os module doesn't work due to function registrations in copy_reg. The perils of reloading modules even for tests... ........ r75486 | nick.coghlan | 2009-10-18 20:29:10 +1000 (Sun, 18 Oct 2009) | 1 line Silence a deprecation warning by using the appropriate replacement construct ........ r75489 | nick.coghlan | 2009-10-18 20:56:21 +1000 (Sun, 18 Oct 2009) | 1 line Restore sys.path in test_tk ........ --- Lib/test/regrtest.py | 168 +++++++++++++++++++++++++++++++++++++------ Lib/test/support.py | 29 ++++++++ Lib/test/test_cmd_line.py | 10 --- Lib/test/test_descr.py | 3 + Lib/test/test_httpservers.py | 2 + Lib/test/test_imp.py | 34 +++++++-- Lib/test/test_import.py | 24 +++++-- Lib/test/test_site.py | 4 +- Lib/test/test_unittest.py | 14 ++-- Lib/test/test_xmlrpc.py | 60 +++++++--------- 10 files changed, 263 insertions(+), 85 deletions(-) diff --git a/Lib/test/regrtest.py b/Lib/test/regrtest.py index a544d67..e808581 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 04964a1..611d48f 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 87ae2b6..b4a1f9f 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 b6ef06d..0a51cd0 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 13e4030..9a3d204 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 70169e0..80df6e7 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 20c2d88..23967cb 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 20444b7..6935798 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 040880a..b6100b1 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 3ea2e1b..1542cd4 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 - - sys.stdin = open("xmldata.txt", "r") - sys.stdout = open(support.TESTFN, "w") - - with support.EnvironmentVarGuard() as env: + + test_method + + + foo + + + bar + + + + """ + + 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, -- cgit v0.12