Rev 5027: (mbp) add import-tariff tests in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Thu Feb 11 04:02:47 GMT 2010
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 5027 [merge]
revision-id: pqm at pqm.ubuntu.com-20100211040241-w6n021dz0uus341n
parent: pqm at pqm.ubuntu.com-20100211031913-hu2gowhvmu10djr8
parent: mbp at canonical.com-20100210020320-ebyrm44ig7maa4f5
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2010-02-11 04:02:41 +0000
message:
(mbp) add import-tariff tests
added:
bzrlib/tests/test_import_tariff.py test_import_tariff.p-20100207155145-ff9infp7goncs7zh-1
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/plugin.py plugin.py-20050622060424-829b654519533d69
bzrlib/tests/__init__.py selftest.py-20050531073622-8d0e3c8845c97a64
doc/developers/testing.txt testing.txt-20080812140359-i70zzh6v2z7grqex-1
profile_imports.py profile_imports.py-20060618020306-k5uw80achysrokj9-1
=== modified file 'NEWS'
--- a/NEWS 2010-02-11 01:13:46 +0000
+++ b/NEWS 2010-02-11 04:02:41 +0000
@@ -60,6 +60,11 @@
Testing
*******
+* New `bzrlib.tests.test_import_tariff` can make assertions about what
+ Python modules are loaded, to guard against startup time or library
+ dependency regressions.
+ (Martin Pool)
+
* Stop sending apport crash files to ``.cache`` in the directory from
which ``bzr selftest`` was run. (Martin Pool, #422350)
=== modified file 'bzrlib/plugin.py'
--- a/bzrlib/plugin.py 2009-09-04 15:36:48 +0000
+++ b/bzrlib/plugin.py 2010-02-10 02:03:20 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2004, 2005, 2007, 2008 Canonical Ltd
+# Copyright (C) 2004, 2005, 2007, 2008, 2010 Canonical Ltd
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -60,6 +60,12 @@
DEFAULT_PLUGIN_PATH = None
_loaded = False
+_plugins_disabled = False
+
+
+def are_plugins_disabled():
+ return _plugins_disabled
+
@deprecated_function(deprecated_in((2, 0, 0)))
def get_default_plugin_path():
@@ -75,6 +81,8 @@
Future calls to load_plugins() will be ignored.
"""
+ global _plugins_disabled
+ _plugins_disabled = True
load_plugins([])
=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py 2010-02-10 17:52:08 +0000
+++ b/bzrlib/tests/__init__.py 2010-02-11 04:02:41 +0000
@@ -1201,6 +1201,10 @@
raise AssertionError('pattern "%s" found in "%s"'
% (needle_re, haystack))
+ def assertContainsString(self, haystack, needle):
+ if haystack.find(needle) == -1:
+ self.fail("string %r not found in '''%s'''" % (needle, haystack))
+
def assertSubset(self, sublist, superlist):
"""Assert that every entry in sublist is present in superlist."""
missing = set(sublist) - set(superlist)
@@ -1542,17 +1546,17 @@
# use an env var so it propagates to subprocesses.
'APPORT_DISABLE': '1',
}
- self.__old_env = {}
+ self._old_env = {}
self.addCleanup(self._restoreEnvironment)
for name, value in new_env.iteritems():
self._captureVar(name, value)
def _captureVar(self, name, newvalue):
"""Set an environment variable, and reset it when finished."""
- self.__old_env[name] = osutils.set_or_unset_env(name, newvalue)
+ self._old_env[name] = osutils.set_or_unset_env(name, newvalue)
def _restoreEnvironment(self):
- for name, value in self.__old_env.iteritems():
+ for name, value in self._old_env.iteritems():
osutils.set_or_unset_env(name, value)
def _restoreHooks(self):
@@ -3658,6 +3662,7 @@
'bzrlib.tests.test_identitymap',
'bzrlib.tests.test_ignores',
'bzrlib.tests.test_index',
+ 'bzrlib.tests.test_import_tariff',
'bzrlib.tests.test_info',
'bzrlib.tests.test_inv',
'bzrlib.tests.test_inventory_delta',
=== added file 'bzrlib/tests/test_import_tariff.py'
--- a/bzrlib/tests/test_import_tariff.py 1970-01-01 00:00:00 +0000
+++ b/bzrlib/tests/test_import_tariff.py 2010-02-10 02:03:20 +0000
@@ -0,0 +1,85 @@
+# Copyright (C) 2010 Canonical Ltd
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+
+"""Tests for how many modules are loaded in executing various commands."""
+
+from testtools import content
+
+from bzrlib.plugin import (
+ are_plugins_disabled,
+ )
+
+from bzrlib.tests import (
+ TestCaseWithTransport,
+ )
+
+
+class TestImportTariffs(TestCaseWithTransport):
+
+ """Check how many modules are loaded for some representative scenarios.
+
+ See the Testing Guide in the developer documentation for more explanation.
+ """
+
+ def run_command_check_imports(self, args, forbidden_imports):
+ # We use PYTHON_VERBOSE rather than --profile-importts because in
+ # experimentation the profile-imports output seems to not always show
+ # the modules you'd expect; this can be debugged but python -v seems
+ # more likely to always show everything. And we use the environment
+ # variable rather than 'python -v' in the hope it will work even if
+ # bzr is frozen and python is not explicitly specified. -- mbp 20100208
+ #
+ # Normally we want test isolation from the real $HOME but here we
+ # explicitly do want to test against things installed there, therefore
+ # we pass it through.
+ env_changes = dict(PYTHONVERBOSE='1')
+ for name in ['BZR_HOME', 'BZR_PLUGIN_PATH', 'HOME',]:
+ env_changes[name] = self._old_env.get(name)
+ out, err = self.run_bzr_subprocess(args,
+ allow_plugins=(not are_plugins_disabled()),
+ env_changes=env_changes)
+
+ self.addDetail('subprocess_stderr',
+ content.Content(content.ContentType("text", "plain"),
+ lambda:[err]))
+
+ bad_modules = []
+ for module_name in forbidden_imports:
+ if err.find("\nimport %s " % module_name) != -1:
+ bad_modules.append(module_name)
+
+ if bad_modules:
+ self.fail("command %r loaded forbidden modules %r"
+ % (args, bad_modules))
+ return out, err
+
+ def test_import_tariffs_working(self):
+ # check some guaranteed-true and false imports to be sure we're
+ # measuring correctly
+ self.make_branch_and_tree('.')
+ self.run_command_check_imports(['st'],
+ ['nonexistentmodulename', 'anothernonexistentmodule'])
+ self.assertRaises(AssertionError,
+ self.run_command_check_imports,
+ ['st'],
+ ['bzrlib.tree'])
+
+ def test_simple_local(self):
+ # 'st' in a working tree shouldn't need many modules
+ self.make_branch_and_tree('.')
+ self.run_command_check_imports(['st'],
+ ['smtplib'])
=== modified file 'doc/developers/testing.txt'
--- a/doc/developers/testing.txt 2010-02-05 10:57:26 +0000
+++ b/doc/developers/testing.txt 2010-02-10 02:03:20 +0000
@@ -368,10 +368,58 @@
''')
-
-.. Effort tests
-.. ~~~~~~~~~~~~
-
+Import tariff tests
+-------------------
+
+`bzrlib.tests.test_import_tariff` has some tests that measure how many
+Python modules are loaded to run some representative commands.
+
+We want to avoid loading code unnecessarily, for reasons including:
+
+* Python modules are interpreted when they're loaded, either to define
+ classes or modules or perhaps to initialize some structures.
+
+* With a cold cache we may incur blocking real disk IO for each module.
+
+* Some modules depend on many others.
+
+* Some optional modules such as `testtools` are meant to be soft
+ dependencies and only needed for particular cases. If they're loaded in
+ other cases then bzr may break for people who don't have those modules.
+
+`test_import_tarrif` allows us to check that removal of imports doesn't
+regress.
+
+This is done by running the command in a subprocess with
+``--profile-imports``. Starting a whole Python interpreter is pretty
+slow, so we don't want exhaustive testing here, but just enough to guard
+against distinct fixed problems.
+
+Assertions about precisely what is loaded tend to be brittle so we instead
+make assertions that particular things aren't loaded.
+
+Unless selftest is run with ``--no-plugins``, modules will be loaded in
+the usual way and checks made on what they cause to be loaded. This is
+probably worth checking into, because many bzr users have at least some
+plugins installed (and they're included in binary installers).
+
+In theory, plugins might have a good reason to load almost anything:
+someone might write a plugin that opens a network connection or pops up a
+gui window every time you run 'bzr status'. However, it's more likely
+that the code to do these things is just being loaded accidentally. We
+might eventually need to have a way to make exceptions for particular
+plugins.
+
+Some things to check:
+
+* non-GUI commands shouldn't load GUI libraries
+
+* operations on bzr native formats sholudn't load foreign branch libraries
+
+* network code shouldn't be loaded for purely local operations
+
+* particularly expensive Python built-in modules shouldn't be loaded
+ unless there is a good reason
Skipping tests
=== modified file 'profile_imports.py'
--- a/profile_imports.py 2010-02-11 03:19:13 +0000
+++ b/profile_imports.py 2010-02-11 04:02:41 +0000
@@ -64,7 +64,8 @@
def log_stack_info(out_file, sorted=True, hide_fast=True):
# Find all of the roots with import = 0
- out_file.write(' cum inline name\t\t\t\t\t\tframe\n')
+ out_file.write('%5s %5s %-40s @ %s:%s\n'
+ % ('cum', 'inline', 'name', 'file', 'line'))
todo = [(value[-1], key) for key,value in _info.iteritems() if value[0] == 0]
if sorted:
@@ -89,9 +90,10 @@
# indent, cum_time, mod_time, name,
# scope_name, frame_name, frame_lineno
- out_file.write('%5.1f %5.1f %s %-35s\t@ %s:%d\n'
- % (info[-1]*1000., mod_time*1000., '+'*info[0],
- cur[1][:35], info[1], info[2]))
+ out_file.write('%5.1f %5.1f %-40s @ %s:%d\n'
+ % (info[-1]*1000., mod_time*1000.,
+ ('+'*info[0] + cur[1]),
+ info[1], info[2]))
if sorted:
c_times.sort()
More information about the bazaar-commits
mailing list