[patch] fixes for test case get_url, make_branch, etc

[John Arbash Meinel]

Martin Pool wrote:
> Thanks to Robert and Andrew for pairing on this.
> 
> This fixes a few url/path inconsistencies in the test case get_url(),
> make_branch(), etc.  There is still (as the comments say) an outstanding
> bug that get_url doesn't really return a URL, but this fixes some things
> that made it hard to test the smart server.
> 
> It's not crucial for 0.9 but I think it'd be good, so seeking +1s.
> 
> 
> 
> 
> ------------------------------------------------------------------------
> 
> === modified file 'bzrlib/tests/__init__.py'
> --- bzrlib/tests/__init__.py	2006-07-25 00:58:17 +0000
> +++ bzrlib/tests/__init__.py	2006-07-27 07:38:38 +0000
> @@ -1072,7 +1072,7 @@
>          return self.__server
>  
>      def get_url(self, relpath=None):
> -        """Get a URL for the readwrite transport.
> +        """Get a URL (or maybe a path) for the readwrite transport.
>  
>          This will either be backed by '.' or to an equivalent non-file based
>          facility.
> @@ -1083,7 +1083,14 @@
>          if relpath is not None and relpath != '.':
>              if not base.endswith('/'):
>                  base = base + '/'
> -            base = base + urlutils.escape(relpath)
> +            # XXX: Really base should be a url; we did after all call
> +            # get_url()!  But sometimes it's just a path (from
> +            # LocalAbspathServer), and it'd be wrong to append urlescaped data
> +            # to a non-escaped local path.
> +            if base.startswith('./') or base.startswith('/'):
> +                base += relpath
> +            else:
> +                base += urlutils.escape(relpath)
>          return base

Why not fix LocalAbspathServer to return a real URL?
Or you can do:

if base.startswith('./') or base.startswith('/'):
  base = urlutils.local_path_to_url(base)
base += urlutils.escape(relpath)

*However* you will still run into the problem that this borks on win32.

Because local paths don't start with '/' there.
I think the best fix is to make get_url() always return a URL.


>  
>      def get_transport(self):
> @@ -1109,20 +1116,18 @@
>  
>      def make_bzrdir(self, relpath, format=None):
>          try:
> -            url = self.get_url(relpath)
> -            mutter('relpath %r => url %r', relpath, url)
> -            segments = url.split('/')
> +            # might be a relative or absolute path
> +            maybe_a_url = self.get_url(relpath)
> +            segments = maybe_a_url.split('/')
> +            t = get_transport(maybe_a_url)
>              if segments and segments[-1] not in ('', '.'):
> -                parent = '/'.join(segments[:-1])
> -                t = get_transport(parent)
>                  try:
> -                    t.mkdir(segments[-1])
> +                    t.mkdir('.')
>                  except errors.FileExists:
>                      pass
>              if format is None:
> -                format=bzrlib.bzrdir.BzrDirFormat.get_default_format()
> -            # FIXME: make this use a single transport someday. RBC 20060418
> -            return format.initialize_on_transport(get_transport(relpath))
> +                format = bzrlib.bzrdir.BzrDirFormat.get_default_format()
> +            return format.initialize_on_transport(t)
>          except errors.UninitializableFormat:
>              raise TestSkipped("Format %s is not initializable." % format)

I think you should get rid of 'segments' because for something like:
file:/// it does the wrong thing. And for 'http://localhost/' it is also
broken (it will try to cd into http://, etc)

Just use "t = t.clone('..')"

Or if you just want to create a single directory, you can use:
base, tip = urlutils.split(url)

So, I don't really like the hacks you put in to handle local paths
versus URLs, since they will fail on other platforms. (Now, it will only
fail if 'relpath' has non-standard characters, but still....)

Anyway +1 on some of your tests, etc, but -0 on the 'maybe_a_url' stuff.

John
=:->

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 254 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060727/01d513b5/attachment.pgp