Rev 5577: Implement a fixture for isolating tests from ``os.environ``. in file:///home/vila/src/bzr/bugs/test-isolation/
Vincent Ladeuil
v.ladeuil+lp at free.fr
Sat Dec 18 10:19:25 GMT 2010
At file:///home/vila/src/bzr/bugs/test-isolation/
------------------------------------------------------------
revno: 5577
revision-id: v.ladeuil+lp at free.fr-20101218101925-cb0qev0gc4k9m7hh
parent: v.ladeuil+lp at free.fr-20101217160334-eqlngxvvgyog3pkm
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: even-better-isolation
timestamp: Sat 2010-12-18 11:19:25 +0100
message:
Implement a fixture for isolating tests from ``os.environ``.
-------------- next part --------------
=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py 2010-12-17 16:03:34 +0000
+++ b/bzrlib/tests/__init__.py 2010-12-18 10:19:25 +0000
@@ -122,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.
@@ -1538,55 +1625,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):
=== 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 10:19:25 +0000
@@ -3455,3 +3455,56 @@
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'])
=== 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 10:19:25 +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 the
+ 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 tsets. (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