[MERGE] Try harder to avoid loading plugins during the test suite.

Thu Jan 8 05:55:47 GMT 2009

Vincent Ladeuil wrote:
> I'm soo totally against even the spirit of this patch ! :-)

:)

[...]
>     Andrew> === modified file 'bzrlib/tests/__init__.py'
>     Andrew> --- bzrlib/tests/__init__.py	2008-12-11 03:08:03 +0000
>     Andrew> +++ bzrlib/tests/__init__.py	2008-12-16 07:14:05 +0000
>     Andrew> @@ -1409,6 +1409,7 @@
>     Andrew>          stdout.encoding = encoding
>     Andrew>          stderr.encoding = encoding
>  
>     Andrew> +        args = ['--no-plugins'] + args
> 
> Wow, when I saw that I thought: "That just makes it impossible to
> use run_bzr to blackbox test plugins !"...
> 
> And happily went to test that hypothesis... and failed...
> 
> AFAIUI, --no-plugins is handled once for a given process by
> calling bzrlib.plugin.disable_plugins() which calls
> bzrlib.plugin.load_plugins([]) which in turn set a plugin._loaded
> to True which ensures that we will never load plugins in that
> process.

Well, I think my reasoning was that we shouldn't be accidentally loading
plugins at this point, and calling disable_plugins() after plugins have
been loaded is harmless.

That said, I don't think this is necessary now; see my comments below.

> 
> That's enough to explain why the above line has no effect for my
> test... but since you added it I imagine it has an effect for
> you...
> 
> Can you tell us *how* you produce tracebacks ?

I run the full test suite, with “bzr --no-plugins selftest”, and get an
error like this:

======================================================================
ERROR: test_branch_to_lightweight_checkout (bzrlib.tests.test_reconfigure.TestReconfigure)

vvvv[log from bzrlib.tests.test_reconfigure.TestReconfigure.test_branch_to_lightweight_checkout]
1596.077  creating repository in file:///tmp/testbzr-Jvf_zD.tmp/bzrlib.tests.test_reconfigure.TestReconfigure.test_branch_to_lightweight_checkout/work/parent/.bzr/.
1596.085  creating branch <bzrlib.branch.BzrBranchFormat6 object at 0xd3be24c> in file:///tmp/testbzr-Jvf_zD.tmp/bzrlib.tests.test_reconfigure.TestReconfigure.test_branch_to_lightweight_checkout/work/parent/.bzr/
1596.216  opening working tree '/tmp/testbzr-Jvf_zD.tmp'

^^^^[log from bzrlib.tests.test_reconfigure.TestReconfigure.test_branch_to_lightweight_checkout]
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/andrew/warthogs/bzr/bencode-tweak/bzrlib/tests/test_reconfigure.py", line 235, in test_branch_to_lightweight_checkout
    self.prepare_branch_to_lightweight_checkout()
  File "/home/andrew/warthogs/bzr/bencode-tweak/bzrlib/tests/test_reconfigure.py", line 225, in prepare_branch_to_lightweight_checkout
    child = parent.bzrdir.sprout('child').open_workingtree()
  File "/home/andrew/warthogs/bzr/bencode-tweak/bzrlib/bzrdir.py", line 1098, in sprout
    force_new_repo, stacked_branch_url, require_stacking=stacked)
  File "/home/andrew/warthogs/bzr/bencode-tweak/bzrlib/bzrdir.py", line 424, in determine_repository_policy
    policy = self._find_containing(repository_policy)
  File "/home/andrew/warthogs/bzr/bencode-tweak/bzrlib/bzrdir.py", line 596, in _find_containing
    next_transport)[0]
  File "/home/andrew/warthogs/bzr/bencode-tweak/bzrlib/bzrdir.py", line 852, in open_containing_from_transport
    result = BzrDir.open_from_transport(a_transport)
  File "/home/andrew/warthogs/bzr/bencode-tweak/bzrlib/bzrdir.py", line 806, in open_from_transport
    redirected)
  File "/home/andrew/warthogs/bzr/bencode-tweak/bzrlib/transport/__init__.py", line 1663, in do_catching_redirections
    return action(transport)
  File "/home/andrew/warthogs/bzr/bencode-tweak/bzrlib/bzrdir.py", line 793, in find_format
    transport, _server_formats=_server_formats)
  File "/home/andrew/warthogs/bzr/bencode-tweak/bzrlib/bzrdir.py", line 1624, in find_format
    return format.probe_transport(transport)
  File "/usr/lib/python2.5/site-packages/bzrlib/plugins/svn/format.py", line 71, in probe_transport
    format = klass()
  File "/usr/lib/python2.5/site-packages/bzrlib/plugins/svn/format.py", line 63, in __init__
    lazy_check_versions()
  File "/usr/lib/python2.5/site-packages/bzrlib/plugins/svn/__init__.py", line 158, in lazy_check_versions
    bzrlib.api.require_any_api(bzrlib, COMPATIBLE_BZR_VERSIONS)
  File "/home/andrew/warthogs/bzr/bencode-tweak/bzrlib/api.py", line 104, in require_any_api
    require_api(object_with_api, wanted_api_list[-1])
  File "/home/andrew/warthogs/bzr/bencode-tweak/bzrlib/api.py", line 82, in require_api
    raise IncompatibleAPI(object_with_api, wanted_api, minimum, current)
IncompatibleAPI: The API for "<module 'bzrlib' from '/home/andrew/warthogs/bzr/bencode-tweak/bzrlib/__init__.pyc'>" is not compatible with "(1, 10, 0)". It supports versions "(1, 11, 0)" to "(1, 11, 0)".

----------------------------------------------------------------------

It's safe to say I think that this is a bogus failure :)

>     Andrew> +        print err
> 
> Left out after debug ?

Oops, yes.  Thanks.

>     Andrew> +        for line in out.splitlines():
>     Andrew>              if not line.startswith(' '):
>     Andrew>                  current = line.split()[0]
>     Andrew>              help[current] = help.get(current, '') + line
>     Andrew> @@ -635,9 +637,14 @@
>     Andrew>      old_plugins_path = bzrlib.plugins.__path__
>     Andrew>      bzrlib.plugins.__path__ = []
>     Andrew>      plugin._loaded = False
> 
> Ha ! The culprit maybe ?

Ah, yes.

> 
>     Andrew> +    def load_from_path(dirs):
>     Andrew> +        pass
>     Andrew> +    old_load_from_path = plugin.load_from_path
>     Andrew> +    plugin.load_from_path = load_from_path
>     Andrew>      def restore_plugins():
>     Andrew>          bzrlib.plugins.__path__ = old_plugins_path
>     Andrew>          plugin._loaded = False
> 
> And again ! Argh.
> 
>     Andrew> +        plugin.load_from_path = old_load_from_path
>     Andrew>      test_case.addCleanup(restore_plugins)
>  
> I can't go further without a recipe to reproduce the problem, but
> I think a cleaner approach would be to not *load* test_plugins if
> --no-plugins is passed on the command-line...

No, I disagree.  test_plugins shouldn't need to change the global state (or be
in any way dependent on it) to do its tests.  If it's too hard to do correctly,
it should run the tests in a subprocess to isolate them, but I don't think it's
too hard to do correctly.

It already takes adequate steps to make sure it works regardless of the state
bzrlib.plugins is in, it just needs to be fixed to put that state *back*
correctly.

I've attached a patch that does so.

> BB:comment
> 
>  Vincent
> 
> P.S.: Jokes aside, I'm not sure people realize that running
> selftest --no-plugins exclude even bzrlib.plugins.... which means
> that bzr itself is not fully tested in that case (yeah, I realize
> that's not the case on pqm, but yet...).

Maybe we need --no-non-core-plugins ;)

I'd settle for --no-plugins-with-slow-or-buggy-test-suites.

-Andrew.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: no-plugins-testsuite-take-2.diff
Type: text/x-diff
Size: 6576 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20090108/1b8046d8/attachment-0001.bin 