Merge lp:~alecu/ubuntuone-client/proxy-tunnel-client into lp:ubuntuone-client
Proposed by
Alejandro J. Cura
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Alejandro J. Cura | ||||
Approved revision: | 1212 | ||||
Merged at revision: | 1203 | ||||
Proposed branch: | lp:~alecu/ubuntuone-client/proxy-tunnel-client | ||||
Merge into: | lp:ubuntuone-client | ||||
Prerequisite: | lp:~alecu/ubuntuone-client/proxy-tunnel-server | ||||
Diff against target: |
264 lines (+247/-0) 3 files modified
tests/proxy/test_tunnel_client.py (+144/-0) tests/proxy/test_tunnel_server.py (+2/-0) ubuntuone/proxy/tunnel_client.py (+101/-0) |
||||
To merge this branch: | bzr merge lp:~alecu/ubuntuone-client/proxy-tunnel-client | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Natalia Bidart (community) | Approve | ||
Manuel de la Peña (community) | Approve | ||
Review via email: mp+95077@code.launchpad.net |
Commit message
- A client for the proxy tunnel, to be used by syncdaemon (LP: #929207).
Description of the change
- A client for the proxy tunnel, to be used by syncdaemon (LP: #929207)
To post a comment you must log in.
In TunnelClientPro tocolTestCase I see that we do a number of times the following:
75 + other_proto = protocol.Protocol() ClientFactory( ) buildProtocol = lambda _addr: other_proto client_ factory = tunnel_ client. TunnelClientFac tory(host, port, client_ factory. buildProtocol( fake_addr) client_ proto.transport = FakeTransport() client_ proto.connectio nMade()
76 + other_factory = protocol.
77 + other_factory.
78 + tunnel_
79 + other_factory)
80 + tunnel_client_proto = tunnel_
81 + tunnel_
82 + tunnel_
Would't it be a good idea to move that to the setUp?
In TunnelClientTes tCase you refactor the tests as follows:
def _assert_
if is_ssl:
def test_connects_ right(self) :
"""Uses the CONNECT method on the tunnel."""
self. _assert_ connection( )
def test_starts_ tls_connection( self):
"""TLS is started after connecting; control passed to the client."""
self. _assert_ connection( is_ssl= True)
Although if you think it does not bring anything to the code you can leave the tests as they are.
Since all the above comments are just about tests and writing less code I'll approve and will trust you to apply or not the comments if you feel they are needed :)