Rev 5577: Merge even-better-isolation into 321320-isolate-doc-tests in file:///home/vila/src/bzr/bugs/test-isolation/
Vincent Ladeuil
v.ladeuil+lp at free.fr
Sat Dec 18 19:01:49 GMT 2010
At file:///home/vila/src/bzr/bugs/test-isolation/
------------------------------------------------------------
revno: 5577 [merge]
revision-id: v.ladeuil+lp at free.fr-20101218190149-438j9yfefv099501
parent: v.ladeuil+lp at free.fr-20101217143057-1jzbj1icd6rbbiyw
parent: v.ladeuil+lp at free.fr-20101218190012-e8ernk1gvn0ci4rq
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 321320-isolate-doc-tests
timestamp: Sat 2010-12-18 20:01:49 +0100
message:
Merge even-better-isolation into 321320-isolate-doc-tests
modified:
bzrlib/tests/TestUtil.py TestUtil.py-20050824080200-5f70140a2d938694
bzrlib/tests/__init__.py selftest.py-20050531073622-8d0e3c8845c97a64
bzrlib/tests/test_selftest.py test_selftest.py-20051202044319-c110a115d8c0456a
doc/en/release-notes/bzr-2.3.txt NEWS-20050323055033-4e00b5db738777ff
-------------- next part --------------
=== modified file 'bzrlib/tests/TestUtil.py'
--- a/bzrlib/tests/TestUtil.py 2010-09-21 03:20:09 +0000
+++ b/bzrlib/tests/TestUtil.py 2010-12-18 10:21:52 +0000
@@ -29,9 +29,11 @@
class LogCollector(logging.Handler):
+
def __init__(self):
logging.Handler.__init__(self)
self.records=[]
+
def emit(self, record):
self.records.append(record.getMessage())
@@ -60,7 +62,8 @@
visitor.visitSuite(test)
visitTests(test, visitor)
else:
- print "unvisitable non-unittest.TestCase element %r (%r)" % (test, test.__class__)
+ print "unvisitable non-unittest.TestCase element %r (%r)" % (
+ test, test.__class__)
class TestSuite(unittest.TestSuite):
@@ -183,7 +186,9 @@
class TestVisitor(object):
"""A visitor for Tests"""
+
def visitSuite(self, aTestSuite):
pass
+
def visitCase(self, aTestCase):
pass
=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py 2010-12-17 14:30:57 +0000
+++ b/bzrlib/tests/__init__.py 2010-12-18 19:01:49 +0000
@@ -64,50 +64,35 @@
branchbuilder,
bzrdir,
chk_map,
+ commands as _mod_commands,
config,
debug,
errors,
hooks,
lock as _mod_lock,
+ lockdir,
memorytree,
osutils,
+ plugin as _mod_plugin,
pyutils,
ui,
urlutils,
registry,
+ symbol_versioning,
+ trace,
transport as _mod_transport,
workingtree,
)
-import bzrlib.branch
-import bzrlib.commands
-import bzrlib.timestamp
-import bzrlib.export
-import bzrlib.inventory
-import bzrlib.iterablefile
-import bzrlib.lockdir
try:
import bzrlib.lsprof
except ImportError:
# lsprof not available
pass
-import bzrlib.merge3
-import bzrlib.plugin
from bzrlib.smart import client, request
-import bzrlib.store
-from bzrlib import symbol_versioning
-from bzrlib.symbol_versioning import (
- DEPRECATED_PARAMETER,
- deprecated_function,
- deprecated_in,
- deprecated_method,
- deprecated_passed,
- )
-import bzrlib.trace
from bzrlib.transport import (
memory,
pathfilter,
)
-from bzrlib.trace import mutter, note
from bzrlib.tests import (
test_server,
TestUtil,
@@ -115,7 +100,6 @@
)
from bzrlib.ui import NullProgressView
from bzrlib.ui.text import TextUIFactory
-import bzrlib.version_info_formats.format_custom
# Mark this python module as being part of the implementation
# of unittest: this gives us better tracebacks where the last
@@ -138,6 +122,93 @@
TestSuite = TestUtil.TestSuite
TestLoader = TestUtil.TestLoader
+# Tests should run in a clean and clearly defined environment. The goal is to
+# keep them isolated from the running environment as mush as possible. The test
+# framework ensures the variables defined below are set (or deleted if the
+# value is None) before a test is run and reset to their original value after
+# the test is run. Generally if some code depends on an environment variable,
+# the tests should start without this variable in the environment. There are a
+# few exceptions but you shouldn't violate this rule lightly.
+isolated_environ = {
+ 'BZR_HOME': None,
+ 'HOME': os.getcwd(),
+ # bzr now uses the Win32 API and doesn't rely on APPDATA, but the
+ # tests do check our impls match APPDATA
+ 'BZR_EDITOR': None, # test_msgeditor manipulates this variable
+ 'VISUAL': None,
+ 'EDITOR': None,
+ 'BZR_EMAIL': None,
+ 'BZREMAIL': None, # may still be present in the environment
+ 'EMAIL': 'jrandom at example.com', # set EMAIL as bzr does not guess
+ 'BZR_PROGRESS_BAR': None,
+ 'BZR_LOG': None,
+ 'BZR_PLUGIN_PATH': None,
+ 'BZR_DISABLE_PLUGINS': None,
+ 'BZR_PLUGINS_AT': None,
+ 'BZR_CONCURRENCY': None,
+ # Make sure that any text ui tests are consistent regardless of
+ # the environment the test case is run in; you may want tests that
+ # test other combinations. 'dumb' is a reasonable guess for tests
+ # going to a pipe or a StringIO.
+ 'TERM': 'dumb',
+ 'LINES': '25',
+ 'COLUMNS': '80',
+ 'BZR_COLUMNS': '80',
+ # Disable SSH Agent
+ 'SSH_AUTH_SOCK': None,
+ # Proxies
+ 'http_proxy': None,
+ 'HTTP_PROXY': None,
+ 'https_proxy': None,
+ 'HTTPS_PROXY': None,
+ 'no_proxy': None,
+ 'NO_PROXY': None,
+ 'all_proxy': None,
+ 'ALL_PROXY': None,
+ # Nobody cares about ftp_proxy, FTP_PROXY AFAIK. So far at
+ # least. If you do (care), please update this comment
+ # -- vila 20080401
+ 'ftp_proxy': None,
+ 'FTP_PROXY': None,
+ 'BZR_REMOTE_PATH': None,
+ # Generally speaking, we don't want apport reporting on crashes in
+ # the test envirnoment unless we're specifically testing apport,
+ # so that it doesn't leak into the real system environment. We
+ # use an env var so it propagates to subprocesses.
+ 'APPORT_DISABLE': '1',
+ }
+
+
+def override_os_environ(test, env=None):
+ """Modify os.environ keeping a copy.
+
+ :param test: A test instance
+
+ :param env: A dict containing variable definitions to be installed
+ """
+ if env is None:
+ env = isolated_environ
+ test._original_os_environ = dict([(var, value)
+ for var, value in os.environ.iteritems()])
+ for var, value in env.iteritems():
+ osutils.set_or_unset_env(var, value)
+ if var not in test._original_os_environ:
+ # The var is new, add it with a value of None, so
+ # restore_os_environ will delete it
+ test._original_os_environ[var] = None
+
+
+def restore_os_environ(test):
+ """Restore os.environ to its original state.
+
+ :param test: A test instance previously passed to override_os_environ.
+ """
+ for var, value in test._original_os_environ.iteritems():
+ # Restore the original value (or delete it if the value has been set to
+ # None in override_os_environ).
+ osutils.set_or_unset_env(var, value)
+
+
class ExtendedTestResult(testtools.TextTestResult):
"""Accepts, reports and accumulates the results of running tests.
@@ -810,6 +881,17 @@
return NullProgressView()
+
+def DocTestSuite(*args, **kwargs):
+ """Overrides doctest.DocTestSuite to handle isolation.
+
+ The method is really a factory and users are expected to use it as such.
+ """
+ kwargs['setUp'] = override_os_environ
+ kwargs['tearDown'] = restore_os_environ
+ return doctest.DocTestSuite(*args, **kwargs)
+
+
class TestCase(testtools.TestCase):
"""Base class for bzr unit tests.
@@ -1127,7 +1209,7 @@
except UnicodeError, e:
# If we can't compare without getting a UnicodeError, then
# obviously they are different
- mutter('UnicodeError: %s', e)
+ trace.mutter('UnicodeError: %s', e)
if message:
message += '\n'
raise AssertionError("%snot equal:\na = %s\nb = %s\n"
@@ -1193,7 +1275,6 @@
def assertLogsError(self, exception_class, func, *args, **kwargs):
"""Assert that func(*args, **kwargs) quietly logs a specific exception.
"""
- from bzrlib import trace
captured = []
orig_log_exception_quietly = trace.log_exception_quietly
try:
@@ -1493,7 +1574,7 @@
The file is removed as the test is torn down.
"""
self._log_file = StringIO()
- self._log_memento = bzrlib.trace.push_log_file(self._log_file)
+ self._log_memento = trace.push_log_file(self._log_file)
self.addCleanup(self._finishLogFile)
def _finishLogFile(self):
@@ -1501,10 +1582,10 @@
Close the file and delete it, unless setKeepLogfile was called.
"""
- if bzrlib.trace._trace_file:
+ if trace._trace_file:
# flush the log file, to get all content
- bzrlib.trace._trace_file.flush()
- bzrlib.trace.pop_log_file(self._log_memento)
+ trace._trace_file.flush()
+ trace.pop_log_file(self._log_memento)
# Cache the log result and delete the file on disk
self._get_log(False)
@@ -1555,55 +1636,7 @@
return value
def _cleanEnvironment(self):
- new_env = {
- 'BZR_HOME': None, # Don't inherit BZR_HOME to all the tests.
- 'HOME': os.getcwd(),
- # bzr now uses the Win32 API and doesn't rely on APPDATA, but the
- # tests do check our impls match APPDATA
- 'BZR_EDITOR': None, # test_msgeditor manipulates this variable
- 'VISUAL': None,
- 'EDITOR': None,
- 'BZR_EMAIL': None,
- 'BZREMAIL': None, # may still be present in the environment
- 'EMAIL': 'jrandom at example.com', # set EMAIL as bzr does not guess
- 'BZR_PROGRESS_BAR': None,
- 'BZR_LOG': None,
- 'BZR_PLUGIN_PATH': None,
- 'BZR_DISABLE_PLUGINS': None,
- 'BZR_PLUGINS_AT': None,
- 'BZR_CONCURRENCY': None,
- # Make sure that any text ui tests are consistent regardless of
- # the environment the test case is run in; you may want tests that
- # test other combinations. 'dumb' is a reasonable guess for tests
- # going to a pipe or a StringIO.
- 'TERM': 'dumb',
- 'LINES': '25',
- 'COLUMNS': '80',
- 'BZR_COLUMNS': '80',
- # SSH Agent
- 'SSH_AUTH_SOCK': None,
- # Proxies
- 'http_proxy': None,
- 'HTTP_PROXY': None,
- 'https_proxy': None,
- 'HTTPS_PROXY': None,
- 'no_proxy': None,
- 'NO_PROXY': None,
- 'all_proxy': None,
- 'ALL_PROXY': None,
- # Nobody cares about ftp_proxy, FTP_PROXY AFAIK. So far at
- # least. If you do (care), please update this comment
- # -- vila 20080401
- 'ftp_proxy': None,
- 'FTP_PROXY': None,
- 'BZR_REMOTE_PATH': None,
- # Generally speaking, we don't want apport reporting on crashes in
- # the test envirnoment unless we're specifically testing apport,
- # so that it doesn't leak into the real system environment. We
- # use an env var so it propagates to subprocesses.
- 'APPORT_DISABLE': '1',
- }
- for name, value in new_env.iteritems():
+ for name, value in isolated_environ.iteritems():
self.overrideEnv(name, value)
def _captureVar(self, name, newvalue):
@@ -1708,7 +1741,7 @@
self._benchtime += time.time() - start
def log(self, *args):
- mutter(*args)
+ trace.mutter(*args)
def _get_log(self, keep_log_file=False):
"""Internal helper to get the log from bzrlib.trace for this test.
@@ -1799,9 +1832,10 @@
try:
try:
- result = self.apply_redirected(ui.ui_factory.stdin,
+ result = self.apply_redirected(
+ ui.ui_factory.stdin,
stdout, stderr,
- bzrlib.commands.run_bzr_catch_user_errors,
+ _mod_commands.run_bzr_catch_user_errors,
args)
except KeyboardInterrupt:
# Reraise KeyboardInterrupt with contents of redirected stdout
@@ -2057,8 +2091,8 @@
if retcode is not None and retcode != process.returncode:
if process_args is None:
process_args = "(unknown args)"
- mutter('Output of bzr %s:\n%s', process_args, out)
- mutter('Error for bzr %s:\n%s', process_args, err)
+ trace.mutter('Output of bzr %s:\n%s', process_args, out)
+ trace.mutter('Error for bzr %s:\n%s', process_args, err)
self.fail('Command bzr %s failed with retcode %s != %s'
% (process_args, retcode, process.returncode))
return [out, err]
@@ -2121,7 +2155,7 @@
Tests that expect to provoke LockContention errors should call this.
"""
- self.overrideAttr(bzrlib.lockdir, '_DEFAULT_TIMEOUT_SECONDS', 0)
+ self.overrideAttr(lockdir, '_DEFAULT_TIMEOUT_SECONDS', 0)
def make_utf8_encoded_stringio(self, encoding_type=None):
"""Return a StringIOWrapper instance, that will encode Unicode
@@ -3608,8 +3642,9 @@
key, obj, help=help, info=info, override_existing=False)
except KeyError:
actual = self.get(key)
- note('Test prefix alias %s is already used for %s, ignoring %s'
- % (key, actual, obj))
+ trace.note(
+ 'Test prefix alias %s is already used for %s, ignoring %s'
+ % (key, actual, obj))
def resolve_alias(self, id_start):
"""Replace the alias by the prefix in the given string.
@@ -3933,7 +3968,7 @@
suite.addTest(doc_suite)
default_encoding = sys.getdefaultencoding()
- for name, plugin in bzrlib.plugin.plugins().items():
+ for name, plugin in _mod_plugin.plugins().items():
if not interesting_module(plugin.module.__name__):
continue
plugin_suite = plugin.test_suite()
@@ -3945,7 +3980,7 @@
if plugin_suite is not None:
suite.addTest(plugin_suite)
if default_encoding != sys.getdefaultencoding():
- bzrlib.trace.warning(
+ trace.warning(
'Plugin "%s" tried to reset default encoding to: %s', name,
sys.getdefaultencoding())
reload(sys)
@@ -3966,9 +4001,9 @@
# Some tests mentioned in the list are not in the test suite. The
# list may be out of date, report to the tester.
for id in not_found:
- bzrlib.trace.warning('"%s" not found in the test suite', id)
+ trace.warning('"%s" not found in the test suite', id)
for id in duplicates:
- bzrlib.trace.warning('"%s" is used as an id by several tests', id)
+ trace.warning('"%s" is used as an id by several tests', id)
return suite
@@ -4312,13 +4347,6 @@
return self.module_name
-# This is kept here for compatibility, it is recommended to use
-# 'bzrlib.tests.feature.paramiko' instead
-ParamikoFeature = _CompatabilityThunkFeature(
- deprecated_in((2,1,0)),
- 'bzrlib.tests.features', 'ParamikoFeature', 'paramiko')
-
-
def probe_unicode_in_user_encoding():
"""Try to encode several unicode strings to use in unicode-aware tests.
Return first successfull match.
@@ -4511,10 +4539,6 @@
case_sensitive_filesystem_feature = _CaseSensitiveFilesystemFeature()
-# Kept for compatibility, use bzrlib.tests.features.subunit instead
-SubUnitFeature = _CompatabilityThunkFeature(
- deprecated_in((2,1,0)),
- 'bzrlib.tests.features', 'SubUnitFeature', 'subunit')
# Only define SubUnitBzrRunner if subunit is available.
try:
from subunit import TestProtocolClient
=== modified file 'bzrlib/tests/test_selftest.py'
--- a/bzrlib/tests/test_selftest.py 2010-12-16 13:15:42 +0000
+++ b/bzrlib/tests/test_selftest.py 2010-12-18 19:00:12 +0000
@@ -17,7 +17,7 @@
"""Tests for the test framework."""
from cStringIO import StringIO
-from doctest import ELLIPSIS
+import doctest
import os
import signal
import sys
@@ -92,7 +92,7 @@
"text", "plain", {"charset": "utf8"})))
self.assertThat(u"".join(log.iter_text()), Equals(self.get_log()))
self.assertThat(self.get_log(),
- DocTestMatches(u"...a test message\n", ELLIPSIS))
+ DocTestMatches(u"...a test message\n", doctest.ELLIPSIS))
class TestUnicodeFilename(tests.TestCase):
@@ -3210,7 +3210,8 @@
tpr.register('bar', 'bBB.aAA.rRR')
self.assertEquals('bbb.aaa.rrr', tpr.get('bar'))
self.assertThat(self.get_log(),
- DocTestMatches("...bar...bbb.aaa.rrr...BB.aAA.rRR", ELLIPSIS))
+ DocTestMatches("...bar...bbb.aaa.rrr...BB.aAA.rRR",
+ doctest.ELLIPSIS))
def test_get_unknown_prefix(self):
tpr = self._get_registry()
@@ -3455,3 +3456,121 @@
self.fail(output.getvalue())
# We get our value back
self.assertEquals('42', os.environ.get('MYVAR'))
+
+
+class TestIsolatedEnv(tests.TestCase):
+ """Test isolating tests from os.environ.
+
+ Since we use tests that are already isolated from os.environ abit of care
+ should be taken when designing the tests to avoid bootstrap side-effects.
+ The tests start an already clean os.environ which allow doing valid
+ assertions about which variables are present or not and design tests around
+ these assertions.
+ """
+
+ class ScratchMonkey(tests.TestCase):
+
+ def test_me(self):
+ pass
+
+ def test_basics(self):
+ # Make sure we know the definition of BZR_HOME: not part of os.environ
+ # for tests.TestCase.
+ self.assertTrue('BZR_HOME' in tests.isolated_environ)
+ self.assertEquals(None, tests.isolated_environ['BZR_HOME'])
+ # Being part of isolated_environ, BZR_HOME should not appear here
+ self.assertFalse('BZR_HOME' in os.environ)
+ # Make sure we know the definition of LINES: part of os.environ for
+ # tests.TestCase
+ self.assertTrue('LINES' in tests.isolated_environ)
+ self.assertEquals('25', tests.isolated_environ['LINES'])
+ self.assertEquals('25', os.environ['LINES'])
+
+ def test_injecting_unknown_variable(self):
+ # BZR_HOME is known to be absent from os.environ
+ test = self.ScratchMonkey('test_me')
+ tests.override_os_environ(test, {'BZR_HOME': 'foo'})
+ self.assertEquals('foo', os.environ['BZR_HOME'])
+ tests.restore_os_environ(test)
+ self.assertFalse('BZR_HOME' in os.environ)
+
+ def test_injecting_known_variable(self):
+ test = self.ScratchMonkey('test_me')
+ # LINES is known to be present in os.environ
+ tests.override_os_environ(test, {'LINES': '42'})
+ self.assertEquals('42', os.environ['LINES'])
+ tests.restore_os_environ(test)
+ self.assertEquals('25', os.environ['LINES'])
+
+ def test_deleting_variable(self):
+ test = self.ScratchMonkey('test_me')
+ # LINES is known to be present in os.environ
+ tests.override_os_environ(test, {'LINES': None})
+ self.assertTrue('LINES' not in os.environ)
+ tests.restore_os_environ(test)
+ self.assertEquals('25', os.environ['LINES'])
+
+
+class TestDocTestSuiteIsolation(tests.TestCase):
+ """Test that `tests.DocTestSuite` isolates doc tests from os.environ.
+
+ Since tests.TestCase alreay provides an isolation from os.environ, we use
+ the clean environment as a base for testing. To precisely capture the
+ isolation provided by tests.DocTestSuite, we use doctest.DocTestSuite to
+ compare against.
+
+ We want to make sure `tests.DocTestSuite` respect `tests.isolated_environ`,
+ not `os.environ` so each test overrides it to suit its needs.
+
+ """
+
+ def get_doctest_suite_for_string(self, klass, string):
+ class Finder(doctest.DocTestFinder):
+
+ def find(*args, **kwargs):
+ test = doctest.DocTestParser().get_doctest(
+ string, {}, 'foo', 'foo.py', 0)
+ return [test]
+
+ suite = klass(test_finder=Finder())
+ return suite
+
+ def run_doctest_suite_for_string(self, klass, string):
+ suite = self.get_doctest_suite_for_string(klass, string)
+ output = StringIO()
+ result = tests.TextTestResult(output, 0, 1)
+ suite.run(result)
+ return result, output
+
+ def assertDocTestStringSucceds(self, klass, string):
+ result, output = self.run_doctest_suite_for_string(klass, string)
+ if not result.wasStrictlySuccessful():
+ self.fail(output.getvalue())
+
+ def assertDocTestStringFails(self, klass, string):
+ result, output = self.run_doctest_suite_for_string(klass, string)
+ if result.wasStrictlySuccessful():
+ self.fail(output.getvalue())
+
+ def test_injected_variable(self):
+ self.overrideAttr(tests, 'isolated_environ', {'LINES': '42'})
+ test = """
+ >>> import os
+ >>> os.environ['LINES']
+ '42'
+ """
+ # doctest.DocTestSuite fails as it sees '25'
+ self.assertDocTestStringFails(doctest.DocTestSuite, test)
+ # tests.DocTestSuite sees '42'
+ self.assertDocTestStringSucceds(tests.DocTestSuite, test)
+
+ def test_deleted_variable(self):
+ self.overrideAttr(tests, 'isolated_environ', {'LINES': None})
+ test = """
+ >>> import os
+ >>> os.environ.get('LINES')
+ """
+ # doctest.DocTestSuite fails as it sees '25'
+ self.assertDocTestStringFails(doctest.DocTestSuite, test)
+ # tests.DocTestSuite sees None
+ self.assertDocTestStringSucceds(tests.DocTestSuite, test)
=== modified file 'doc/en/release-notes/bzr-2.3.txt'
--- a/doc/en/release-notes/bzr-2.3.txt 2010-12-17 14:29:44 +0000
+++ b/doc/en/release-notes/bzr-2.3.txt 2010-12-18 18:50:01 +0000
@@ -82,6 +82,11 @@
suite. This can include new facilities for writing tests, fixes to
spurious test failures and changes to the way things should be tested.
+* ``bzrlib.tests`` defines ``isolated_environ`` with the definitions of all
+ the environment variables the tests should care about. It also defines
+ ``override_os_environ`` and ``restore_os_environ`` to properly implement
+ isolation from ``os.environ`` for tests. (Vincent Ladeuil, #321320)
+
* Catch exceptions related to bug #637821 during test cleanup to avoid
spurious failures. (Vincent Ladeuil, #686008).
More information about the bazaar-commits
mailing list