Fix Mock constructor params/url order mishandling (#111)

* Add test to reproduce #104

* Fix #104

src/pook/helpers.py

@@ -1,27 +1,50 @@
+import re
+
 from inspect import ismethod, isfunction
 from .exceptions import PookInvalidArgument
 
 
-def trigger_methods(instance, args):
+reply_response_re = re.compile("^(response|reply)_")
+
+
+def _get_key(key_order):
+    def key(x):
+        raw = reply_response_re.sub("", x)
+        try:
+            return key_order.index(raw)
+        except KeyError:
+            raise PookInvalidArgument("Unsupported argument: {}".format(x))
+
+    return key
+
+
+def trigger_methods(instance, args, key_order=None):
     """
     Triggers specific class methods using a simple reflection
     mechanism based on the given input dictionary params.
 
     Arguments:
         instance (object): target instance to dynamically trigger methods.
         args (iterable): input arguments to trigger objects to
+        key_order (None|iterable): optional order in which to process keys; falls back to `sorted`'s default behaviour if not present
 
     Returns:
         None
     """
     # Start the magic
-    for name in sorted(args):
+    if key_order:
+        key = _get_key(key_order)
+        sorted_args = sorted(args, key=key)
+    else:
+        sorted_args = sorted(args)
+
+    for name in sorted_args:
         value = args[name]
         target = instance
 
         # If response attibutes
-        if name.startswith("response_") or name.startswith("reply_"):
-            name = name.replace("response_", "").replace("reply_", "")
+        if reply_response_re.match(name):
+            name = reply_response_re.sub("", name)
             # If instance has response attribute, use it
             if hasattr(instance, "_response"):
                 target = instance._response

src/pook/mock.py

@@ -96,6 +96,46 @@ class Mock(object):
         pook.Mock
     """
 
+    _KEY_ORDER = (
+        "add_matcher",
+        "body",
+        "callback",
+        "calls",
+        "content",
+        "delay",
+        "done",
+        "error",
+        "file",
+        "filter",
+        "header",
+        "header_present",
+        "headers",
+        "headers_present",
+        "isdone",
+        "ismatched",
+        "json",
+        "jsonschema",
+        "map",
+        "match",
+        "matched",
+        "matches",
+        "method",
+        "url",
+        "param",
+        "param_exists",
+        "params",
+        "path",
+        "persist",
+        "reply",
+        "response",
+        "status",
+        "times",
+        "total_matches",
+        "type",
+        "use",
+        "xml",
+    )
+
     def __init__(self, request=None, response=None, **kw):
         # Stores the number of times the mock should live
         self._times = 1
@@ -126,7 +166,7 @@ def __init__(self, request=None, response=None, **kw):
         self.callbacks = []
 
         # Triggers instance methods based on argument names
-        trigger_methods(self, kw)
+        trigger_methods(self, kw, self._KEY_ORDER)
 
         # Trigger matchers based on predefined request object, if needed
         if request:

src/pook/request.py

@@ -44,7 +44,7 @@ def __init__(self, method="GET", **kw):
         self._extra = kw.get("extra")
         self._headers = HTTPHeaderDict()
 
-        trigger_methods(self, kw)
+        trigger_methods(self, kw, self.keys)
 
     @property
     def method(self):

src/pook/response.py

@@ -23,6 +23,20 @@ class Response(object):
         mock (pook.Mock): reference to mock instance.
     """
 
+    _KEY_ORDER = (
+        "body",
+        "content",
+        "file",
+        "header",
+        "headers",
+        "json",
+        "mock",
+        "set",
+        "status",
+        "type",
+        "xml",
+    )
+
     def __init__(self, **kw):
         self._status = 200
         self._mock = None
@@ -31,7 +45,7 @@ def __init__(self, **kw):
         self._headers = HTTPHeaderDict()
 
         # Trigger response method based on input arguments
-        trigger_methods(self, kw)
+        trigger_methods(self, kw, self._KEY_ORDER)
 
     def status(self, code=200):
         """

tests/unit/mock_test.py

@@ -1,6 +1,12 @@
 import pytest
+import json
+import re
+
+import pook
 from pook.mock import Mock
 from pook.request import Request
+from urllib.request import urlopen
+from urllib.parse import urlencode
 
 
 @pytest.fixture
@@ -17,6 +23,47 @@ def test_mock_url(mock):
     assert str(matcher(mock)) == "http://google.es"
 
 
+@pytest.mark.parametrize(
+    ("param_kwargs", "query_string"),
+    (
+        pytest.param({"params": {"x": "1"}}, "?x=1", id="params"),
+        pytest.param(
+            {"param": ("y", "pook")},
+            "?y=pook",
+            marks=pytest.mark.xfail(
+                condition=True,
+                reason="Constructor does not correctly handle multi-argument methods from kwargs",
+            ),
+            id="param",
+        ),
+        pytest.param(
+            {"param_exists": "z"},
+            # This complexity is needed until https://github.com/h2non/pook/issues/110
+            # is resolved
+            f'?{urlencode({"z": re.compile("(.*)")})}',
+            id="param_exists",
+        ),
+    ),
+)
+def test_constructor(param_kwargs, query_string):
+    # Should not raise
+    mock = Mock(
+        url="https://httpbin.org/404",
+        reply_status=200,
+        response_json={"hello": "from pook"},
+        **param_kwargs,
+    )
+
+    expected_url = f"https://httpbin.org/404{query_string}"
+    assert mock._request.rawurl == expected_url
+
+    with pook.use():
+        pook.engine().add_mock(mock)
+        res = urlopen(expected_url)
+        assert res.status == 200
+        assert json.loads(res.read()) == {"hello": "from pook"}
+
+
 @pytest.mark.parametrize(
     "url, params, req, expected",
     [