[MERGE] Add some unicode-related tests from the hpss branch, and a few other nits (also from the hpss branch).

[Andrew Bennetts]

John Arbash Meinel wrote:
[...]
> >> will fail for anything but args == tuple.
> > 
> > I think this is a YAGNI.  It's easy to make more flexible later if we decide
> > it's worth it, but so far the hpss branch hasn't had trouble with using tuples.
> > Perhaps because the current protocol is largely about encoding and decoding
> > tuples?  Anyway, I'm not worried about this.
> > 
> 
> ^- I think it is a YAGNI, but I also think it is something that non-uber
> hackers could stumble on.... Would they stumble very hard?
> 
> >>> x = (1,2,3)
> >>> x + [4, 5]
> Traceback (most recent call last):
>   File "<stdin>", line 1, in <module>
> TypeError: can only concatenate tuple (not "list") to tuple
> 
> Is probably sufficient for them to know what to do.

Right.

[...]
> > Although it seems almost too obvious to be worth stating, I've expanded the
> > docstring like so:
> > 
> >     def remote_path_from_transport(self, transport):
> >         """Convert transport into a path suitable for using in a request.
> >         
> >         Note that the resulting remote path doesn't encode the host name or
> >         anything but path, so it is only safe to use it in requests sent over
> >         the medium from the matching transport.
> >         """
> 
> ^- Well, too obvious to you, and understandable to me :) This is a
> really nice docstring. Thank you. I honestly didn't correctly understand
> the function before this.

I'm glad it helped :)

It's tricky writing a clear docstring for something that seems obvious to the
person writing it!

[...]
> > I'd like there to be direct tests, but at this point it's not a big concern for
> > me; I'm confident in this code anyway, and SmartClient doesn't change very much
> > so the lack of more focussed tests isn't much of a burden to developing it.  If
> > SmartClient evolves more later, that would be a good time to define tests for it
> > I think.
> 
> ^- I really don't like "public" code that isn't tested. Once we have a
> release, we have a backwards compatibility statement. (Which we
> certainly seem to break regularly, but we try not to). Without failing
> tests, you don't even know if you've broken compatibility.
> 
> If SmartClient is meant as an implementation detail, then it should be
> marked private IMO.
> 
> You seem to be writing tests to make sure the code you've written is
> correct (which is valid). I also write them to make sure other people
> updating the code don't mess it up.

That's a fair point.  I think the pragmatic thing to do for now is to rename
SmartClient to _SmartClient for 0.16, which clearly makes it an implementation
detail and not a public feature.

[...]
> > Yep, it passes.  Worth checking though! :)

(And it turns out it failed on 2.4 but not 2.5, now fixed).

> > Any chance of an upgrade to +1?
> > 
> > -Andrew.
> 
> I don't want to get bogged down without getting the real improvements
> merged. I think there are some testing philosophy discussions to be had,
> but the changes are fine. (+1).

Thanks!

-Andrew.