[PATCH] [0.13] Bug #74759

[John A Meinel]

John Arbash Meinel has voted +1 (conditional).
Status is now: Semi-approved
Comment:
Looking pretty good.

First, I try to avoid double underscore private members like this.
+    def setUp(self):
+        super(TestCaseWithTwoWebservers, self).setUp()
+        self.transport_secondary_server = HttpServer
+        self.__secondary_server = None

If you have a reason to make it __secondary_server, you can leave it, but I generally prefer _secondary_server.


For these:
 self._http_proxy = os.environ.get("http_proxy") 
 if self._http_proxy is not None:
   del os.environ["http_proxy"]
+ self._no_proxy = os.environ.get("no_proxy")
+ if self._no_proxy is not None:
+   del os.environ["no_proxy"]

You can actually use osutils.set_or_unset_env('no_proxy', None)

It might be better to restore them if they exist, in which case you should do it in bzrlib/tests/__init__.py where we have:

def _cleanEnvironment(self):
  new_env = {...
     'http_proxy':None,
     'no_proxy':None,
     'HTTP_PROXY':None. #Just in case?
  }

That way they get clear when all tests initialize, and get restored when the test is done.

+        # Oh my ! pycurl do not check the port part :-( So we
+        # just test the host part

^- # Oh my! pycurl does not check for the port as part of no_proxy. :-(
   # So we only test the host portion.

If you have tested this, and made sure it works correctly with both http_proxy and no_proxy, I think it would be good to merge.

For details, see: http://bundlebuggy.aaronbentley.com/request/%3C87vekiwob2.fsf%40alplog.fr%3E