[MERGE] fix blackbox failures when run from windows executable

[Andrew Bennetts]

Mark Hammond wrote:
> I had a bit of a mini "win32 test sprint" while my girlfriend was safely
> sidetracked by the Olympics ;)  I've updated the patch with some more
> "simple" test fixes - but it does *not* include any of the os.listdir()
> fixes from the other couple of threads which still need to be tackled
> independently.

bb:tweak

I'm not sure that adding os.path.realpath calls to the test_bzrdir tests is the
right thing to do; see comments below.  I'm ok with trusting your judgement if
you disagree (I don't have a Win32 system to test on after all).

The other fixes look good (although that hasn't stopped me making comments...).

> === modified file 'bzrlib/tests/__init__.py'
> --- bzrlib/tests/__init__.py	2008-08-04 07:55:41 +0000
> +++ bzrlib/tests/__init__.py	2008-08-09 07:37:44 +0000
> @@ -1591,7 +1591,10 @@
>              # so we will avoid using it on all platforms, just to
>              # make sure the code path is used, and we don't break on win32
>              cleanup_environment()
> -            command = [sys.executable, bzr_path]
> +            command = [sys.executable]
> +            # frozen executables don't need the path to bzr
> +            if not hasattr(sys, "frozen"):
> +                command.append(bzr_path)

It doesn't matter very much, but I'd prefer 3-arg getattr here (“if getattr(sys,
"frozen", None) is None”) rather than hasattr, because getattr won't swallow
unrelated errors like KeyboardInterrupt.

> === modified file 'bzrlib/tests/interrepository_implementations/test_fetch.py'
> --- bzrlib/tests/interrepository_implementations/test_fetch.py	2008-06-11 04:20:16 +0000
> +++ bzrlib/tests/interrepository_implementations/test_fetch.py	2008-08-10 07:00:59 +0000
> @@ -36,6 +36,9 @@
>  from bzrlib.tests.interrepository_implementations import (
>      TestCaseWithInterRepository,
>      )
> +from bzrlib.tests.interrepository_implementations.test_interrepository import (
> +    check_repo_format_for_funky_id_on_win32
> +    )

Oops!  Good fix.

> === modified file 'bzrlib/tests/test_bzrdir.py'
> --- bzrlib/tests/test_bzrdir.py	2008-08-01 07:30:20 +0000
> +++ bzrlib/tests/test_bzrdir.py	2008-08-10 06:55:33 +0000
> @@ -533,7 +533,7 @@
>                           self.local_branch_path(branch))
>          self.assertEqual(
>              os.path.realpath(os.path.join('topdir', '.bzr', 'repository')),
> -            repo.bzrdir.transport.local_abspath('repository'))
> +            os.path.realpath(repo.bzrdir.transport.local_abspath('repository')))

Transport.local_abspath is supposed to be “the absolute path on the local
filesystem.”  I'm a little surprised that local_abspath is different to the
realpath.  Is it just a \ vs. / issue?

In fact, the docstring for Transport.local_abspath says:

        “This function is quite expensive: it calls realpath which resolves
        symlinks.”

So I think perhaps the right fix might be to fix Transport.local_abspath (i.e.
urlutils._win32_local_path_from_url) rather than these tests?  (And I wonder if
doing that would break or fix any other tests?)

If Transport.local_abspath isn't changed, then its docstring probably needs some
clarification.

> === modified file 'bzrlib/tests/test_plugins.py'
> --- bzrlib/tests/test_plugins.py	2008-05-08 04:21:01 +0000
> +++ bzrlib/tests/test_plugins.py	2008-08-10 08:01:19 +0000
> @@ -441,12 +441,18 @@
>          old_path = bzrlib.plugins.__path__
>          old_env = os.environ.get('BZR_PLUGIN_PATH')
>          try:
> +            # first determine the default builtin path
> +            bzrlib.plugins.__path__ = []
> +            if old_env is not None:
> +                del os.environ['BZR_PLUGIN_PATH']
> +            bzrlib.plugin.set_plugins_path()
> +            default = bzrlib.plugins.__path__[-1:]
> +            # now adjust env and new ones correctly added.
>              bzrlib.plugins.__path__ = []
>              os.environ['BZR_PLUGIN_PATH'] = "first\\//\\" + os.pathsep + \
>                  "second/\\/\\/"
>              bzrlib.plugin.set_plugins_path()
> -            expected_path = ['first', 'second',
> -                os.path.dirname(bzrlib.plugins.__file__)]
> +            expected_path = ['first', 'second'] + default
>              self.assertEqual(expected_path,
>                  bzrlib.plugins.__path__[:len(expected_path)])
>          finally:

This change looks a bit weird, particularly how you set bzrlib.plugins.__path__
to [] multiple times.  I think it's correct, but confusing.  I think if you
extract the “first determine the default builtin path” section into a helper
method it would be a little clearer.

> === modified file 'bzrlib/tests/test_source.py'
> --- bzrlib/tests/test_source.py	2008-04-24 07:38:09 +0000
> +++ bzrlib/tests/test_source.py	2008-08-10 10:46:37 +0000
> @@ -55,6 +55,8 @@
>  
>      def source_file_name(self, package):
>          """Return the path of the .py file for package."""
> +        if hasattr(sys, "frozen"):
> +            raise TestSkipped("can't test sources in frozen distributions.")

Again, 3-arg getattr would be good.

-Andrew.