Code review comment for lp:~alecu/ubuntuone-client/proxy-tunnel-process

Revision history for this message
Manuel de la Peña (mandel) wrote :

A very stupid comments but why doing:

if len(proxies) and proxies[0].type() != QNetworkProxy.NoProxy:
    return True

When you an do:

return len(proxies) and proxies[0].type() != QNetworkProxy.NoProxy

same for:

if settings:
    return True

In IsProxyEnabledTestCase I think you can merge the tests cases, for example:

             def _assert_enabled(self, platform):
                 """Assert that the proxy is enabled."""
                 self.patch(tunnel_server.sys, "platform", platform)
                 ret = tunnel_server.is_proxy_enabled(SAMPLE_HOST, str(SAMPLE_PORT))
                 self.assertTrue(ret, "Proxy is enabled.")

             def test_platform_linux_enabled(self):
                 """Tests for the linux platform with proxies enabled."""
                 self.patch(tunnel_server.gsettings, "get_proxy_settings",
                            lambda: FAKE_SETTINGS)
                 self._assert_enabled('linux3')

             def test_platform_other_enabled(self):
                 """Tests for any other platform with proxies enabled."""
                 fake_netproxfact = FakeNetworkProxyFactoryClass(True)
                 self.patch(tunnel_server, "QNetworkProxyFactory", fake_netproxfact)
                 self._assert_enabled('windows 1.0')

Same for the disabled one.

I think it would be a good idea to use sys.exit in main specially in the case were there is no proxy.

review: Needs Fixing

« Back to merge proposal