HPSS: Other test changes
John Arbash Meinel
john at arbash-meinel.com
Tue Apr 17 19:52:21 BST 2007
Andrew Bennetts wrote:
> This subset of the full hpss diff has all the testing-related changes not in the
> first two subsets (new server commands, client-side objects). Mostly this is
> tweaks to existing tests.
>
> -Andrew.
>
>
>
> ------------------------------------------------------------------------
>
> === modified file 'bzrlib/repository.py'
> --- bzrlib/repository.py 2007-04-13 05:06:23 +0000
> +++ bzrlib/repository.py 2007-04-16 05:57:09 +0000
> @@ -1703,9 +1705,11 @@
> to make it easy to identify.
> """
>
> - def __init__(self, transport_server, transport_readonly_server, formats):
> + def __init__(self, transport_server, transport_readonly_server, formats,
> + vfs_transport_factory=None):
^- I'm pretty sure PEP8 says that this should be indented to the (
def __init__(self, transport_server, transport_readonly_server, formats,
vfs_transport_factory=None):
I know *I* prefer it that way.
Especially when you don't have a doc string, it makes it more obvious
where the code starts.
> self._transport_server = transport_server
> self._transport_readonly_server = transport_readonly_server
> + self._vfs_transport_factory = vfs_transport_factory
> self._formats = formats
>
> def adapt(self, test):
> @@ -1715,6 +1719,8 @@
> new_test = deepcopy(test)
> new_test.transport_server = self._transport_server
> new_test.transport_readonly_server = self._transport_readonly_server
> + if self._vfs_transport_factory:
> + new_test.vfs_transport_factory = self._vfs_transport_factory
> new_test.bzrdir_format = bzrdir_format
> new_test.repository_format = repository_format
> def make_new_test_id():
^- Why do you need the "if()" check here. And why are you setting ".vfs"
from "._vfs" ? It seems like you would set "new_test._vfs... = self._vfs..."
>
> === modified file 'bzrlib/tests/__init__.py'
> --- bzrlib/tests/__init__.py 2007-04-12 00:59:53 +0000
> +++ bzrlib/tests/__init__.py 2007-04-16 05:57:09 +0000
> @@ -2196,6 +2196,7 @@
> 'bzrlib.tests.test_progress',
> 'bzrlib.tests.test_reconcile',
> 'bzrlib.tests.test_registry',
> + 'bzrlib.tests.test_remote',
> 'bzrlib.tests.test_repository',
> 'bzrlib.tests.test_revert',
> 'bzrlib.tests.test_revision',
> @@ -2206,13 +2207,13 @@
> 'bzrlib.tests.test_selftest',
> 'bzrlib.tests.test_setup',
> 'bzrlib.tests.test_sftp_transport',
> + 'bzrlib.tests.test_smart',
> 'bzrlib.tests.test_smart_add',
> 'bzrlib.tests.test_smart_transport',
> 'bzrlib.tests.test_source',
> 'bzrlib.tests.test_ssh_transport',
> 'bzrlib.tests.test_status',
> 'bzrlib.tests.test_store',
> - 'bzrlib.tests.test_strace',
> 'bzrlib.tests.test_subsume',
> 'bzrlib.tests.test_symbol_versioning',
> 'bzrlib.tests.test_tag',
>
> === modified file 'bzrlib/tests/branch_implementations/__init__.py'
> --- bzrlib/tests/branch_implementations/__init__.py 2007-03-28 13:33:58 +0000
> +++ bzrlib/tests/branch_implementations/__init__.py 2007-04-16 05:57:08 +0000
> @@ -30,7 +30,6 @@
> )
> from bzrlib.tests import (
> adapt_modules,
> - default_transport,
> TestLoader,
> TestSuite,
> )
> @@ -62,11 +61,31 @@
> combinations = [(format, format._matchingbzrdir) for format in
> BranchFormat._formats.values() + _legacy_formats]
> adapter = BranchTestProviderAdapter(
> - default_transport,
> + # None here will cause the default vfs transport server to be used.
> + None,
> # None here will cause a readonly decorator to be created
> # by the TestCaseWithTransport.get_readonly_transport method.
> None,
> combinations)
> loader = TestLoader()
> adapt_modules(test_branch_implementations, adapter, loader, result)
...
Do you really want to do another import here? Shouldn't they at least
all be done at the beginning of the function
> +
> +
> + from bzrlib.smart.server import (
> + SmartTCPServer_for_testing,
> + ReadonlySmartTCPServer_for_testing,
> + )
> + from bzrlib.remote import RemoteBranchFormat, RemoteBzrDirFormat
> + from bzrlib.transport.memory import MemoryServer
> + adapt_to_smart_server = BranchTestProviderAdapter(
> + SmartTCPServer_for_testing,
> + ReadonlySmartTCPServer_for_testing,
> + [(RemoteBranchFormat(), RemoteBzrDirFormat())],
> + MemoryServer
> + )
> + adapt_modules(test_branch_implementations,
> + adapt_to_smart_server,
> + loader,
> + result)
> +
> return result
>
> === modified file 'bzrlib/tests/branch_implementations/test_parent.py'
> --- bzrlib/tests/branch_implementations/test_parent.py 2007-04-13 07:18:57 +0000
> +++ bzrlib/tests/branch_implementations/test_parent.py 2007-04-16 05:57:08 +0000
> @@ -19,6 +19,7 @@
> import os
> import sys
>
> +from bzrlib import urlutils
> from bzrlib.branch import Branch
> import bzrlib.errors
> from bzrlib.osutils import abspath, realpath, getcwd
^- You do the import, but don't actually use it. (At least there is no
change to the file itself)
...
...
> @@ -242,7 +243,7 @@
> tree.bzrdir.open_branch().set_revision_history([])
> tree.set_parent_trees([])
> tree.commit('revision 2', rev_id='2')
> - tree.bzrdir.open_repository().copy_content_into(shared_repo)
> + tree.branch.bzrdir.open_repository().copy_content_into(shared_repo)
^- This looks rather bogus no matter which form it was using. Why not just
tree.branch.repository.copy_content_into()?
v- Same later on
> dir = self.make_bzrdir('shared/source')
> dir.create_branch()
> target = dir.clone(self.get_url('shared/target'))
> @@ -256,15 +257,16 @@
> except errors.IncompatibleFormat:
> return
> tree = self.make_branch_and_tree('commit_tree')
> - self.build_tree(['foo'], transport=tree.bzrdir.transport.clone('..'))
> + self.build_tree(['commit_tree/foo'])
> tree.add('foo')
> tree.commit('revision 1', rev_id='1')
> - tree.bzrdir.open_branch().set_revision_history([])
> + tree.branch.bzrdir.open_branch().set_revision_history([])
> tree.set_parent_trees([])
> tree.commit('revision 2', rev_id='2')
> - tree.bzrdir.open_repository().copy_content_into(shared_repo)
> - shared_repo.set_make_working_trees(False)
> - self.assertFalse(shared_repo.make_working_trees())
> + tree.branch.bzrdir.open_repository().copy_content_into(shared_repo)
> + if shared_repo.make_working_trees():
> + shared_repo.set_make_working_trees(False)
> + self.assertFalse(shared_repo.make_working_trees())
> self.assertTrue(shared_repo.has_revision('1'))
> dir = self.make_bzrdir('shared/source')
> dir.create_branch()
>
> def test_sprout_bzrdir_branch_and_repo(self):
> tree = self.make_branch_and_tree('commit_tree')
> - self.build_tree(['foo'], transport=tree.bzrdir.transport.clone('..'))
> + self.build_tree(['commit_tree/foo'])
> tree.add('foo')
> tree.commit('revision 1')
> source = self.make_branch('source')
> - tree.bzrdir.open_repository().copy_content_into(source.repository)
> + tree.branch.bzrdir.open_repository().copy_content_into(source.repository)
> tree.bzrdir.open_branch().copy_content_into(source)
> dir = source.bzrdir
> target = self.sproutOrSkip(dir, self.get_url('target'))
> @@ -723,11 +732,11 @@
> # sprouting a branch with a repo into a shared repo uses the shared
> # repo
> tree = self.make_branch_and_tree('commit_tree')
> - self.build_tree(['foo'], transport=tree.bzrdir.transport.clone('..'))
> + self.build_tree(['commit_tree/foo'])
> tree.add('foo')
> tree.commit('revision 1', rev_id='1')
> source = self.make_branch('source')
> - tree.bzrdir.open_repository().copy_content_into(source.repository)
> + tree.branch.bzrdir.open_repository().copy_content_into(source.repository)
> tree.bzrdir.open_branch().copy_content_into(source)
> dir = source.bzrdir
> try:
Otherwise (modulo Robert's comments) it seems good.
John
=:->
More information about the bazaar
mailing list