Add tests for missing core modules (#674)

* Rename is_py3 to is_py2 for more logical guard

* Add stubs for missing tests, add few more tests for core modules

* Lint fixes

* Line too long fix

* Remove unnecessary KeyboardInterrupt

* Consistent workflow names

* Update homebrew formulae.  Doesnt seems to work now

* test_enable_dashboard and test_enable_events

* test_enable_dashboard and test_enable_events

* Fix problem where empty plugin string was passed as plugin module

* test_enable_devtools and remove redundant guards for None and "" which was there due to a bug

.github/workflows/test-brew.yml

@@ -1,12 +1,12 @@
 ---
-name: Proxy.py Brew
+name: brew
 
 on: [push, pull_request]  # yamllint disable-line rule:truthy
 
 jobs:
   build:
     runs-on: ${{ matrix.os }}-latest
-    name: Brew - Python ${{ matrix.python }} on ${{ matrix.os }}
+    name: 🐍${{ matrix.python }} @ ${{ matrix.os }}
     strategy:
       matrix:
         os: [macOS]

.github/workflows/test-dashboard.yml

@@ -1,12 +1,12 @@
 ---
-name: Proxy.py Dashboard
+name: dashboard
 
 on: [push, pull_request]  # yamllint disable-line rule:truthy
 
 jobs:
   build:
     runs-on: ${{ matrix.os }}-latest
-    name: Dashboard - Node ${{ matrix.node }} on ${{ matrix.os }}
+    name: Node ${{ matrix.node }} @ ${{ matrix.os }}
     strategy:
       matrix:
         os: [macOS, ubuntu, windows]

.github/workflows/test-docker.yml

@@ -1,12 +1,12 @@
 ---
-name: Proxy.py Docker
+name: docker
 
 on: [push, pull_request]  # yamllint disable-line rule:truthy
 
 jobs:
   build:
     runs-on: ${{ matrix.os }}-latest
-    name: Docker - Python ${{ matrix.python }} on ${{ matrix.os }}
+    name: 🐍${{ matrix.python }} @ ${{ matrix.os }}
     strategy:
       matrix:
         os: [ubuntu]

helper/homebrew/develop/proxy.rb

@@ -9,11 +9,6 @@ class Proxy < Formula
 
   depends_on "python"
 
-  resource "typing-extensions" do
-    url "https://files.pythonhosted.org/packages/6a/28/d32852f2af6b5ead85d396249d5bdf450833f3a69896d76eb480d9c5e406/typing_extensions-3.7.4.2.tar.gz"
-    sha256 "79ee589a3caca649a9bfd2a8de4709837400dfa00b6cc81962a1e6a1815969ae"
-  end
-
   def install
     virtualenv_install_with_resources
   end

helper/homebrew/stable/proxy.rb

@@ -5,15 +5,11 @@ class Proxy < Formula
     Network monitoring, controls & Application development, testing, debugging."
   homepage "https://github.com/abhinavsingh/proxy.py"
   url "https://github.com/abhinavsingh/proxy.py/archive/master.zip"
-  version "2.2.0"
+  sha256 "715687cebd451285d266f29d6509a64becc93da21f61ba9b4414e7dc4ecaaeed"
+  version "2.3.1"
 
   depends_on "python"
 
-  resource "typing-extensions" do
-    url "https://files.pythonhosted.org/packages/6a/28/d32852f2af6b5ead85d396249d5bdf450833f3a69896d76eb480d9c5e406/typing_extensions-3.7.4.2.tar.gz"
-    sha256 "79ee589a3caca649a9bfd2a8de4709837400dfa00b6cc81962a1e6a1815969ae"
-  end
-
   def install
     virtualenv_install_with_resources
   end

proxy/core/acceptor/threadless.py

@@ -198,8 +198,6 @@ def run(self) -> None:
             self.loop = asyncio.get_event_loop()
             while not self.running.is_set():
                 self.run_once()
-        except KeyboardInterrupt:
-            pass
         finally:
             assert self.selector is not None
             self.selector.unregister(self.client_queue)

proxy/core/acceptor/work.py

@@ -73,7 +73,7 @@ def run(self) -> None:
         compatibility with threaded mode where work class is started as
         a separate thread.
         """
-        pass
+        pass    # pragma: no cover
 
     def publish_event(
             self,

proxy/http/parser.py

@@ -180,7 +180,8 @@ def parse(self, raw: bytes) -> None:
                     more = False
                 else:
                     raise NotImplementedError(
-                        'Parser shouldn\'t have reached here',
+                        'Parser shouldn\'t have reached here. ' +
+                        'This can happen when content length header is missing but their is a body in the payload',
                     )
             else:
                 more, raw = self.process(raw)
@@ -285,7 +286,8 @@ def build_response(self) -> bytes:
             headers={} if not self.headers else {
                 self.headers[k][0]: self.headers[k][1] for k in self.headers
             },
-            body=self.body if not self.is_chunked_encoded() else ChunkParser.to_chunks(self.body),
+            body=self.body if not self.is_chunked_encoded(
+            ) else ChunkParser.to_chunks(self.body),
         )
 
     def has_host(self) -> bool:

proxy/proxy.py

@@ -23,7 +23,7 @@
 import inspect
 
 from types import TracebackType
-from typing import Dict, List, Optional, Any, Tuple, Type, Union, cast
+from typing import Dict, List, Optional, Any, Type, Union, cast
 
 from proxy.core.acceptor.work import Work
 
@@ -205,7 +205,7 @@ def initialize(
         if input_args is None:
             input_args = []
 
-        if not Proxy.is_py3():
+        if Proxy.is_py2():
             print(PY2_DEPRECATION_MESSAGE)
             sys.exit(1)
 
@@ -230,16 +230,15 @@ def initialize(
         Proxy.set_open_file_limit(args.open_file_limit)
 
         # Load plugins
-        default_plugins = Proxy.get_default_plugins(args)
+        default_plugins = [bytes_(p) for p in Proxy.get_default_plugins(args)]
+        extra_plugins = [
+            p if isinstance(p, type) else bytes_(p)
+            for p in opts.get('plugins', args.plugins.split(text_(COMMA)))
+            if not (isinstance(p, str) and len(p) == 0)
+        ]
 
         # Load default plugins along with user provided --plugins
-        plugins = Proxy.load_plugins(
-            [bytes_(p) for p in collections.OrderedDict(default_plugins).keys()] +
-            [
-                p if isinstance(p, type) else bytes_(p)
-                for p in opts.get('plugins', args.plugins.split(text_(COMMA)))
-            ],
-        )
+        plugins = Proxy.load_plugins(default_plugins + extra_plugins)
 
         # proxy.py currently cannot serve over HTTPS and also perform TLS interception
         # at the same time.  Check if user is trying to enable both feature
@@ -413,8 +412,7 @@ def load_plugins(
         }
         for plugin_ in plugins:
             klass, module_name = Proxy.import_plugin(plugin_)
-            if klass is None and module_name is None:
-                continue
+            assert klass and module_name
             mro = list(inspect.getmro(klass))
             mro.reverse()
             iterator = iter(mro)
@@ -433,8 +431,7 @@ def import_plugin(plugin: Union[bytes, type]) -> Any:
             klass = plugin
         else:
             plugin_ = text_(plugin.strip())
-            if plugin_ == '':
-                return (None, None)
+            assert plugin_ != ''
             module_name, klass_name = plugin_.rsplit(text_(DOT), 1)
             klass = getattr(
                 importlib.import_module(
@@ -449,36 +446,37 @@ def import_plugin(plugin: Union[bytes, type]) -> Any:
     @staticmethod
     def get_default_plugins(
             args: argparse.Namespace,
-    ) -> List[Tuple[str, bool]]:
-        # Prepare list of plugins to load based upon
-        # --enable-*, --disable-* and --basic-auth flags.
-        default_plugins: List[Tuple[str, bool]] = []
+    ) -> List[str]:
+        """Prepare list of plugins to load based upon
+        --enable-*, --disable-* and --basic-auth flags.
+        """
+        default_plugins: List[str] = []
         if args.basic_auth is not None:
-            default_plugins.append((PLUGIN_PROXY_AUTH, True))
+            default_plugins.append(PLUGIN_PROXY_AUTH)
         if args.enable_dashboard:
-            default_plugins.append((PLUGIN_WEB_SERVER, True))
+            default_plugins.append(PLUGIN_WEB_SERVER)
             args.enable_static_server = True
-            default_plugins.append((PLUGIN_DASHBOARD, True))
-            default_plugins.append((PLUGIN_INSPECT_TRAFFIC, True))
+            default_plugins.append(PLUGIN_DASHBOARD)
+            default_plugins.append(PLUGIN_INSPECT_TRAFFIC)
             args.enable_events = True
             args.enable_devtools = True
         if args.enable_devtools:
-            default_plugins.append((PLUGIN_DEVTOOLS_PROTOCOL, True))
-            default_plugins.append((PLUGIN_WEB_SERVER, True))
+            default_plugins.append(PLUGIN_DEVTOOLS_PROTOCOL)
+            default_plugins.append(PLUGIN_WEB_SERVER)
         if not args.disable_http_proxy:
-            default_plugins.append((PLUGIN_HTTP_PROXY, True))
+            default_plugins.append(PLUGIN_HTTP_PROXY)
         if args.enable_web_server or \
                 args.pac_file is not None or \
                 args.enable_static_server:
-            default_plugins.append((PLUGIN_WEB_SERVER, True))
+            default_plugins.append(PLUGIN_WEB_SERVER)
         if args.pac_file is not None:
-            default_plugins.append((PLUGIN_PAC_FILE, True))
-        return default_plugins
+            default_plugins.append(PLUGIN_PAC_FILE)
+        return list(collections.OrderedDict.fromkeys(default_plugins).keys())
 
     @staticmethod
-    def is_py3() -> bool:
+    def is_py2() -> bool:
         """Exists only to avoid mocking sys.version_info in tests."""
-        return sys.version_info[0] != 2
+        return sys.version_info[0] == 2
 
     @staticmethod
     def set_open_file_limit(soft_limit: int) -> None:
@@ -515,7 +513,7 @@ def main(
             # at runtime.  Example, updating flags, plugin
             # configuration etc.
             #
-            # TODO: Python shell within running proxy.py environment
+            # TODO: Python shell within running proxy.py environment?
             while True:
                 time.sleep(1)
     except KeyboardInterrupt:

tests/http/test_http_parser.py

@@ -272,7 +272,10 @@ def test_get_partial_parse2(self) -> None:
         self.parser.parse(b'Content-Type: text/plain' + CRLF)
         self.assertEqual(self.parser.buffer, b'')
         self.assertEqual(
-            self.parser.headers[b'content-type'], (b'Content-Type', b'text/plain'),
+            self.parser.headers[b'content-type'], (
+                b'Content-Type',
+                b'text/plain',
+            ),
         )
         self.assertEqual(
             self.parser.state,
@@ -632,3 +635,33 @@ def test_paramiko_doc(self) -> None:
         self.parser = HttpParser(httpParserTypes.RESPONSE_PARSER)
         self.parser.parse(response)
         self.assertEqual(self.parser.state, httpParserStates.COMPLETE)
+
+    def test_request_factory(self) -> None:
+        r = HttpParser.request(
+            b'POST http://localhost:12345 HTTP/1.1' + CRLF +
+            b'key: value' + CRLF +
+            b'Content-Length: 13' + CRLF + CRLF +
+            b'Hello from py',
+        )
+        self.assertEqual(r.host, b'localhost')
+        self.assertEqual(r.port, 12345)
+        self.assertEqual(r.path, b'/')
+        self.assertEqual(r.header(b'key'), b'value')
+        self.assertEqual(r.header(b'KEY'), b'value')
+        self.assertEqual(r.header(b'content-length'), b'13')
+        self.assertEqual(r.body, b'Hello from py')
+
+    def test_response_factory(self) -> None:
+        r = HttpParser.response(
+            b'HTTP/1.1 200 OK\r\nkey: value\r\n\r\n',
+        )
+        self.assertEqual(r.code, b'200')
+        self.assertEqual(r.reason, b'OK')
+        self.assertEqual(r.header(b'key'), b'value')
+
+    def test_parser_shouldnt_have_reached_here(self) -> None:
+        with self.assertRaises(NotImplementedError):
+            HttpParser.request(
+                b'POST http://localhost:12345 HTTP/1.1' + CRLF +
+                b'key: value' + CRLF + CRLF + b'Hello from py',
+            )

tests/http/test_http_proxy.py

@@ -115,3 +115,42 @@ def test_proxy_plugin_before_upstream_connection_can_teardown(
         self.protocol_handler.run_once()
         mock_server_conn.assert_not_called()
         self.plugin.return_value.before_upstream_connection.assert_called()
+
+    def test_proxy_plugin_plugins_can_teardown_from_write_to_descriptors(self) -> None:
+        pass
+
+    def test_proxy_plugin_retries_on_ssl_want_write_error(self) -> None:
+        pass
+
+    def test_proxy_plugin_broken_pipe_error_on_write_will_teardown(self) -> None:
+        pass
+
+    def test_proxy_plugin_plugins_can_teardown_from_read_from_descriptors(self) -> None:
+        pass
+
+    def test_proxy_plugin_retries_on_ssl_want_read_error(self) -> None:
+        pass
+
+    def test_proxy_plugin_timeout_error_on_read_will_teardown(self) -> None:
+        pass
+
+    def test_proxy_plugin_invokes_handle_pipeline_response(self) -> None:
+        pass
+
+    def test_proxy_plugin_invokes_on_access_log(self) -> None:
+        pass
+
+    def test_proxy_plugin_skips_server_teardown_when_client_closes_and_server_never_initialized(self) -> None:
+        pass
+
+    def test_proxy_plugin_invokes_handle_client_data(self) -> None:
+        pass
+
+    def test_proxy_plugin_handles_pipeline_response(self) -> None:
+        pass
+
+    def test_proxy_plugin_invokes_resolve_dns(self) -> None:
+        pass
+
+    def test_proxy_plugin_require_both_host_port_to_connect(self) -> None:
+        pass