Rev 4692: Add stronger test isolation by interception BzrDir.open and checking the thing being opened is known to the test suite. in http://bazaar.launchpad.net/~lifeless/bzr/test-speed

Robert Collins robertc at robertcollins.net
Thu Sep 17 12:54:54 BST 2009


At http://bazaar.launchpad.net/~lifeless/bzr/test-speed

------------------------------------------------------------
revno: 4692
revision-id: robertc at robertcollins.net-20090917115441-2ug57z6eyrnb6zim
parent: pqm at pqm.ubuntu.com-20090916025214-6cyz9179xs7f1w70
committer: Robert Collins <robertc at robertcollins.net>
branch nick: test-speed
timestamp: Thu 2009-09-17 21:54:41 +1000
message:
  Add stronger test isolation by interception BzrDir.open and checking the thing being opened is known to the test suite.
=== modified file 'NEWS'
--- a/NEWS	2009-09-16 02:52:14 +0000
+++ b/NEWS	2009-09-17 11:54:41 +0000
@@ -122,6 +122,11 @@
   to be parameterised. This is not expected to break external use of test
   parameterisation, and is substantially faster. (Robert Collins)
 
+* Test's that try to open a bzr dir on an arbitrary transport will now
+  fail unless they have explicitly permitted the transport via
+  ``self.permit_url``. The standard test factories such as ``self.get_url``
+  will permit the urls they provide automatically, so only exceptional
+ tests should need to do this. (Robert Collins)
 
 bzr 2.0rc2
 ##########

=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py	2009-09-14 02:15:45 +0000
+++ b/bzrlib/tests/__init__.py	2009-09-17 11:54:41 +0000
@@ -90,7 +90,7 @@
     deprecated_passed,
     )
 import bzrlib.trace
-from bzrlib.transport import get_transport
+from bzrlib.transport import chroot, get_transport
 import bzrlib.transport
 from bzrlib.transport.local import LocalURLServer
 from bzrlib.transport.memory import MemoryServer
@@ -819,6 +819,7 @@
         self._cleanups = []
         self._bzr_test_setUp_run = False
         self._bzr_test_tearDown_run = False
+        self._directory_isolation = True
 
     def setUp(self):
         unittest.TestCase.setUp(self)
@@ -829,6 +830,7 @@
         self._benchcalls = []
         self._benchtime = None
         self._clear_hooks()
+        self._track_transports()
         self._track_locks()
         self._clear_debug_flags()
         TestCase._active_threads = threading.activeCount()
@@ -875,6 +877,14 @@
         # this hook should always be installed
         request._install_hook()
 
+    def disable_directory_isolation(self):
+        """Turn off directory isolation checks."""
+        self._directory_isolation = False
+
+    def enable_directory_isolation(self):
+        """Enable directory isolation checks."""
+        self._directory_isolation = True
+
     def _silenceUI(self):
         """Turn off UI for duration of test"""
         # by default the UI is off; tests can turn it on if they want it.
@@ -935,6 +945,74 @@
     def _lock_broken(self, result):
         self._lock_actions.append(('broken', result))
 
+    def permit_dir(self, name):
+        """Permit a directory to be used by this test. See permit_url."""
+        name_transport = get_transport(name)
+        self.permit_url(name)
+        self.permit_url(name_transport.base)
+
+    def permit_url(self, url):
+        """Declare that url is an ok url to use in this test.
+        
+        Do this for memory transports, temporary test directory etc.
+        
+        Do not do this for the current working directory, /tmp, or any other
+        preexisting non isolated url.
+        """
+        if not url.endswith('/'):
+            url += '/'
+        self._bzr_selftest_roots.append(url)
+
+    def permit_source_tree_branch_repo(self):
+        """Permit the source tree bzr is running from to be opened.
+
+        Some code such as bzrlib.version attempts to read from the bzr branch
+        that bzr is executing from (if any). This method permits that directory
+        to be used in the test suite.
+        """
+        path = self.get_source_path()
+        self.disable_directory_isolation()
+        try:
+            try:
+                tree = workingtree.WorkingTree.open(path)
+            except errors.NoWorkingTree:
+                return
+        finally:
+            self.enable_directory_isolation()
+        self.permit_url(tree.bzrdir.root_transport.base)
+        self.permit_url(tree.branch.bzrdir.root_transport.base)
+        self.permit_url(tree.branch.repository.bzrdir.root_transport.base)
+
+    def _preopen_isolate_transport(self, transport):
+        """Check that all transport openings are done in the test work area."""
+        if isinstance(transport, chroot.ChrootTransport):
+            # Unwrap chrooted transports
+            url = transport.server.backing_transport.clone(
+                transport._safe_relpath('.')).base
+        else:
+            url = transport.base
+        # ReadonlySmartTCPServer_for_testing decorates the backing transport
+        # urls it is given by prepending readonly+. This is appropriate as the
+        # client shouldn't know that the server is readonly (or not readonly).
+        # We could register all servers twice, with readonly+ prepending, but
+        # that makes for a long list; this is about the same but easier to
+        # read.
+        if url.startswith('readonly+'):
+            url = url[len('readonly+'):]
+        self._preopen_isolate_url(url)
+
+    def _preopen_isolate_url(self, url):
+        if not self._directory_isolation:
+            return
+        # This prevents all transports, including e.g. sftp ones backed on disk
+        # from working unless they are explicitly granted permission. We then
+        # depend on the code that sets up test transports to check that they are
+        # appropriately isolated and enable their use by calling
+        # self.permit_transport()
+        if not osutils.is_inside_any(self._bzr_selftest_roots, url):
+            raise errors.BzrError("Attempt to escape test isolation: %r %r"
+                % (url, self._bzr_selftest_roots))
+
     def start_server(self, transport_server, backing_server=None):
         """Start transport_server for this test.
 
@@ -946,6 +1024,46 @@
         else:
             transport_server.setUp(backing_server)
         self.addCleanup(transport_server.tearDown)
+        # Obtain a real transport because if the server supplies a password, it
+        # will be hidden from the base on the client side.
+        t = get_transport(transport_server.get_url())
+        # Some transport servers effectively chroot the backing transport;
+        # others like SFTPServer don't - users of the transport can walk up the
+        # transport to read the entire backing transport. This wouldn't matter
+        # except that the workdir tests are given - and that they expect the
+        # server's url to point at - is one directory under the safety net. So
+        # Branch operations into the transport will attempt to walk up one
+        # directory. Chrooting all servers would avoid this but also mean that
+        # we wouldn't be testing directly against non-root urls. Alternatively
+        # getting the test framework to start the server with a backing server
+        # at the actual safety net directory would work too, but this then
+        # means that the self.get_url/self.get_transport methods would need
+        # to transform all their results. On balance its cleaner to handle it
+        # here, and permit a higher url when we have one of these transports.
+        if t.base.endswith('/work/'):
+            # we have safety net/test root/work
+            t = t.clone('../..')
+        elif isinstance(transport_server, server.SmartTCPServer_for_testing):
+            # The smart server adds a path similar to work, which is traversed
+            # up from by the client. But the server is chrooted - the actual
+            # backing transport is not escaped from, and VFS requests to the
+            # root will error (because they try to escape the chroot).
+            t2 = t.clone('..')
+            while t2.base != t.base:
+                t = t2
+                t2 = t.clone('..')
+        self.permit_url(t.base)
+
+    def _track_transports(self):
+        """Install checks for transport usage."""
+        # TestCase has no safe place it can write to.
+        self._bzr_selftest_roots = []
+        # Currently the easiest way to be sure that nothing is going on is to
+        # hook into bzr dir opening. This leaves a small window of error for
+        # transport tests, but they are well known, and we can improve on this
+        # step.
+        bzrdir.BzrDir.hooks.install_named_hook("pre_open",
+            self._preopen_isolate_transport, "Check bzr directories are safe.")
 
     def _ndiff_strings(self, a, b):
         """Return ndiff between two strings containing lines.
@@ -1869,9 +1987,13 @@
         """
         return Popen(*args, **kwargs)
 
+    def get_source_path(self):
+        """Return the path of the directory containing bzrlib."""
+        return os.path.dirname(os.path.dirname(bzrlib.__file__))
+
     def get_bzr_path(self):
         """Return the path of the 'bzr' executable for this test suite."""
-        bzr_path = os.path.dirname(os.path.dirname(bzrlib.__file__))+'/bzr'
+        bzr_path = self.get_source_path()+'/bzr'
         if not os.path.isfile(bzr_path):
             # We are probably installed. Assume sys.argv is the right file
             bzr_path = sys.argv[0]
@@ -2203,6 +2325,7 @@
         propagating. This method ensures than a test did not leaked.
         """
         root = TestCaseWithMemoryTransport.TEST_ROOT
+        self.permit_url(get_transport(root).base)
         wt = workingtree.WorkingTree.open(root)
         last_rev = wt.last_revision()
         if last_rev != 'null:':
@@ -2225,6 +2348,7 @@
             # specifically told when all tests are finished.  This will do.
             atexit.register(_rmtree_temp_dir, root)
 
+        self.permit_dir(TestCaseWithMemoryTransport.TEST_ROOT)
         self.addCleanup(self._check_safety_net)
 
     def makeAndChdirToTestDir(self):
@@ -2238,6 +2362,7 @@
         os.chdir(TestCaseWithMemoryTransport.TEST_ROOT)
         self.test_dir = TestCaseWithMemoryTransport.TEST_ROOT
         self.test_home_dir = self.test_dir + "/MemoryTransportMissingHomeDir"
+        self.permit_dir(self.test_dir)
 
     def make_branch(self, relpath, format=None):
         """Create a branch on the transport at relpath."""
@@ -2375,10 +2500,18 @@
             if os.path.exists(name):
                 name = name_prefix + '_' + str(i)
             else:
-                os.mkdir(name)
+                # now create test and home directories within this dir
+                self.test_base_dir = name
+                self.addCleanup(self.deleteTestDir)
+                os.mkdir(self.test_base_dir)
                 break
-        # now create test and home directories within this dir
-        self.test_base_dir = name
+        self.permit_dir(self.test_base_dir)
+        # 'sprouting' and 'init' of a branch both walk up the tree to find
+        # stacking policy to honour; create a bzr dir with an unshared
+        # repository (but not a branch - our code would be trying to escape
+        # then!) to stop them, and permit it to be read.
+        # control = bzrdir.BzrDir.create(self.test_base_dir)
+        # control.create_repository()
         self.test_home_dir = self.test_base_dir + '/home'
         os.mkdir(self.test_home_dir)
         self.test_dir = self.test_base_dir + '/work'
@@ -2390,7 +2523,6 @@
             f.write(self.id())
         finally:
             f.close()
-        self.addCleanup(self.deleteTestDir)
 
     def deleteTestDir(self):
         os.chdir(TestCaseWithMemoryTransport.TEST_ROOT)

=== modified file 'bzrlib/tests/blackbox/test_bound_branches.py'
--- a/bzrlib/tests/blackbox/test_bound_branches.py	2009-05-07 05:08:46 +0000
+++ b/bzrlib/tests/blackbox/test_bound_branches.py	2009-09-17 11:54:41 +0000
@@ -64,14 +64,13 @@
 class TestBoundBranches(TestCaseWithTransport):
 
     def create_branches(self):
-        self.build_tree(['base/', 'base/a', 'base/b'])
-
-        branch = self.init_meta_branch('base')
-        base_tree = branch.bzrdir.open_workingtree()
+        base_tree = self.make_branch_and_tree('base')
         base_tree.lock_write()
+        self.build_tree(['base/a', 'base/b'])
         base_tree.add(['a', 'b'])
         base_tree.commit('init')
         base_tree.unlock()
+        branch = base_tree.branch
 
         child_tree = branch.create_checkout('child')
 
@@ -86,12 +85,11 @@
             val, len(BzrDir.open(loc).open_branch().revision_history()))
 
     def test_simple_binding(self):
-        self.build_tree(['base/', 'base/a', 'base/b'])
-
-        branch = self.init_meta_branch('base')
-        tree = branch.bzrdir.open_workingtree()
+        tree = self.make_branch_and_tree('base')
+        self.build_tree(['base/a', 'base/b'])
         tree.add('a', 'b')
         tree.commit(message='init')
+        branch = tree.branch
 
         tree.bzrdir.sprout('child')
 
@@ -131,10 +129,6 @@
         error = self.run_bzr('bind', retcode=3)[1]
         self.assertContainsRe(error, 'old locations')
 
-    def init_meta_branch(self, path):
-        format = bzrdir.format_registry.make_bzrdir('default')
-        return BzrDir.create_branch_convenience(path, format=format)
-
     def test_bound_commit(self):
         child_tree = self.create_branches()[1]
 

=== modified file 'bzrlib/tests/blackbox/test_cat.py'
--- a/bzrlib/tests/blackbox/test_cat.py	2009-03-23 14:59:43 +0000
+++ b/bzrlib/tests/blackbox/test_cat.py	2009-09-17 11:54:41 +0000
@@ -22,7 +22,7 @@
 import os
 import sys
 
-from bzrlib.tests.blackbox import TestCaseWithTransport
+from bzrlib.tests import MemoryServer, TestCaseWithTransport
 
 class TestCat(TestCaseWithTransport):
 
@@ -182,8 +182,5 @@
         self.assertEqual('contents of README\n', out)
 
     def test_cat_nonexistent_branch(self):
-        if sys.platform == "win32":
-            location = "C:/i/do/not/exist"
-        else:
-            location = "/i/do/not/exist"
-        self.run_bzr_error(['^bzr: ERROR: Not a branch'], ['cat', location])
+        self.vfs_transport_factory = MemoryServer
+        self.run_bzr_error(['^bzr: ERROR: Not a branch'], ['cat', self.get_url()])

=== modified file 'bzrlib/tests/blackbox/test_info.py'
--- a/bzrlib/tests/blackbox/test_info.py	2009-08-25 23:38:10 +0000
+++ b/bzrlib/tests/blackbox/test_info.py	2009-09-17 11:54:41 +0000
@@ -29,7 +29,7 @@
     urlutils,
     )
 from bzrlib.osutils import format_date
-from bzrlib.tests import TestSkipped
+from bzrlib.tests import TestSkipped, MemoryServer
 from bzrlib.tests.blackbox import ExternalBase
 
 
@@ -40,10 +40,8 @@
         self._repo_strings = "2a or development-subtree"
 
     def test_info_non_existing(self):
-        if sys.platform == "win32":
-            location = "C:/i/do/not/exist/"
-        else:
-            location = "/i/do/not/exist/"
+        self.vfs_transport_factory = MemoryServer
+        location = self.get_url()
         out, err = self.run_bzr('info '+location, retcode=3)
         self.assertEqual(out, '')
         self.assertEqual(err, 'bzr: ERROR: Not a branch: "%s".\n' % location)

=== modified file 'bzrlib/tests/blackbox/test_mv.py'
--- a/bzrlib/tests/blackbox/test_mv.py	2009-07-16 01:36:57 +0000
+++ b/bzrlib/tests/blackbox/test_mv.py	2009-09-17 11:54:41 +0000
@@ -489,7 +489,7 @@
 
     def test_mv_readonly_lightweight_checkout(self):
         branch = self.make_branch('foo')
-        branch = bzrlib.branch.Branch.open('readonly+' + branch.base)
+        branch = bzrlib.branch.Branch.open(self.get_readonly_url('foo'))
         tree = branch.create_checkout('tree', lightweight=True)
         self.build_tree(['tree/path'])
         tree.add('path')

=== modified file 'bzrlib/tests/blackbox/test_outside_wt.py'
--- a/bzrlib/tests/blackbox/test_outside_wt.py	2009-03-23 14:59:43 +0000
+++ b/bzrlib/tests/blackbox/test_outside_wt.py	2009-09-17 11:54:41 +0000
@@ -23,6 +23,7 @@
 from bzrlib import (
     osutils,
     tests,
+    transport,
     urlutils,
     )
 
@@ -32,12 +33,12 @@
 
     def test_cwd_log(self):
         tmp_dir = osutils.mkdtemp()
+        # We expect a read-to-root attempt to occur.
+        self.permit_url('file:///')
         self.addCleanup(lambda: osutils.rmtree(tmp_dir))
-        self.addCleanup(lambda: os.chdir('..'))
-        os.chdir(tmp_dir)
-        out, err = self.run_bzr('log', retcode=3)
+        out, err = self.run_bzr('log', retcode=3, working_dir=tmp_dir)
         self.assertEqual(u'bzr: ERROR: Not a branch: "%s/".\n'
-                         % (osutils.getcwd(),),
+                         % (tmp_dir,),
                          err)
 
     def test_url_log(self):
@@ -47,34 +48,33 @@
                          u' "%s".\n' % url, err)
 
     def test_diff_outside_tree(self):
+        tree = self.make_branch_and_tree('branch1')
+        tree.commit('nothing')
+        tree.commit('nothing')
+        # A directory we can run commands from which we hope is not contained
+        # in a bzr tree (though if there is one at or above $TEMPDIR, this is
+        # false and may cause test failures).
         tmp_dir = osutils.mkdtemp()
         self.addCleanup(lambda: osutils.rmtree(tmp_dir))
-        self.addCleanup(lambda: os.chdir('..'))
-        os.chdir(tmp_dir)
-        self.run_bzr('init branch1')
-        self.run_bzr(['commit', '-m', 'nothing',
-                               '--unchanged', 'branch1'])
-        self.run_bzr(['commit', '-m', 'nothing',
-                               '--unchanged', 'branch1'])
-        this_dir = osutils.getcwd()
-        branch2 = "%s/branch2" % (this_dir,)
+        # We expect a read-to-root attempt to occur.
+        self.permit_url('file:///')
+        expected_error = u'bzr: ERROR: Not a branch: "%s/branch2/".\n' % tmp_dir
         # -r X..Y
-        out, err = self.run_bzr('diff -r revno:2:branch2..revno:1', retcode=3)
-        self.assertEquals('', out)
-        self.assertEqual(u'bzr: ERROR: Not a branch: "%s/".\n' % (branch2,),
-                         err)
+        out, err = self.run_bzr('diff -r revno:2:branch2..revno:1', retcode=3,
+            working_dir=tmp_dir)
+        self.assertEqual('', out)
+        self.assertEqual(expected_error, err)
         # -r X
-        out, err = self.run_bzr('diff -r revno:2:branch2', retcode=3)
-        self.assertEquals('', out)
-        self.assertEqual(u'bzr: ERROR: Not a branch: "%s/".\n' % (branch2,),
-                         err)
+        out, err = self.run_bzr('diff -r revno:2:branch2', retcode=3,
+            working_dir=tmp_dir)
+        self.assertEqual('', out)
+        self.assertEqual(expected_error, err)
         # -r X..
-        out, err = self.run_bzr('diff -r revno:2:branch2..', retcode=3)
-        self.assertEquals('', out)
-        self.assertEqual(u'bzr: ERROR: Not a branch: "%s/".\n' % (branch2,),
-                         err)
+        out, err = self.run_bzr('diff -r revno:2:branch2..', retcode=3,
+            working_dir=tmp_dir)
+        self.assertEqual('', out)
+        self.assertEqual(expected_error, err)
         # no -r at all.
-        out, err = self.run_bzr('diff', retcode=3)
-        self.assertEquals('', out)
-        self.assertEqual(u'bzr: ERROR: Not a branch: "%s/".\n' % (this_dir,),
-                         err)
+        out, err = self.run_bzr('diff', retcode=3, working_dir=tmp_dir)
+        self.assertEqual('', out)
+        self.assertEqual(u'bzr: ERROR: Not a branch: "%s/".\n' % tmp_dir, err)

=== modified file 'bzrlib/tests/blackbox/test_send.py'
--- a/bzrlib/tests/blackbox/test_send.py	2009-07-01 15:17:33 +0000
+++ b/bzrlib/tests/blackbox/test_send.py	2009-09-17 11:54:41 +0000
@@ -280,10 +280,8 @@
         self.assertEqual('rev3', md.revision_id)
 
     def test_nonexistant_branch(self):
-        if sys.platform == "win32":
-            location = "C:/i/do/not/exist/"
-        else:
-            location = "/i/do/not/exist/"
+        self.vfs_transport_factory = tests.MemoryServer
+        location = self.get_url('absentdir/')
         out, err = self.run_bzr(["send", "--from", location], retcode=3)
         self.assertEqual(out, '')
         self.assertEqual(err, 'bzr: ERROR: Not a branch: "%s".\n' % location)

=== modified file 'bzrlib/tests/blackbox/test_serve.py'
--- a/bzrlib/tests/blackbox/test_serve.py	2009-09-08 03:31:31 +0000
+++ b/bzrlib/tests/blackbox/test_serve.py	2009-09-17 11:54:41 +0000
@@ -85,6 +85,7 @@
         # We use this url because while this is no valid URL to connect to this
         # server instance, the transport needs a URL.
         url = 'bzr://localhost/'
+        self.permit_url(url)
         client_medium = medium.SmartSimplePipesClientMedium(
             process.stdout, process.stdin, url)
         transport = remote.RemoteTransport(url, medium=client_medium)
@@ -105,7 +106,9 @@
         prefix = 'listening on port: '
         self.assertStartsWith(port_line, prefix)
         port = int(port_line[len(prefix):])
-        return process,'bzr://localhost:%d/' % port
+        url = 'bzr://localhost:%d/' % port
+        self.permit_url(url)
+        return process, url
 
     def test_bzr_serve_inet_readonly(self):
         """bzr server should provide a read only filesystem by default."""
@@ -160,7 +163,9 @@
         bzr+ssh:// should cause bzr to run a remote bzr smart server over SSH.
         """
         try:
-            from bzrlib.transport.sftp import SFTPServer
+            # SFTPFullAbsoluteServer has a get_url method, and doesn't
+            # override the interface (doesn't change self._vendor).
+            from bzrlib.transport.sftp import SFTPFullAbsoluteServer
         except ParamikoNotPresent:
             raise TestSkipped('Paramiko not installed')
         from bzrlib.tests.stub_sftp import StubServer
@@ -206,7 +211,7 @@
 
                 return True
 
-        ssh_server = SFTPServer(StubSSHServer)
+        ssh_server = SFTPFullAbsoluteServer(StubSSHServer)
         # XXX: We *don't* want to override the default SSH vendor, so we set
         # _vendor to what _get_ssh_vendor returns.
         self.start_server(ssh_server)
@@ -224,8 +229,9 @@
         try:
             if sys.platform == 'win32':
                 path_to_branch = os.path.splitdrive(path_to_branch)[1]
-            branch = Branch.open(
-                'bzr+ssh://fred:secret@localhost:%d%s' % (port, path_to_branch))
+            url_suffix = '@localhost:%d%s' % (port, path_to_branch)
+            self.permit_url('bzr+ssh://fred' + url_suffix)
+            branch = Branch.open('bzr+ssh://fred:secret' + url_suffix)
             self.make_read_requests(branch)
             # Check we can perform write operations
             branch.bzrdir.root_transport.mkdir('foo')

=== modified file 'bzrlib/tests/blackbox/test_too_much.py'
--- a/bzrlib/tests/blackbox/test_too_much.py	2009-08-20 04:09:58 +0000
+++ b/bzrlib/tests/blackbox/test_too_much.py	2009-09-17 11:54:41 +0000
@@ -112,19 +112,6 @@
         self.run_bzr('revert')
         os.chdir('..')
 
-    def test_main_version(self):
-        """Check output from version command and master option is reasonable"""
-        # output is intentionally passed through to stdout so that we
-        # can see the version being tested
-        output = self.run_bzr('version')[0]
-        self.log('bzr version output:')
-        self.log(output)
-        self.assert_(output.startswith('Bazaar (bzr) '))
-        self.assertNotEqual(output.index('Canonical'), -1)
-        # make sure --version is consistent
-        tmp_output = self.run_bzr('--version')[0]
-        self.assertEquals(output, tmp_output)
-
     def example_branch(test):
         test.run_bzr('init')
         file('hello', 'wt').write('foo')

=== modified file 'bzrlib/tests/blackbox/test_version.py'
--- a/bzrlib/tests/blackbox/test_version.py	2009-03-23 14:59:43 +0000
+++ b/bzrlib/tests/blackbox/test_version.py	2009-09-17 11:54:41 +0000
@@ -31,7 +31,22 @@
 
 class TestVersion(TestCase):
 
+    def test_main_version(self):
+        """Check output from version command and master option is reasonable"""
+        # output is intentionally passed through to stdout so that we
+        # can see the version being tested
+        self.permit_source_tree_branch_repo()
+        output = self.run_bzr('version')[0]
+        self.log('bzr version output:')
+        self.log(output)
+        self.assert_(output.startswith('Bazaar (bzr) '))
+        self.assertNotEqual(output.index('Canonical'), -1)
+        # make sure --version is consistent
+        tmp_output = self.run_bzr('--version')[0]
+        self.assertEquals(output, tmp_output)
+
     def test_version(self):
+        self.permit_source_tree_branch_repo()
         out = self.run_bzr("version")[0]
         self.assertTrue(len(out) > 0)
         self.assertEqualDiff(out.splitlines()[0],
@@ -43,6 +58,7 @@
         self.assertContainsRe(out, r'(?m)^  Bazaar log file:.*\.bzr\.log')
 
     def test_version_short(self):
+        self.permit_source_tree_branch_repo()
         out = self.run_bzr(["version", "--short"])[0]
         self.assertEqualDiff(out, bzrlib.version_string + '\n')
 
@@ -50,6 +66,7 @@
 class TestVersionUnicodeOutput(TestCaseInTempDir):
 
     def _check(self, args):
+        self.permit_source_tree_branch_repo()
         # Even though trace._bzr_log_filename variable
         # is used only to keep actual log filename
         # and changing this variable in selftest
@@ -79,6 +96,7 @@
                               ' encoding %s' % (osutils.get_user_encoding(),))
 
         osutils.set_or_unset_env('BZR_HOME', str_val)
+        self.permit_source_tree_branch_repo()
         out = self.run_bzr("version")[0]
         self.assertTrue(len(out) > 0)
         self.assertContainsRe(out, r"(?m)^  Bazaar configuration: " + str_val)
@@ -98,6 +116,8 @@
         self.failUnlessExists(bzr_log)
 
     def test_dev_null(self):
+        # This test uses a subprocess to cause the log opening logic to
+        # execute. It would be better to just execute that logic directly.
         if sys.platform == 'win32':
             bzr_log = 'NUL'
         else:

=== modified file 'bzrlib/tests/per_repository/test_repository.py'
--- a/bzrlib/tests/per_repository/test_repository.py	2009-09-15 01:52:34 +0000
+++ b/bzrlib/tests/per_repository/test_repository.py	2009-09-17 11:54:41 +0000
@@ -52,6 +52,7 @@
     )
 from bzrlib.tests.per_repository import TestCaseWithRepository
 from bzrlib.transport import get_transport
+from bzrlib.transport.fakevfat import FakeVFATServer
 from bzrlib.upgrade import upgrade
 from bzrlib.workingtree import WorkingTree
 
@@ -1295,6 +1296,7 @@
         from bzrlib.remote import RemoteRepositoryFormat
         if isinstance(self.repository_format, RemoteRepositoryFormat):
             return
+        self.transport_server = FakeVFATServer
         FOO_ID = 'foo<:>ID'
         REV_ID = 'revid-1'
         # this makes a default format repository always, which is wrong:
@@ -1306,7 +1308,7 @@
         wt.add(['foo'], [FOO_ID])
         wt.commit('this is my new commit', rev_id=REV_ID)
         # now access over vfat; should be safe
-        branch = bzrdir.BzrDir.open('vfat+' + self.get_url('repo')).open_branch()
+        branch = bzrdir.BzrDir.open(self.get_url('repo')).open_branch()
         revtree = branch.repository.revision_tree(REV_ID)
         revtree.lock_read()
         self.addCleanup(revtree.unlock)

=== modified file 'bzrlib/tests/test_bzrdir.py'
--- a/bzrlib/tests/test_bzrdir.py	2009-07-10 07:14:02 +0000
+++ b/bzrlib/tests/test_bzrdir.py	2009-09-17 11:54:41 +0000
@@ -1152,7 +1152,9 @@
     _transport = HttpTransport_urllib
 
     def _qualified_url(self, host, port):
-        return 'http+urllib://%s:%s' % (host, port)
+        result = 'http+urllib://%s:%s' % (host, port)
+        self.permit_url(result)
+        return result
 
 
 
@@ -1162,7 +1164,9 @@
     """Tests redirections for pycurl implementation"""
 
     def _qualified_url(self, host, port):
-        return 'http+pycurl://%s:%s' % (host, port)
+        result = 'http+pycurl://%s:%s' % (host, port)
+        self.permit_url(result)
+        return result
 
 
 class TestHTTPRedirections_nosmart(TestHTTPRedirections,
@@ -1172,7 +1176,9 @@
     _transport = NoSmartTransportDecorator
 
     def _qualified_url(self, host, port):
-        return 'nosmart+http://%s:%s' % (host, port)
+        result = 'nosmart+http://%s:%s' % (host, port)
+        self.permit_url(result)
+        return result
 
 
 class TestHTTPRedirections_readonly(TestHTTPRedirections,
@@ -1182,7 +1188,9 @@
     _transport = ReadonlyTransportDecorator
 
     def _qualified_url(self, host, port):
-        return 'readonly+http://%s:%s' % (host, port)
+        result = 'readonly+http://%s:%s' % (host, port)
+        self.permit_url(result)
+        return result
 
 
 class TestDotBzrHidden(TestCaseWithTransport):

=== modified file 'bzrlib/tests/test_http.py'
--- a/bzrlib/tests/test_http.py	2009-08-27 22:17:35 +0000
+++ b/bzrlib/tests/test_http.py	2009-09-17 11:54:41 +0000
@@ -204,7 +204,7 @@
     It records the bytes sent to it, and replies with a 200.
     """
 
-    def __init__(self, expect_body_tail=None):
+    def __init__(self, expect_body_tail=None, scheme=''):
         """Constructor.
 
         :type expect_body_tail: str
@@ -215,6 +215,10 @@
         self.host = None
         self.port = None
         self.received_bytes = ''
+        self.scheme = scheme
+
+    def get_url(self):
+        return '%s://%s:%s/' % (self.scheme, self.host, self.port)
 
     def setUp(self):
         self._sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
@@ -541,10 +545,10 @@
 class TestPost(tests.TestCase):
 
     def test_post_body_is_received(self):
-        server = RecordingServer(expect_body_tail='end-of-body')
+        server = RecordingServer(expect_body_tail='end-of-body',
+            scheme=self._qualified_prefix)
         self.start_server(server)
-        scheme = self._qualified_prefix
-        url = '%s://%s:%s/' % (scheme, server.host, server.port)
+        url = server.get_url()
         http_transport = self._transport(url)
         code, response = http_transport._post('abc def end-of-body')
         self.assertTrue(
@@ -776,7 +780,7 @@
         self.assertEqual(None, server.port)
 
     def test_send_receive_bytes(self):
-        server = RecordingServer(expect_body_tail='c')
+        server = RecordingServer(expect_body_tail='c', scheme='http')
         self.start_server(server)
         sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
         sock.connect((server.host, server.port))

=== modified file 'bzrlib/tests/test_remote.py'
--- a/bzrlib/tests/test_remote.py	2009-09-15 01:52:34 +0000
+++ b/bzrlib/tests/test_remote.py	2009-09-17 11:54:41 +0000
@@ -689,7 +689,9 @@
         # fallback all the way to the first version.
         reference_format = self.get_repo_format()
         network_name = reference_format.network_name()
-        client = FakeClient('bzr://example.com/')
+        server_url = 'bzr://example.com/'
+        self.permit_url(server_url)
+        client = FakeClient(server_url)
         client.add_unknown_method_response('BzrDir.find_repositoryV3')
         client.add_unknown_method_response('BzrDir.find_repositoryV2')
         client.add_success_response('ok', '', 'no', 'no')
@@ -701,7 +703,7 @@
             reference_format.get_format_string(), 'ok')
         # PackRepository wants to do a stat
         client.add_success_response('stat', '0', '65535')
-        remote_transport = RemoteTransport('bzr://example.com/quack/', medium=False,
+        remote_transport = RemoteTransport(server_url + 'quack/', medium=False,
             _client=client)
         bzrdir = RemoteBzrDir(remote_transport, remote.RemoteBzrDirFormat(),
             _client=client)
@@ -721,7 +723,9 @@
         # fallback to find_repositoryV2
         reference_format = self.get_repo_format()
         network_name = reference_format.network_name()
-        client = FakeClient('bzr://example.com/')
+        server_url = 'bzr://example.com/'
+        self.permit_url(server_url)
+        client = FakeClient(server_url)
         client.add_unknown_method_response('BzrDir.find_repositoryV3')
         client.add_success_response('ok', '', 'no', 'no', 'no')
         # A real repository instance will be created to determine the network
@@ -732,7 +736,7 @@
             reference_format.get_format_string(), 'ok')
         # PackRepository wants to do a stat
         client.add_success_response('stat', '0', '65535')
-        remote_transport = RemoteTransport('bzr://example.com/quack/', medium=False,
+        remote_transport = RemoteTransport(server_url + 'quack/', medium=False,
             _client=client)
         bzrdir = RemoteBzrDir(remote_transport, remote.RemoteBzrDirFormat(),
             _client=client)

=== modified file 'bzrlib/tests/test_selftest.py'
--- a/bzrlib/tests/test_selftest.py	2009-09-09 15:27:48 +0000
+++ b/bzrlib/tests/test_selftest.py	2009-09-17 11:54:41 +0000
@@ -38,6 +38,7 @@
     repository,
     symbol_versioning,
     tests,
+    transport,
     workingtree,
     )
 from bzrlib.repofmt import (
@@ -576,19 +577,6 @@
                          self.get_transport().get_bytes(
                             'dir/.bzr/repository/format'))
 
-    def test_safety_net(self):
-        """No test should modify the safety .bzr directory.
-
-        We just test that the _check_safety_net private method raises
-        AssertionError, it's easier than building a test suite with the same
-        test.
-        """
-        # Oops, a commit in the current directory (i.e. without local .bzr
-        # directory) will crawl up the hierarchy to find a .bzr directory.
-        self.run_bzr(['commit', '-mfoo', '--unchanged'])
-        # But we have a safety net in place.
-        self.assertRaises(AssertionError, self._check_safety_net)
-
     def test_dangling_locks_cause_failures(self):
         class TestDanglingLock(tests.TestCaseWithMemoryTransport):
             def test_function(self):
@@ -739,7 +727,19 @@
         self.check_timing(ShortDelayTestCase('test_short_delay'),
                           r"^ +[0-9]+ms$")
 
+    def _patch_get_bzr_source_tree(self):
+        # Reading from the actual source tree breaks isolation, but we don't
+        # want to assume that thats *all* that would happen.
+        def _get_bzr_source_tree():
+            return None
+        orig_get_bzr_source_tree = bzrlib.version._get_bzr_source_tree
+        bzrlib.version._get_bzr_source_tree = _get_bzr_source_tree
+        def restore():
+            bzrlib.version._get_bzr_source_tree = orig_get_bzr_source_tree
+        self.addCleanup(restore)
+
     def test_assigned_benchmark_file_stores_date(self):
+        self._patch_get_bzr_source_tree()
         output = StringIO()
         result = bzrlib.tests.TextTestResult(self._log_file,
                                         descriptions=0,
@@ -753,6 +753,7 @@
         self.assertContainsRe(output_string, "--date [0-9.]+")
 
     def test_benchhistory_records_test_times(self):
+        self._patch_get_bzr_source_tree()
         result_stream = StringIO()
         result = bzrlib.tests.TextTestResult(
             self._log_file,
@@ -1165,11 +1166,24 @@
             ],
             lines[-3:])
 
+    def _patch_get_bzr_source_tree(self):
+        # Reading from the actual source tree breaks isolation, but we don't
+        # want to assume that thats *all* that would happen.
+        self._get_source_tree_calls = []
+        def _get_bzr_source_tree():
+            self._get_source_tree_calls.append("called")
+            return None
+        orig_get_bzr_source_tree = bzrlib.version._get_bzr_source_tree
+        bzrlib.version._get_bzr_source_tree = _get_bzr_source_tree
+        def restore():
+            bzrlib.version._get_bzr_source_tree = orig_get_bzr_source_tree
+        self.addCleanup(restore)
+
     def test_bench_history(self):
-        # tests that the running the benchmark produces a history file
-        # containing a timestamp and the revision id of the bzrlib source which
-        # was tested.
-        workingtree = _get_bzr_source_tree()
+        # tests that the running the benchmark passes bench_history into
+        # the test result object. We can tell that happens if
+        # _get_bzr_source_tree is called.
+        self._patch_get_bzr_source_tree()
         test = TestRunner('dummy_test')
         output = StringIO()
         runner = tests.TextTestRunner(stream=self._log_file,
@@ -1177,9 +1191,7 @@
         result = self.run_test_runner(runner, test)
         output_string = output.getvalue()
         self.assertContainsRe(output_string, "--date [0-9.]+")
-        if workingtree is not None:
-            revision_id = workingtree.get_parent_ids()[0]
-            self.assertEndsWith(output_string.rstrip(), revision_id)
+        self.assertLength(1, self._get_source_tree_calls)
 
     def assertLogDeleted(self, test):
         log = test._get_log()
@@ -1548,6 +1560,23 @@
         """Self.knownFailure() should raise a KnownFailure exception."""
         self.assertRaises(tests.KnownFailure, self.knownFailure, "A Failure")
 
+    def test_open_bzrdir_safe_roots(self):
+        # even a memory transport should fail to open when its url isn't 
+        # permitted.
+        # Manually set one up (TestCase doesn't and shouldn't provide magic
+        # machinery)
+        transport_server = MemoryServer()
+        transport_server.setUp()
+        self.addCleanup(transport_server.tearDown)
+        t = transport.get_transport(transport_server.get_url())
+        bzrdir.BzrDir.create(t.base)
+        self.assertRaises(errors.BzrError,
+            bzrdir.BzrDir.open_from_transport, t)
+        # But if we declare this as safe, we can open the bzrdir.
+        self.permit_url(t.base)
+        self._bzr_selftest_roots.append(t.base)
+        bzrdir.BzrDir.open_from_transport(t)
+
     def test_requireFeature_available(self):
         """self.requireFeature(available) is a no-op."""
         class Available(tests.Feature):
@@ -1620,6 +1649,14 @@
             ],
             result.calls)
 
+    def test_start_server_registers_url(self):
+        transport_server = MemoryServer()
+        # A little strict, but unlikely to be changed soon.
+        self.assertEqual([], self._bzr_selftest_roots)
+        self.start_server(transport_server)
+        self.assertSubset([transport_server.get_url()],
+            self._bzr_selftest_roots)
+
     def test_assert_list_raises_on_generator(self):
         def generator_which_will_raise():
             # This will not raise until after the first yield
@@ -2632,7 +2669,12 @@
         # Running bzr in blackbox mode, normal/expected/user errors should be
         # caught in the regular way and turned into an error message plus exit
         # code.
-        out, err = self.run_bzr(["log", "/nonexistantpath"], retcode=3)
+        transport_server = MemoryServer()
+        transport_server.setUp()
+        self.addCleanup(transport_server.tearDown)
+        url = transport_server.get_url()
+        self.permit_url(url)
+        out, err = self.run_bzr(["log", "%s/nonexistantpath" % url], retcode=3)
         self.assertEqual(out, '')
         self.assertContainsRe(err,
             'bzr: ERROR: Not a branch: ".*nonexistantpath/".\n')

=== modified file 'bzrlib/tests/test_smart_transport.py'
--- a/bzrlib/tests/test_smart_transport.py	2009-09-14 01:48:19 +0000
+++ b/bzrlib/tests/test_smart_transport.py	2009-09-17 11:54:41 +0000
@@ -993,6 +993,9 @@
 
         :param readonly: Create a readonly server.
         """
+        # NB: Tests using this fall into two categories: tests of the server,
+        # tests wanting a server. The latter should be updated to use
+        # self.vfs_transport_factory etc.
         if not backing_transport:
             self.backing_transport = memory.MemoryTransport()
         else:
@@ -1004,6 +1007,7 @@
         self.server.start_background_thread('-' + self.id())
         self.transport = remote.RemoteTCPTransport(self.server.get_url())
         self.addCleanup(self.tearDownServer)
+        self.permit_url(self.server.get_url())
 
     def tearDownServer(self):
         if getattr(self, 'transport', None):

=== modified file 'bzrlib/tests/test_version.py'
--- a/bzrlib/tests/test_version.py	2009-03-23 14:59:43 +0000
+++ b/bzrlib/tests/test_version.py	2009-09-17 11:54:41 +0000
@@ -24,6 +24,7 @@
 
     def test_get_bzr_source_tree(self):
         """Get tree for bzr source, if any."""
+        self.permit_source_tree_branch_repo()
         # We don't know if these tests are being run from a checkout or branch
         # of bzr, from an installed copy, or from source unpacked from a
         # tarball.  We don't construct a branch just for testing this, so we

=== modified file 'bzrlib/tests/transport_util.py'
--- a/bzrlib/tests/transport_util.py	2009-03-23 14:59:43 +0000
+++ b/bzrlib/tests/transport_util.py	2009-09-17 11:54:41 +0000
@@ -33,6 +33,7 @@
 
 from bzrlib.transport import (
     ConnectedTransport,
+    get_transport,
     register_transport,
     register_urlparse_netloc_protocol,
     unregister_transport,
@@ -106,6 +107,16 @@
         self.addCleanup(unregister)
         super(TestCaseWithConnectionHookedTransport, self).setUp()
         self.reset_connections()
+        # Add the 'hooked' url to the permitted url list.
+        # XXX: See TestCase.start_server. This whole module shouldn't need to
+        # exist - a bug has been filed on that. once its cleanedup/removed, the
+        # standard test support code will work and permit the server url
+        # correctly.
+        url = self.get_url()
+        t = get_transport(url)
+        if t.base.endswith('work/'):
+            t = t.clone('../..')
+        self.permit_url(t.base)
 
     def get_url(self, relpath=None):
         super_self = super(TestCaseWithConnectionHookedTransport, self)




More information about the bazaar-commits mailing list