Merge lp:~lifeless/bzr/test-speed into lp:~bzr/bzr/trunk-old

[Robert Collins]

[Robert Collins (lifeless)]

This branch:
* Use copy, not deepcopy, to clone tests in test parameterisation.
* Make tests for finish_bzr_subprocess that really only care about the
   interface use StubProcess.
* Remove duplicated test_start_and_stop_working_dir test.
* Remove unnecessary use of an SFTP server connection to test the
   behaviour of TestCase.make_branch_and_tree.
* Refactor test_suite to make stubbing out the list of tests to load
possible without sacrificing coverage.

-Rob

[John A Meinel (jameinel)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Robert Collins wrote:
> Robert Collins has proposed merging lp:~lifeless/bzr/test-speed into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
>
> This branch:
> * Use copy, not deepcopy, to clone tests in test parameterisation.

^- My primary concern is whether this is actually valid. Obviously you
believe so or you wouldn't have done it.

My understanding is that the main difference is things like:

 l = [[1]]
 lcopy = copy(l)
 ldeepcopy = deepcopy(l)
 l[0] is lcopy[0]
 l[0] is not ldeepcopy[0]

Now, I don't think we store any state in member variables that would end
up persisting between tests. I suppose if we did then you would get test
failures?

How much of an impact does this actually have? If it is .02s out 10s
loading, then it isn't really worthwhile versus the safety. If it is
more than 10% then it seems reasonable.

> * Make tests for finish_bzr_subprocess that really only care about the
> interface use StubProcess.
> * Remove duplicated test_start_and_stop_working_dir test.
> * Remove unnecessary use of an SFTP server connection to test the
> behaviour of TestCase.make_branch_and_tree.
> * Refactor test_suite to make stubbing out the list of tests to load
> possible without sacrificing coverage.

This seems like a lot of churn that is somewhat likely to cause
conflicts (as the data has now been moved). If you find it a lot easier,
I guess that is ok. I've always found:

^V B B B I #

(block select, down page, insert before, comment char)

to work well for stubbing out lots of tests at once.

You can even use

^V % I #

(jump to matching parenthesis/brace/etc) to block out the whole group in
just a couple keystrokes. And I know you use ViM :)

>
> -Rob

Anyway, other than the copy/deepcopy question, this doesn't seem to
reduce code coverage (note that I didn't check to make sure all the test
names were still present after the move.) So I'll defer to your
judgement about the rest.

 review: approve

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkqVQ84ACgkQJdeBCYSNAANDswCfRoctFvjboQvSDWaPkqdN5zuo
0y8AnREcXttKeXhpdj71zA/KpgMRRsZg
=aRNZ
-----END PGP SIGNATURE-----

review: Approve

[Vincent Ladeuil (vila)]

> This branch:
> * Use copy, not deepcopy, to clone tests in test parameterisation.
> * Make tests for finish_bzr_subprocess that really only care about the
> interface use StubProcess.
> * Remove duplicated test_start_and_stop_working_dir test.
> * Remove unnecessary use of an SFTP server connection to test the
> behaviour of TestCase.make_branch_and_tree.
> * Refactor test_suite to make stubbing out the list of tests to load
> possible without sacrificing coverage.
>

Cough, that's two or three submissions crammed into one, here.

I approve them all, but yet.

I think the copy usage is fine, setUp() is really the test constructor so nothing occurring before that should have an impact on a test.

That may be worth adding a comment though.

You may want to check if your subprocess modifications (removing some bzr --version abusive uses) have addressed the bug about running the test suite in a bound branch.

review: Approve

[Robert Collins (lifeless)]

On Wed, 2009-08-26 at 14:18 +0000, John A Meinel wrote:
> Review: Approve
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Robert Collins wrote:
> > Robert Collins has proposed merging lp:~lifeless/bzr/test-speed into lp:bzr.
> >
> > Requested reviews:
> > bzr-core (bzr-core)
> >
> > This branch:
> > * Use copy, not deepcopy, to clone tests in test parameterisation.
>
> ^- My primary concern is whether this is actually valid. Obviously you
> believe so or you wouldn't have done it.

Its only the test objects we copy:
so
foo = Test("method")
case1 = deepcopy(foo)
apply_attributes_here
becomes
foo = Test("method")
case1 = copy(foo)
apply_attributes_here

There aren't any attributes on the test at all; attributes on the class
will still be on the class -
foo.bar = gam
wouldn't be affecting them anyway.

So its pretty safe.

> > * Refactor test_suite to make stubbing out the list of tests to load
> > possible without sacrificing coverage.
>
> This seems like a lot of churn that is somewhat likely to cause
> conflicts (as the data has now been moved). If you find it a lot easier,
> I guess that is ok. I've always found:

Its stubbed out by the test suite now, rather than by hand ;). So editor
stubbing is a bit different :).

> Anyway, other than the copy/deepcopy question, this doesn't seem to
> reduce code coverage (note that I didn't check to make sure all the test
> names were still present after the move.) So I'll defer to your
> judgement about the rest.

Thanks, landing.

-Rob

=== modified file 'NEWS'
--- NEWS	2009-08-26 10:24:28 +0000
+++ NEWS	2009-08-26 19:35:12 +0000
@@ -103,6 +103,10 @@
 * Passing ``--lsprof-tests -v`` to bzr selftest will cause lsprof output to
   be output for every test. Note that this is very verbose! (Robert Collins)
 
+* Test parameterisation now does a shallow copy, not a deep copy of the test
+  to be parameterised. This is not expected to break external use of test
+  parameterisation, and is substantially faster. (Robert Collins)
+
 bzr 1.18
 ########
 

=== modified file 'bzrlib/tests/__init__.py'
--- bzrlib/tests/__init__.py	2009-08-25 05:04:05 +0000
+++ bzrlib/tests/__init__.py	2009-08-26 19:35:13 +0000
@@ -28,6 +28,7 @@
 
 import atexit
 import codecs
+from copy import copy
 from cStringIO import StringIO
 import difflib
 import doctest
@@ -2498,6 +2499,15 @@
         repository will also be accessed locally. Otherwise a lightweight
         checkout is created and returned.
 
+        We do this because we can't physically create a tree in the local
+        path, with a branch reference to the transport_factory url, and
+        a branch + repository in the vfs_transport, unless the vfs_transport
+        namespace is distinct from the local disk - the two branch objects
+        would collide. While we could construct a tree with its branch object
+        pointing at the transport_factory transport in memory, reopening it
+        would behaving unexpectedly, and has in the past caused testing bugs
+        when we tried to do it that way.
+
         :param format: The BzrDirFormat.
         :returns: the WorkingTree.
         """
@@ -3477,6 +3487,206 @@
 test_prefix_alias_registry.register('bp', 'bzrlib.plugins')
 
 
+def _test_suite_testmod_names():
+    """Return the standard list of test module names to test."""
+    return [
+        'bzrlib.doc',
+        'bzrlib.tests.blackbox',
+        'bzrlib.tests.commands',
+        'bzrlib.tests.per_branch',
+        'bzrlib.tests.per_bzrdir',
+        'bzrlib.tests.per_interrepository',
+        'bzrlib.tests.per_intertree',
+        'bzrlib.tests.per_inventory',
+        'bzrlib.tests.per_interbranch',
+        'bzrlib.tests.per_lock',
+        'bzrlib.tests.per_transport',
+        'bzrlib.tests.per_tree',
+        'bzrlib.tests.per_pack_repository',
+        'bzrlib.tests.per_repository',
+        'bzrlib.tests.per_repository_chk',
+        'bzrlib.tests.per_repository_reference',
+        'bzrlib.tests.per_versionedfile',
+        'bzrlib.tests.per_workingtree',
+        'bzrlib.tests.test__annotator',
+        'bzrlib.tests.test__chk_map',
+        'bzrlib.tests.test__dirstate_helpers',
+        'bzrlib.tests.test__groupcompress',
+        'bzrlib.tests.test__known_graph',
+        'bzrlib.tests.test__rio',
+        'bzrlib.tests.test__walkdirs_win32',
+        'bzrlib.tests.test_ancestry',
+        'bzrlib.tests.test_annotate',
+        'bzrlib.tests.test_api',
+        'bzrlib.tests.test_atomicfile',
+        'bzrlib.tests.test_bad_files',
+        'bzrlib.tests.test_bencode',
+        'bzrlib.tests.test_bisect_multi',
+        'bzrlib.tests.test_branch',
+        'bzrlib.tests.test_branchbuilder',
+        'bzrlib.tests.test_btree_index',
+        'bzrlib.tests.test_bugtracker',
+        'bzrlib.tests.test_bundle',
+        'bzrlib.tests.test_bzrdir',
+        'bzrlib.tests.test__chunks_to_lines',
+        'bzrlib.tests.test_cache_utf8',
+        'bzrlib.tests.test_chk_map',
+        'bzrlib.tests.test_chk_serializer',
+        'bzrlib.tests.test_chunk_writer',
+        'bzrlib.tests.test_clean_tree',
+        'bzrlib.tests.test_commands',
+        'bzrlib.tests.test_commit',
+        'bzrlib.tests.test_commit_merge',
+        'bzrlib.tests.test_config',
+        'bzrlib.tests.test_conflicts',
+        'bzrlib.tests.test_counted_lock',
+        'bzrlib.tests.test_crash',
+        'bzrlib.tests.test_decorators',
+        'bzrlib.tests.test_delta',
+        'bzrlib.tests.test_debug',
+        'bzrlib.tests.test_deprecated_graph',
+        'bzrlib.tests.test_diff',
+        'bzrlib.tests.test_directory_service',
+        'bzrlib.tests.test_dirstate',
+        'bzrlib.tests.test_email_message',
+        'bzrlib.tests.test_eol_filters',
+        'bzrlib.tests.test_errors',
+        'bzrlib.tests.test_export',
+        'bzrlib.tests.test_extract',
+        'bzrlib.tests.test_fetch',
+        'bzrlib.tests.test_fifo_cache',
+        'bzrlib.tests.test_filters',
+        'bzrlib.tests.test_ftp_transport',
+        'bzrlib.tests.test_foreign',
+        'bzrlib.tests.test_generate_docs',
+        'bzrlib.tests.test_generate_ids',
+        'bzrlib.tests.test_globbing',
+        'bzrlib.tests.test_gpg',
+        'bzrlib.tests.test_graph',
+        'bzrlib.tests.test_groupcompress',
+        'bzrlib.tests.test_hashcache',
+        'bzrlib.tests.test_help',
+        'bzrlib.tests.test_hooks',
+        'bzrlib.tests.test_http',
+        'bzrlib.tests.test_http_response',
+        'bzrlib.tests.test_https_ca_bundle',
+        'bzrlib.tests.test_identitymap',
+        'bzrlib.tests.test_ignores',
+        'bzrlib.tests.test_index',
+        'bzrlib.tests.test_info',
+        'bzrlib.tests.test_inv',
+        'bzrlib.tests.test_inventory_delta',
+        'bzrlib.tests.test_knit',
+        'bzrlib.tests.test_lazy_import',
+        'bzrlib.tests.test_lazy_regex',
+        'bzrlib.tests.test_lock',
+        'bzrlib.tests.test_lockable_files',
+        'bzrlib.tests.test_lockdir',
+        'bzrlib.tests.test_log',
+        'bzrlib.tests.test_lru_cache',
+        'bzrlib.tests.test_lsprof',
+        'bzrlib.tests.test_mail_client',
+        'bzrlib.tests.test_memorytree',
+        'bzrlib.tests.test_merge',
+        'bzrlib.tests.test_merge3',
+        'bzrlib.tests.test_merge_core',
+        'bzrlib.tests.test_merge_directive',
+        'bzrlib.tests.test_missing',
+        'bzrlib.tests.test_msgeditor',
+        'bzrlib.tests.test_multiparent',
+        'bzrlib.tests.test_mutabletree',
+        'bzrlib.tests.test_nonascii',
+        'bzrlib.tests.test_options',
+        'bzrlib.tests.test_osutils',
+        'bzrlib.tests.test_osutils_encodings',
+        'bzrlib.tests.test_pack',
+        'bzrlib.tests.test_patch',
+        'bzrlib.tests.test_patches',
+        'bzrlib.tests.test_permissions',
+        'bzrlib.tests.test_plugins',
+        'bzrlib.tests.test_progress',
+        'bzrlib.tests.test_read_bundle',
+        'bzrlib.tests.test_reconcile',
+        'bzrlib.tests.test_reconfigure',
+        'bzrlib.tests.test_registry',
+        'bzrlib.tests.test_remote',
+        'bzrlib.tests.test_rename_map',
+        'bzrlib.tests.test_repository',
+        'bzrlib.tests.test_revert',
+        'bzrlib.tests.test_revision',
+        'bzrlib.tests.test_revisionspec',
+        'bzrlib.tests.test_revisiontree',
+        'bzrlib.tests.test_rio',
+        'bzrlib.tests.test_rules',
+        'bzrlib.tests.test_sampler',
+        'bzrlib.tests.test_selftest',
+        'bzrlib.tests.test_serializer',
+        'bzrlib.tests.test_setup',
+        'bzrlib.tests.test_sftp_transport',
+        'bzrlib.tests.test_shelf',
+        'bzrlib.tests.test_shelf_ui',
+        'bzrlib.tests.test_smart',
+        'bzrlib.tests.test_smart_add',
+        'bzrlib.tests.test_smart_request',
+        'bzrlib.tests.test_smart_transport',
+        'bzrlib.tests.test_smtp_connection',
+        '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_switch',
+        'bzrlib.tests.test_symbol_versioning',
+        'bzrlib.tests.test_tag',
+        'bzrlib.tests.test_testament',
+        'bzrlib.tests.test_textfile',
+        'bzrlib.tests.test_textmerge',
+        'bzrlib.tests.test_timestamp',
+        'bzrlib.tests.test_trace',
+        'bzrlib.tests.test_transactions',
+        'bzrlib.tests.test_transform',
+        'bzrlib.tests.test_transport',
+        'bzrlib.tests.test_transport_log',
+        'bzrlib.tests.test_tree',
+        'bzrlib.tests.test_treebuilder',
+        'bzrlib.tests.test_tsort',
+        'bzrlib.tests.test_tuned_gzip',
+        'bzrlib.tests.test_ui',
+        'bzrlib.tests.test_uncommit',
+        'bzrlib.tests.test_upgrade',
+        'bzrlib.tests.test_upgrade_stacked',
+        'bzrlib.tests.test_urlutils',
+        'bzrlib.tests.test_version',
+        'bzrlib.tests.test_version_info',
+        'bzrlib.tests.test_weave',
+        'bzrlib.tests.test_whitebox',
+        'bzrlib.tests.test_win32utils',
+        'bzrlib.tests.test_workingtree',
+        'bzrlib.tests.test_workingtree_4',
+        'bzrlib.tests.test_wsgi',
+        'bzrlib.tests.test_xml',
+        ]
+
+
+def _test_suite_modules_to_doctest():
+    """Return the list of modules to doctest."""   
+    return [
+        'bzrlib',
+        'bzrlib.branchbuilder',
+        'bzrlib.export',
+        'bzrlib.inventory',
+        'bzrlib.iterablefile',
+        'bzrlib.lockdir',
+        'bzrlib.merge3',
+        'bzrlib.option',
+        'bzrlib.symbol_versioning',
+        'bzrlib.tests',
+        'bzrlib.timestamp',
+        'bzrlib.version_info_formats.format_custom',
+        ]
+
+
 def test_suite(keep_only=None, starting_with=None):
     """Build and return TestSuite for the whole of bzrlib.
 
@@ -3488,184 +3698,6 @@
     This function can be replaced if you need to change the default test
     suite on a global basis, but it is not encouraged.
     """
-    testmod_names = [
-                   'bzrlib.doc',
-                   'bzrlib.tests.blackbox',
-                   'bzrlib.tests.commands',
-                   'bzrlib.tests.per_branch',
-                   'bzrlib.tests.per_bzrdir',
-                   'bzrlib.tests.per_interrepository',
-                   'bzrlib.tests.per_intertree',
-                   'bzrlib.tests.per_inventory',
-                   'bzrlib.tests.per_interbranch',
-                   'bzrlib.tests.per_lock',
-                   'bzrlib.tests.per_transport',
-                   'bzrlib.tests.per_tree',
-                   'bzrlib.tests.per_pack_repository',
-                   'bzrlib.tests.per_repository',
-                   'bzrlib.tests.per_repository_chk',
-                   'bzrlib.tests.per_repository_reference',
-                   'bzrlib.tests.per_versionedfile',
-                   'bzrlib.tests.per_workingtree',
-                   'bzrlib.tests.test__annotator',
-                   'bzrlib.tests.test__chk_map',
-                   'bzrlib.tests.test__dirstate_helpers',
-                   'bzrlib.tests.test__groupcompress',
-                   'bzrlib.tests.test__known_graph',
-                   'bzrlib.tests.test__rio',
-                   'bzrlib.tests.test__walkdirs_win32',
-                   'bzrlib.tests.test_ancestry',
-                   'bzrlib.tests.test_annotate',
-                   'bzrlib.tests.test_api',
-                   'bzrlib.tests.test_atomicfile',
-                   'bzrlib.tests.test_bad_files',
-                   'bzrlib.tests.test_bencode',
-                   'bzrlib.tests.test_bisect_multi',
-                   'bzrlib.tests.test_branch',
-                   'bzrlib.tests.test_branchbuilder',
-                   'bzrlib.tests.test_btree_index',
-                   'bzrlib.tests.test_bugtracker',
-                   'bzrlib.tests.test_bundle',
-                   'bzrlib.tests.test_bzrdir',
-                   'bzrlib.tests.test__chunks_to_lines',
-                   'bzrlib.tests.test_cache_utf8',
-                   'bzrlib.tests.test_chk_map',
-                   'bzrlib.tests.test_chk_serializer',
-                   'bzrlib.tests.test_chunk_writer',
-                   'bzrlib.tests.test_clean_tree',
-                   'bzrlib.tests.test_commands',
-                   'bzrlib.tests.test_commit',
-                   'bzrlib.tests.test_commit_merge',
-                   'bzrlib.tests.test_config',
-                   'bzrlib.tests.test_conflicts',
-                   'bzrlib.tests.test_counted_lock',
-                   'bzrlib.tests.test_crash',
-                   'bzrlib.tests.test_decorators',
-                   'bzrlib.tests.test_delta',
-                   'bzrlib.tests.test_debug',
-                   'bzrlib.tests.test_deprecated_graph',
-                   'bzrlib.tests.test_diff',
-                   'bzrlib.tests.test_directory_service',
-                   'bzrlib.tests.test_dirstate',
-                   'bzrlib.tests.test_email_message',
-                   'bzrlib.tests.test_eol_filters',
-                   'bzrlib.tests.test_errors',
-                   'bzrlib.tests.test_export',
-                   'bzrlib.tests.test_extract',
-                   'bzrlib.tests.test_fetch',
-                   'bzrlib.tests.test_fifo_cache',
-                   'bzrlib.tests.test_filters',
-                   'bzrlib.tests.test_ftp_transport',
-                   'bzrlib.tests.test_foreign',
-                   'bzrlib.tests.test_generate_docs',
-                   'bzrlib.tests.test_generate_ids',
-                   'bzrlib.tests.test_globbing',
-                   'bzrlib.tests.test_gpg',
-                   'bzrlib.tests.test_graph',
-                   'bzrlib.tests.test_groupcompress',
-                   'bzrlib.tests.test_hashcache',
-                   'bzrlib.tests.test_help',
-                   'bzrlib.tests.test_hooks',
-                   'bzrlib.tests.test_http',
-                   'bzrlib.tests.test_http_response',
-                   'bzrlib.tests.test_https_ca_bundle',
-                   'bzrlib.tests.test_identitymap',
-                   'bzrlib.tests.test_ignores',
-                   'bzrlib.tests.test_index',
-                   'bzrlib.tests.test_info',
-                   'bzrlib.tests.test_inv',
-                   'bzrlib.tests.test_inventory_delta',
-                   'bzrlib.tests.test_knit',
-                   'bzrlib.tests.test_lazy_import',
-                   'bzrlib.tests.test_lazy_regex',
-                   'bzrlib.tests.test_lock',
-                   'bzrlib.tests.test_lockable_files',
-                   'bzrlib.tests.test_lockdir',
-                   'bzrlib.tests.test_log',
-                   'bzrlib.tests.test_lru_cache',
-                   'bzrlib.tests.test_lsprof',
-                   'bzrlib.tests.test_mail_client',
-                   'bzrlib.tests.test_memorytree',
-                   'bzrlib.tests.test_merge',
-                   'bzrlib.tests.test_merge3',
-                   'bzrlib.tests.test_merge_core',
-                   'bzrlib.tests.test_merge_directive',
-                   'bzrlib.tests.test_missing',
-                   'bzrlib.tests.test_msgeditor',
-                   'bzrlib.tests.test_multiparent',
-                   'bzrlib.tests.test_mutabletree',
-                   'bzrlib.tests.test_nonascii',
-                   'bzrlib.tests.test_options',
-                   'bzrlib.tests.test_osutils',
-                   'bzrlib.tests.test_osutils_encodings',
-                   'bzrlib.tests.test_pack',
-                   'bzrlib.tests.test_patch',
-                   'bzrlib.tests.test_patches',
-                   'bzrlib.tests.test_permissions',
-                   'bzrlib.tests.test_plugins',
-                   'bzrlib.tests.test_progress',
-                   'bzrlib.tests.test_read_bundle',
-                   'bzrlib.tests.test_reconcile',
-                   'bzrlib.tests.test_reconfigure',
-                   'bzrlib.tests.test_registry',
-                   'bzrlib.tests.test_remote',
-                   'bzrlib.tests.test_rename_map',
-                   'bzrlib.tests.test_repository',
-                   'bzrlib.tests.test_revert',
-                   'bzrlib.tests.test_revision',
-                   'bzrlib.tests.test_revisionspec',
-                   'bzrlib.tests.test_revisiontree',
-                   'bzrlib.tests.test_rio',
-                   'bzrlib.tests.test_rules',
-                   'bzrlib.tests.test_sampler',
-                   'bzrlib.tests.test_selftest',
-                   'bzrlib.tests.test_serializer',
-                   'bzrlib.tests.test_setup',
-                   'bzrlib.tests.test_sftp_transport',
-                   'bzrlib.tests.test_shelf',
-                   'bzrlib.tests.test_shelf_ui',
-                   'bzrlib.tests.test_smart',
-                   'bzrlib.tests.test_smart_add',
-                   'bzrlib.tests.test_smart_request',
-                   'bzrlib.tests.test_smart_transport',
-                   'bzrlib.tests.test_smtp_connection',
-                   '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_switch',
-                   'bzrlib.tests.test_symbol_versioning',
-                   'bzrlib.tests.test_tag',
-                   'bzrlib.tests.test_testament',
-                   'bzrlib.tests.test_textfile',
-                   'bzrlib.tests.test_textmerge',
-                   'bzrlib.tests.test_timestamp',
-                   'bzrlib.tests.test_trace',
-                   'bzrlib.tests.test_transactions',
-                   'bzrlib.tests.test_transform',
-                   'bzrlib.tests.test_transport',
-                   'bzrlib.tests.test_transport_log',
-                   'bzrlib.tests.test_tree',
-                   'bzrlib.tests.test_treebuilder',
-                   'bzrlib.tests.test_tsort',
-                   'bzrlib.tests.test_tuned_gzip',
-                   'bzrlib.tests.test_ui',
-                   'bzrlib.tests.test_uncommit',
-                   'bzrlib.tests.test_upgrade',
-                   'bzrlib.tests.test_upgrade_stacked',
-                   'bzrlib.tests.test_urlutils',
-                   'bzrlib.tests.test_version',
-                   'bzrlib.tests.test_version_info',
-                   'bzrlib.tests.test_weave',
-                   'bzrlib.tests.test_whitebox',
-                   'bzrlib.tests.test_win32utils',
-                   'bzrlib.tests.test_workingtree',
-                   'bzrlib.tests.test_workingtree_4',
-                   'bzrlib.tests.test_wsgi',
-                   'bzrlib.tests.test_xml',
-                   ]
 
     loader = TestUtil.TestLoader()
 
@@ -3700,24 +3732,9 @@
     suite = loader.suiteClass()
 
     # modules building their suite with loadTestsFromModuleNames
-    suite.addTest(loader.loadTestsFromModuleNames(testmod_names))
-
-    modules_to_doctest = [
-        'bzrlib',
-        'bzrlib.branchbuilder',
-        'bzrlib.export',
-        'bzrlib.inventory',
-        'bzrlib.iterablefile',
-        'bzrlib.lockdir',
-        'bzrlib.merge3',
-        'bzrlib.option',
-        'bzrlib.symbol_versioning',
-        'bzrlib.tests',
-        'bzrlib.timestamp',
-        'bzrlib.version_info_formats.format_custom',
-        ]
-
-    for mod in modules_to_doctest:
+    suite.addTest(loader.loadTestsFromModuleNames(_test_suite_testmod_names()))
+
+    for mod in _test_suite_modules_to_doctest():
         if not interesting_module(mod):
             # No tests to keep here, move along
             continue
@@ -3864,8 +3881,7 @@
     :param new_id: The id to assign to it.
     :return: The new test.
     """
-    from copy import deepcopy
-    new_test = deepcopy(test)
+    new_test = copy(test)
     new_test.id = lambda: new_id
     return new_test
 

=== modified file 'bzrlib/tests/test_selftest.py'
--- bzrlib/tests/test_selftest.py	2009-08-25 08:11:17 +0000
+++ bzrlib/tests/test_selftest.py	2009-08-26 19:35:13 +0000
@@ -1773,16 +1773,16 @@
         tree = self.make_branch_and_memory_tree('a')
         self.assertIsInstance(tree, bzrlib.memorytree.MemoryTree)
 
-
-class TestSFTPMakeBranchAndTree(test_sftp_transport.TestCaseWithSFTPServer):
-
-    def test_make_tree_for_sftp_branch(self):
-        """Transports backed by local directories create local trees."""
-        # NB: This is arguably a bug in the definition of make_branch_and_tree.
+    def test_make_tree_for_local_vfs_backed_transport(self):
+        # make_branch_and_tree has to use local branch and repositories
+        # when the vfs transport and local disk are colocated, even if
+        # a different transport is in use for url generation.
+        from bzrlib.transport.fakevfat import FakeVFATServer
+        self.transport_server = FakeVFATServer
+        self.assertFalse(self.get_url('t1').startswith('file://'))
         tree = self.make_branch_and_tree('t1')
         base = tree.bzrdir.root_transport.base
-        self.failIf(base.startswith('sftp'),
-                'base %r is on sftp but should be local' % base)
+        self.assertStartsWith(base, 'file://')
         self.assertEquals(tree.bzrdir.root_transport,
                 tree.branch.bzrdir.root_transport)
         self.assertEquals(tree.bzrdir.root_transport,
@@ -2117,7 +2117,8 @@
         return self.out, self.err
 
 
-class TestRunBzrSubprocess(tests.TestCaseWithTransport):
+class TestWithFakedStartBzrSubprocess(tests.TestCaseWithTransport):
+    """Base class for tests testing how we might run bzr."""
 
     def setUp(self):
         tests.TestCaseWithTransport.setUp(self)
@@ -2134,6 +2135,9 @@
             'working_dir':working_dir, 'allow_plugins':allow_plugins})
         return self.next_subprocess
 
+
+class TestRunBzrSubprocess(TestWithFakedStartBzrSubprocess):
+
     def assertRunBzrSubprocess(self, expected_args, process, *args, **kwargs):
         """Run run_bzr_subprocess with args and kwargs using a stubbed process.
 
@@ -2202,6 +2206,32 @@
             StubProcess(), '', allow_plugins=True)
 
 
+class TestFinishBzrSubprocess(TestWithFakedStartBzrSubprocess):
+
+    def test_finish_bzr_subprocess_with_error(self):
+        """finish_bzr_subprocess allows specification of the desired exit code.
+        """
+        process = StubProcess(err="unknown command", retcode=3)
+        result = self.finish_bzr_subprocess(process, retcode=3)
+        self.assertEqual('', result[0])
+        self.assertContainsRe(result[1], 'unknown command')
+
+    def test_finish_bzr_subprocess_ignoring_retcode(self):
+        """finish_bzr_subprocess allows the exit code to be ignored."""
+        process = StubProcess(err="unknown command", retcode=3)
+        result = self.finish_bzr_subprocess(process, retcode=None)
+        self.assertEqual('', result[0])
+        self.assertContainsRe(result[1], 'unknown command')
+
+    def test_finish_subprocess_with_unexpected_retcode(self):
+        """finish_bzr_subprocess raises self.failureException if the retcode is
+        not the expected one.
+        """
+        process = StubProcess(err="unknown command", retcode=3)
+        self.assertRaises(self.failureException, self.finish_bzr_subprocess,
+                          process)
+
+
 class _DontSpawnProcess(Exception):
     """A simple exception which just allows us to skip unnecessary steps"""
 
@@ -2285,39 +2315,8 @@
         self.assertEqual(['foo', 'current'], chdirs)
 
 
-class TestBzrSubprocess(tests.TestCaseWithTransport):
-
-    def test_start_and_stop_bzr_subprocess(self):
-        """We can start and perform other test actions while that process is
-        still alive.
-        """
-        process = self.start_bzr_subprocess(['--version'])
-        result = self.finish_bzr_subprocess(process)
-        self.assertContainsRe(result[0], 'is free software')
-        self.assertEqual('', result[1])
-
-    def test_start_and_stop_bzr_subprocess_with_error(self):
-        """finish_bzr_subprocess allows specification of the desired exit code.
-        """
-        process = self.start_bzr_subprocess(['--versionn'])
-        result = self.finish_bzr_subprocess(process, retcode=3)
-        self.assertEqual('', result[0])
-        self.assertContainsRe(result[1], 'unknown command')
-
-    def test_start_and_stop_bzr_subprocess_ignoring_retcode(self):
-        """finish_bzr_subprocess allows the exit code to be ignored."""
-        process = self.start_bzr_subprocess(['--versionn'])
-        result = self.finish_bzr_subprocess(process, retcode=None)
-        self.assertEqual('', result[0])
-        self.assertContainsRe(result[1], 'unknown command')
-
-    def test_start_and_stop_bzr_subprocess_with_unexpected_retcode(self):
-        """finish_bzr_subprocess raises self.failureException if the retcode is
-        not the expected one.
-        """
-        process = self.start_bzr_subprocess(['--versionn'])
-        self.assertRaises(self.failureException, self.finish_bzr_subprocess,
-                          process)
+class TestActuallyStartBzrSubprocess(tests.TestCaseWithTransport):
+    """Tests that really need to do things with an external bzr."""
 
     def test_start_and_stop_bzr_subprocess_send_signal(self):
         """finish_bzr_subprocess raises self.failureException if the retcode is
@@ -2331,14 +2330,6 @@
         self.assertEqual('', result[0])
         self.assertEqual('bzr: interrupted\n', result[1])
 
-    def test_start_and_stop_working_dir(self):
-        cwd = osutils.getcwd()
-        self.make_branch_and_tree('one')
-        process = self.start_bzr_subprocess(['root'], working_dir='one')
-        result = self.finish_bzr_subprocess(process, universal_newlines=True)
-        self.assertEndsWith(result[0], 'one\n')
-        self.assertEqual('', result[1])
-
 
 class TestKnownFailure(tests.TestCase):
 
@@ -2726,10 +2717,52 @@
 
 class TestTestSuite(tests.TestCase):
 
+    def test__test_suite_testmod_names(self):
+        # Test that a plausible list of test module names are returned
+        # by _test_suite_testmod_names.
+        test_list = tests._test_suite_testmod_names()
+        self.assertSubset([
+            'bzrlib.tests.blackbox',
+            'bzrlib.tests.per_transport',
+            'bzrlib.tests.test_selftest',
+            ],
+            test_list)
+
+    def test__test_suite_modules_to_doctest(self):
+        # Test that a plausible list of modules to doctest is returned
+        # by _test_suite_modules_to_doctest.
+        test_list = tests._test_suite_modules_to_doctest()
+        self.assertSubset([
+            'bzrlib.timestamp',
+            ],
+            test_list)
+
     def test_test_suite(self):
-        # This test is slow - it loads the entire test suite to operate, so we
-        # do a single test with one test in each category
-        test_list = [
+        # test_suite() loads the entire test suite to operate. To avoid this
+        # overhead, and yet still be confident that things are happening,
+        # we temporarily replace two functions used by test_suite with 
+        # test doubles that supply a few sample tests to load, and check they
+        # are loaded.
+        calls = []
+        def _test_suite_testmod_names():
+            calls.append("testmod_names")
+            return [
+                'bzrlib.tests.blackbox.test_branch',
+                'bzrlib.tests.per_transport',
+                'bzrlib.tests.test_selftest',
+                ]
+        original_testmod_names = tests._test_suite_testmod_names
+        def _test_suite_modules_to_doctest():
+            calls.append("modules_to_doctest")
+            return ['bzrlib.timestamp']
+        orig_modules_to_doctest = tests._test_suite_modules_to_doctest
+        def restore_names():
+            tests._test_suite_testmod_names = original_testmod_names
+            tests._test_suite_modules_to_doctest = orig_modules_to_doctest
+        self.addCleanup(restore_names)
+        tests._test_suite_testmod_names = _test_suite_testmod_names
+        tests._test_suite_modules_to_doctest = _test_suite_modules_to_doctest
+        expected_test_list = [
             # testmod_names
             'bzrlib.tests.blackbox.test_branch.TestBranch.test_branch',
             ('bzrlib.tests.per_transport.TransportTests'
@@ -2740,13 +2773,16 @@
             # plugins can't be tested that way since selftest may be run with
             # --no-plugins
             ]
-        suite = tests.test_suite(test_list)
-        self.assertEquals(test_list, _test_ids(suite))
+        suite = tests.test_suite()
+        self.assertEqual(set(["testmod_names", "modules_to_doctest"]),
+            set(calls))
+        self.assertSubset(expected_test_list, _test_ids(suite))
 
     def test_test_suite_list_and_start(self):
         # We cannot test this at the same time as the main load, because we want
-        # to know that starting_with == None works. So a second full load is
-        # incurred.
+        # to know that starting_with == None works. So a second load is
+        # incurred - note that the starting_with parameter causes a partial load
+        # rather than a full load so this test should be pretty quick.
         test_list = ['bzrlib.tests.test_selftest.TestTestSuite.test_test_suite']
         suite = tests.test_suite(test_list,
                                  ['bzrlib.tests.test_selftest.TestTestSuite'])