From 99617e9eb6f31d5b9355c6d0d839b4b6db34a90f Mon Sep 17 00:00:00 2001
From: Tom Most <tmost@blueplanet.com>
Date: Tue, 29 Sep 2020 14:33:20 -0700
Subject: [PATCH 01/19] Add changelog entry

---
 changelog.d/287.bugfix.rst | 2 ++
 1 file changed, 2 insertions(+)
 create mode 100644 changelog.d/287.bugfix.rst

diff --git a/changelog.d/287.bugfix.rst b/changelog.d/287.bugfix.rst
new file mode 100644
index 00000000..e42675a5
--- /dev/null
+++ b/changelog.d/287.bugfix.rst
@@ -0,0 +1,2 @@
+treq request functions and methods like :func:`treq.get()` and :meth:`HTTPClient.post()` now issue a ``DeprecationWarning``.
+This will change to a ``TypeError`` in the next treq release.

From 10e9736d55af8cf0f2ffedb6f9fff7760cb3f234 Mon Sep 17 00:00:00 2001
From: Tom Most <tmost@blueplanet.com>
Date: Tue, 29 Sep 2020 15:52:30 -0700
Subject: [PATCH 02/19] pop from kwargs

---
 src/treq/client.py | 147 ++++++++++++++++++++++++++++++---------------
 1 file changed, 98 insertions(+), 49 deletions(-)

diff --git a/src/treq/client.py b/src/treq/client.py
index 4c13b3a9..fa708fbd 100644
--- a/src/treq/client.py
+++ b/src/treq/client.py
@@ -38,6 +38,9 @@
 from requests.cookies import cookiejar_from_dict, merge_cookies
 
 
+_NOTHING = object()
+
+
 def urlencode(query, doseq):
     return six.ensure_binary(_urlencode(query, doseq), encoding='ascii')
 
@@ -153,7 +156,7 @@ def request(self, method, url, **kwargs):
 
         # Join parameters provided in the URL
         # and the ones passed as argument.
-        params = kwargs.get('params')
+        params = kwargs.pop('params', None)
         if params:
             parsed_url = parsed_url.replace(
                 query=parsed_url.query + tuple(_coerced_query_params(params))
@@ -163,7 +166,7 @@ def request(self, method, url, **kwargs):
 
         # Convert headers dictionary to
         # twisted raw headers format.
-        headers = kwargs.get('headers')
+        headers = kwargs.pop('headers', None)
         if headers:
             if isinstance(headers, dict):
                 h = Headers({})
@@ -177,48 +180,15 @@ def request(self, method, url, **kwargs):
         else:
             headers = Headers({})
 
-        # Here we choose a right producer
-        # based on the parameters passed in.
-        bodyProducer = None
-        data = kwargs.get('data')
-        files = kwargs.get('files')
-        # since json=None needs to be serialized as 'null', we need to
-        # explicitly check kwargs for this key
-        has_json = 'json' in kwargs
-
-        if files:
-            # If the files keyword is present we will issue a
-            # multipart/form-data request as it suits better for cases
-            # with files and/or large objects.
-            files = list(_convert_files(files))
-            boundary = str(uuid.uuid4()).encode('ascii')
-            headers.setRawHeaders(
-                b'content-type', [
-                    b'multipart/form-data; boundary=' + boundary])
-            if data:
-                data = _convert_params(data)
-            else:
-                data = []
+        bodyProducer, contentType = self._request_body(
+            data=kwargs.pop('data', _NOTHING),
+            files=kwargs.pop('files', _NOTHING),
+            json=kwargs.pop('json', _NOTHING),
+        )
+        if contentType is not None:
+            headers.setRawHeaders(b'Content-Type', [contentType])
 
-            bodyProducer = multipart.MultiPartProducer(
-                data + files, boundary=boundary)
-        elif data:
-            # Otherwise stick to x-www-form-urlencoded format
-            # as it's generally faster for smaller requests.
-            if isinstance(data, (dict, list, tuple)):
-                headers.setRawHeaders(
-                    b'content-type', [b'application/x-www-form-urlencoded'])
-                data = urlencode(data, doseq=True)
-            bodyProducer = self._data_to_body_producer(data)
-        elif has_json:
-            # If data is sent as json, set Content-Type as 'application/json'
-            headers.setRawHeaders(
-                b'content-type', [b'application/json; charset=UTF-8'])
-            content = kwargs['json']
-            json = json_dumps(content, separators=(u',', u':')).encode('utf-8')
-            bodyProducer = self._data_to_body_producer(json)
-
-        cookies = kwargs.get('cookies', {})
+        cookies = kwargs.pop('cookies', {})
 
         if not isinstance(cookies, CookieJar):
             cookies = cookiejar_from_dict(cookies)
@@ -226,8 +196,9 @@ def request(self, method, url, **kwargs):
         cookies = merge_cookies(self._cookiejar, cookies)
         wrapped_agent = CookieAgent(self._agent, cookies)
 
-        if kwargs.get('allow_redirects', True):
-            if kwargs.get('browser_like_redirects', False):
+        browser_like_redirects = kwargs.pop('browser_like_redirects', False)
+        if kwargs.pop('allow_redirects', True):
+            if browser_like_redirects:
                 wrapped_agent = BrowserLikeRedirectAgent(wrapped_agent)
             else:
                 wrapped_agent = RedirectAgent(wrapped_agent)
@@ -235,7 +206,7 @@ def request(self, method, url, **kwargs):
         wrapped_agent = ContentDecoderAgent(wrapped_agent,
                                             [(b'gzip', GzipDecoder)])
 
-        auth = kwargs.get('auth')
+        auth = kwargs.pop('auth', None)
         if auth:
             wrapped_agent = add_auth(wrapped_agent, auth)
 
@@ -243,9 +214,10 @@ def request(self, method, url, **kwargs):
             method, url, headers=headers,
             bodyProducer=bodyProducer)
 
-        timeout = kwargs.get('timeout')
+        reactor = kwargs.pop('reactor', None)
+        timeout = kwargs.pop('timeout', None)
         if timeout:
-            delayedCall = default_reactor(kwargs.get('reactor')).callLater(
+            delayedCall = default_reactor(reactor).callLater(
                 timeout, d.cancel)
 
             def gotResult(result):
@@ -255,11 +227,88 @@ def gotResult(result):
 
             d.addBoth(gotResult)
 
-        if not kwargs.get('unbuffered', False):
+        if not kwargs.pop('unbuffered', False):
             d.addCallback(_BufferedResponse)
 
         return d.addCallback(_Response, cookies)
 
+    def _request_body(self, data, files, json):
+        """
+        Here we choose a right producer based on the parameters passed in.
+
+        :params data:
+            Arbitrary request body data.
+
+            If *files* is also passed this must be a :class:`dict`,
+            a :class:`tuple` or :class:`list` of field tuples as accepted by
+            :class:`MultiPartProducer`. The request is assigned a Content-Type
+            of ``multipart/form-data``.
+
+            If a :class:`dict`, :class:`list`, or :class:`tuple` it is
+            URL-encoded and the request assigned a Content-Type of
+            ``application/x-www-form-urlencoded``.
+
+            Otherwise, any non-``None`` value is passed to the client's
+            *data_to_body_producer* callable (by default,
+            :class:`IBodyProducer`), which accepts file-like objects.
+
+        :params files:
+            Files to include in the request body, in any of the several formats
+            described in :func:`_convert_files()`.
+
+        :params json:
+            JSON-encodable data, or the sentinel `_NOTHING`.
+        """
+        # Not all combinations of keyword arguments are meaningful. These make
+        # sense:
+        #
+        # - files=...
+        # - data=...
+        # - files=... and data=...
+        # - json=...
+        #
+        # TODO: Deprecate passing json when files or data is passed, as it is
+        # ignored.
+
+        if files is not _NOTHING and files:
+            # If the files keyword is present we will issue a
+            # multipart/form-data request as it suits better for cases
+            # with files and/or large objects.
+            files = list(_convert_files(files))
+            boundary = str(uuid.uuid4()).encode('ascii')
+            if data is not _NOTHING and data:
+                data = _convert_params(data)
+            else:
+                data = []
+
+            return (
+                multipart.MultiPartProducer(data + files, boundary=boundary),
+                b'multipart/form-data; boundary=' + boundary,
+            )
+
+        # Otherwise stick to x-www-form-urlencoded format
+        # as it's generally faster for smaller requests.
+        if isinstance(data, (dict, list, tuple)):
+            return (
+                self._data_to_body_producer(urlencode(data, doseq=True)),
+                b'application/x-www-form-urlencoded',
+            )
+        elif data and data is not _NOTHING:
+            return (
+                self._data_to_body_producer(data),
+                None,
+            )
+
+        if json is not _NOTHING:
+            return (
+                self._data_to_body_producer(
+                    json_dumps(json, separators=(u',', u':')).encode('utf-8'),
+                ),
+                b'application/json; charset=UTF-8',
+            )
+
+        return None, None
+
 
 def _convert_params(params):
     if hasattr(params, "iteritems"):

From 771a60a69a77ba85be2931e340fb3d851107d504 Mon Sep 17 00:00:00 2001
From: Tom Most <tmost@blueplanet.com>
Date: Tue, 29 Sep 2020 16:47:53 -0700
Subject: [PATCH 03/19] Generate a warning

---
 src/treq/client.py           | 12 ++++++++++++
 src/treq/test/test_client.py | 17 +++++++++++++++++
 tox.ini                      |  4 +++-
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/src/treq/client.py b/src/treq/client.py
index fa708fbd..160d6d23 100644
--- a/src/treq/client.py
+++ b/src/treq/client.py
@@ -2,6 +2,7 @@
 
 import mimetypes
 import uuid
+import warnings
 
 import io
 
@@ -230,6 +231,17 @@ def gotResult(result):
         if not kwargs.pop('unbuffered', False):
             d.addCallback(_BufferedResponse)
 
+        if kwargs:
+            warnings.warn(
+                (
+                    "Got unexpected keyword argument: {}."
+                    " treq will ignore this argument,"
+                    " but will raise TypeError in the next treq release."
+                ).format(", ".join(repr(k) for k in kwargs)),
+                DeprecationWarning,
+                stacklevel=2,
+            )
+
         return d.addCallback(_Response, cookies)
 
     def _request_body(self, data, files, json):
diff --git a/src/treq/test/test_client.py b/src/treq/test/test_client.py
index 1334fc7a..ae1cde64 100644
--- a/src/treq/test/test_client.py
+++ b/src/treq/test/test_client.py
@@ -473,6 +473,23 @@ def test_request_dict_headers(self):
                      b'Accept': [b'application/json', b'text/plain']}),
             None)
 
+    def test_request_invalid_param(self):
+        """
+        `HTTPClient.request()` warns that invalid parameters are ignored and
+        that this is deprecated.
+        """
+        self.client.request('GET', 'http://example.com', invalid=True)
+
+        [w] = self.flushWarnings([self.test_request_invalid_param])
+        self.assertEqual(
+            (
+                "Got unexpected keyword argument: 'invalid'."
+                " treq will ignore this argument,"
+                " but will raise TypeError in the next treq release."
+            ),
+            w['message'],
+        )
+
     @with_clock
     def test_request_timeout_fired(self, clock):
         """
diff --git a/tox.ini b/tox.ini
index b20489ec..9e6a8aca 100644
--- a/tox.ini
+++ b/tox.ini
@@ -26,7 +26,9 @@ setenv =
 passenv = TERM  # ensure colors
 commands =
     pip list
-    coverage run {envbindir}/trial {posargs:treq}
+    python -Wall \
+    {envbindir}/coverage run \
+    {envbindir}/trial {posargs:treq}
     coverage report -m
 
 [testenv:flake8]

From 133551dc6eadd68e77150d087cc8d0d7532175b8 Mon Sep 17 00:00:00 2001
From: Tom Most <tmost@blueplanet.com>
Date: Tue, 29 Sep 2020 17:04:26 -0700
Subject: [PATCH 04/19] Deprecate mixing json with data or files

---
 src/treq/client.py           | 33 ++++++++++++++--------------
 src/treq/test/test_client.py | 42 ++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 17 deletions(-)

diff --git a/src/treq/client.py b/src/treq/client.py
index 160d6d23..188fd8a6 100644
--- a/src/treq/client.py
+++ b/src/treq/client.py
@@ -182,8 +182,8 @@ def request(self, method, url, **kwargs):
             headers = Headers({})
 
         bodyProducer, contentType = self._request_body(
-            data=kwargs.pop('data', _NOTHING),
-            files=kwargs.pop('files', _NOTHING),
+            data=kwargs.pop('data', None),
+            files=kwargs.pop('files', None),
             json=kwargs.pop('json', _NOTHING),
         )
         if contentType is not None:
@@ -269,26 +269,25 @@ def _request_body(self, data, files, json):
             described in :func:`_convert_files()`.
 
         :params json:
-            JSON-encodable data, or the sentinel `_NOTHING`.
+            JSON-encodable data, or the sentinel `_NOTHING`. The sentinel is
+            necessary because ``None`` is a valid JSON value.
         """
-        # Not all combinations of keyword arguments are meaningful. These make
-        # sense:
-        #
-        # - files=...
-        # - data=...
-        # - files=... and data=...
-        # - json=...
-        #
-        # TODO: Deprecate passing json when files or data is passed, as it is
-        # ignored.
-
-        if files is not _NOTHING and files:
+        if json is not _NOTHING and (files or data):
+            warnings.warn(
+                (
+                    "Argument 'json' will be ignored because '{}' was also passed."
+                    " This will raise TypeError in the next treq release."
+                ).format("data" if data else "files"),
+                stacklevel=3,
+            )
+
+        if files:
             # If the files keyword is present we will issue a
             # multipart/form-data request as it suits better for cases
             # with files and/or large objects.
             files = list(_convert_files(files))
             boundary = str(uuid.uuid4()).encode('ascii')
-            if data is not _NOTHING and data:
+            if data:
                 data = _convert_params(data)
             else:
                 data = []
@@ -305,7 +304,7 @@ def _request_body(self, data, files, json):
                 self._data_to_body_producer(urlencode(data, doseq=True)),
                 b'application/x-www-form-urlencoded',
             )
-        elif data and data is not _NOTHING:
+        elif data:
             return (
                 self._data_to_body_producer(data),
                 None,
diff --git a/src/treq/test/test_client.py b/src/treq/test/test_client.py
index ae1cde64..6f850731 100644
--- a/src/treq/test/test_client.py
+++ b/src/treq/test/test_client.py
@@ -460,6 +460,48 @@ def test_request_unsupported_params_combination(self):
                           data=BytesIO(b"yo"),
                           files={"file1": BytesIO(b"hey")})
 
+    def test_request_json_with_data(self):
+        """
+        Passing `HTTPClient.request()` both *data* and *json* parameters is
+        invalid because *json* is ignored. This behavior is deprecated.
+        """
+        self.client.request(
+            "POST",
+            "http://example.com/",
+            data=BytesIO(b"..."),
+            json=None,  # NB: None is a valid value. It encodes to b'null'.
+        )
+
+        [w] = self.flushWarnings([self.test_request_json_with_data])
+        self.assertEqual(
+            (
+                "Argument 'json' will be ignored because 'data' was also passed."
+                " This will raise TypeError in the next treq release."
+            ),
+            w['message'],
+        )
+
+    def test_request_json_with_files(self):
+        """
+        Passing `HTTPClient.request()` both *files* and *json* parameters is
+        invalid because *json* is ignored. This behavior is deprecated.
+        """
+        self.client.request(
+            "POST",
+            "http://example.com/",
+            files={"f1": ("foo.txt", "text/plain", BytesIO(b"...\n"))},
+            json=["this is ignored"],
+        )
+
+        [w] = self.flushWarnings([self.test_request_json_with_files])
+        self.assertEqual(
+            (
+                "Argument 'json' will be ignored because 'files' was also passed."
+                " This will raise TypeError in the next treq release."
+            ),
+            w['message'],
+        )
+
     def test_request_dict_headers(self):
         self.client.request('GET', 'http://example.com/', headers={
             'User-Agent': 'treq/0.1dev',

From b9da50e6c113a1689463653c4204922b0239ded7 Mon Sep 17 00:00:00 2001
From: Tom Most <tmost@blueplanet.com>
Date: Tue, 29 Sep 2020 17:19:43 -0700
Subject: [PATCH 05/19] Remove unused *args

---
 src/treq/api.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/treq/api.py b/src/treq/api.py
index 88bb868d..9b5e190f 100644
--- a/src/treq/api.py
+++ b/src/treq/api.py
@@ -130,7 +130,7 @@ def request(method, url, **kwargs):
 # Private API
 #
 
-def _client(*args, **kwargs):
+def _client(**kwargs):
     agent = kwargs.get('agent')
     if agent is None:
         reactor = default_reactor(kwargs.get('reactor'))

From 93df28d9f7184c73af17253b5503b09360152179 Mon Sep 17 00:00:00 2001
From: Tom Most <tmost@blueplanet.com>
Date: Tue, 29 Sep 2020 17:21:03 -0700
Subject: [PATCH 06/19] Pass kwargs by reference

---
 src/treq/api.py | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/treq/api.py b/src/treq/api.py
index 9b5e190f..d90f0e13 100644
--- a/src/treq/api.py
+++ b/src/treq/api.py
@@ -12,7 +12,7 @@ def head(url, **kwargs):
 
     See :py:func:`treq.request`
     """
-    return _client(**kwargs).head(url, **kwargs)
+    return _client(kwargs).head(url, **kwargs)
 
 
 def get(url, headers=None, **kwargs):
@@ -21,7 +21,7 @@ def get(url, headers=None, **kwargs):
 
     See :py:func:`treq.request`
     """
-    return _client(**kwargs).get(url, headers=headers, **kwargs)
+    return _client(kwargs).get(url, headers=headers, **kwargs)
 
 
 def post(url, data=None, **kwargs):
@@ -30,7 +30,7 @@ def post(url, data=None, **kwargs):
 
     See :py:func:`treq.request`
     """
-    return _client(**kwargs).post(url, data=data, **kwargs)
+    return _client(kwargs).post(url, data=data, **kwargs)
 
 
 def put(url, data=None, **kwargs):
@@ -39,7 +39,7 @@ def put(url, data=None, **kwargs):
 
     See :py:func:`treq.request`
     """
-    return _client(**kwargs).put(url, data=data, **kwargs)
+    return _client(kwargs).put(url, data=data, **kwargs)
 
 
 def patch(url, data=None, **kwargs):
@@ -48,7 +48,7 @@ def patch(url, data=None, **kwargs):
 
     See :py:func:`treq.request`
     """
-    return _client(**kwargs).patch(url, data=data, **kwargs)
+    return _client(kwargs).patch(url, data=data, **kwargs)
 
 
 def delete(url, **kwargs):
@@ -57,7 +57,7 @@ def delete(url, **kwargs):
 
     See :py:func:`treq.request`
     """
-    return _client(**kwargs).delete(url, **kwargs)
+    return _client(kwargs).delete(url, **kwargs)
 
 
 def request(method, url, **kwargs):
@@ -123,14 +123,14 @@ def request(method, url, **kwargs):
         The *url* param now accepts :class:`hyperlink.DecodedURL` and
         :class:`hyperlink.EncodedURL` objects.
     """
-    return _client(**kwargs).request(method, url, **kwargs)
+    return _client(kwargs).request(method, url, **kwargs)
 
 
 #
 # Private API
 #
 
-def _client(**kwargs):
+def _client(kwargs):
     agent = kwargs.get('agent')
     if agent is None:
         reactor = default_reactor(kwargs.get('reactor'))

From f63d5521c71f0de0a199329798000abfbd3a2f25 Mon Sep 17 00:00:00 2001
From: Tom Most <tmost@blueplanet.com>
Date: Tue, 29 Sep 2020 17:35:54 -0700
Subject: [PATCH 07/19] Merge treq._utils into treq.api

Put all the icky global state in one place.
---
 src/treq/_utils.py          | 47 ----------------------
 src/treq/api.py             | 44 ++++++++++++++++++++-
 src/treq/client.py          |  6 +--
 src/treq/test/test_api.py   | 76 +++++++++++++++++++++++++++++++++++-
 src/treq/test/test_utils.py | 77 -------------------------------------
 5 files changed, 119 insertions(+), 131 deletions(-)
 delete mode 100644 src/treq/_utils.py
 delete mode 100644 src/treq/test/test_utils.py

diff --git a/src/treq/_utils.py b/src/treq/_utils.py
deleted file mode 100644
index 69719ff9..00000000
--- a/src/treq/_utils.py
+++ /dev/null
@@ -1,47 +0,0 @@
-"""
-Strictly internal utilities.
-"""
-
-from __future__ import absolute_import, division, print_function
-
-from twisted.web.client import HTTPConnectionPool
-
-
-def default_reactor(reactor):
-    """
-    Return the specified reactor or the default.
-    """
-    if reactor is None:
-        from twisted.internet import reactor
-
-    return reactor
-
-
-_global_pool = [None]
-
-
-def get_global_pool():
-    return _global_pool[0]
-
-
-def set_global_pool(pool):
-    _global_pool[0] = pool
-
-
-def default_pool(reactor, pool, persistent):
-    """
-    Return the specified pool or a a pool with the specified reactor and
-    persistence.
-    """
-    reactor = default_reactor(reactor)
-
-    if pool is not None:
-        return pool
-
-    if persistent is False:
-        return HTTPConnectionPool(reactor, persistent=persistent)
-
-    if get_global_pool() is None:
-        set_global_pool(HTTPConnectionPool(reactor, persistent=True))
-
-    return get_global_pool()
diff --git a/src/treq/api.py b/src/treq/api.py
index d90f0e13..cc8ccb85 100644
--- a/src/treq/api.py
+++ b/src/treq/api.py
@@ -1,9 +1,8 @@
 from __future__ import absolute_import, division, print_function
 
-from twisted.web.client import Agent
+from twisted.web.client import Agent, HTTPConnectionPool
 
 from treq.client import HTTPClient
-from treq._utils import default_pool, default_reactor
 
 
 def head(url, **kwargs):
@@ -130,6 +129,47 @@ def request(method, url, **kwargs):
 # Private API
 #
 
+
+def default_reactor(reactor):
+    """
+    Return the specified reactor or the default.
+    """
+    if reactor is None:
+        from twisted.internet import reactor
+
+    return reactor
+
+
+_global_pool = [None]
+
+
+def get_global_pool():
+    return _global_pool[0]
+
+
+def set_global_pool(pool):
+    _global_pool[0] = pool
+
+
+def default_pool(reactor, pool, persistent):
+    """
+    Return the specified pool or a a pool with the specified reactor and
+    persistence.
+    """
+    reactor = default_reactor(reactor)
+
+    if pool is not None:
+        return pool
+
+    if persistent is False:
+        return HTTPConnectionPool(reactor, persistent=persistent)
+
+    if get_global_pool() is None:
+        set_global_pool(HTTPConnectionPool(reactor, persistent=True))
+
+    return get_global_pool()
+
+
 def _client(kwargs):
     agent = kwargs.get('agent')
     if agent is None:
diff --git a/src/treq/client.py b/src/treq/client.py
index 188fd8a6..9bf2e5d1 100644
--- a/src/treq/client.py
+++ b/src/treq/client.py
@@ -32,7 +32,6 @@
 from twisted.python.components import registerAdapter
 from json import dumps as json_dumps
 
-from treq._utils import default_reactor
 from treq.auth import add_auth
 from treq import multipart
 from treq.response import _Response
@@ -216,10 +215,11 @@ def request(self, method, url, **kwargs):
             bodyProducer=bodyProducer)
 
         reactor = kwargs.pop('reactor', None)
+        if reactor is None:
+            from twisted.internet import reactor
         timeout = kwargs.pop('timeout', None)
         if timeout:
-            delayedCall = default_reactor(reactor).callLater(
-                timeout, d.cancel)
+            delayedCall = reactor.callLater(timeout, d.cancel)
 
             def gotResult(result):
                 if delayedCall.active():
diff --git a/src/treq/test/test_api.py b/src/treq/test/test_api.py
index 61d1b026..85cd704a 100644
--- a/src/treq/test/test_api.py
+++ b/src/treq/test/test_api.py
@@ -5,7 +5,7 @@
 from twisted.trial.unittest import TestCase
 
 import treq
-from treq._utils import set_global_pool
+from treq.api import default_reactor, default_pool, set_global_pool
 
 
 class TreqAPITests(TestCase):
@@ -20,7 +20,7 @@ def setUp(self):
         self.HTTPClient = client_patcher.start()
         self.addCleanup(client_patcher.stop)
 
-        pool_patcher = mock.patch('treq._utils.HTTPConnectionPool')
+        pool_patcher = mock.patch('treq.api.HTTPConnectionPool')
         self.HTTPConnectionPool = pool_patcher.start()
         self.addCleanup(pool_patcher.stop)
 
@@ -54,3 +54,75 @@ def test_custom_agent(self):
         custom_agent = mock.Mock()
         treq.get('https://www.example.org/', agent=custom_agent)
         self.HTTPClient.assert_called_once_with(custom_agent)
+
+
+class DefaultReactorTests(TestCase):
+    def test_passes_reactor(self):
+        mock_reactor = mock.Mock()
+
+        self.assertEqual(default_reactor(mock_reactor), mock_reactor)
+
+    def test_uses_default_reactor(self):
+        from twisted.internet import reactor
+        self.assertEqual(default_reactor(None), reactor)
+
+
+class DefaultPoolTests(TestCase):
+    def setUp(self):
+        set_global_pool(None)
+
+        pool_patcher = mock.patch('treq.api.HTTPConnectionPool')
+
+        self.HTTPConnectionPool = pool_patcher.start()
+        self.addCleanup(pool_patcher.stop)
+
+        self.reactor = mock.Mock()
+
+    def test_persistent_false(self):
+        self.assertEqual(
+            default_pool(self.reactor, None, False),
+            self.HTTPConnectionPool.return_value
+        )
+
+        self.HTTPConnectionPool.assert_called_once_with(
+            self.reactor, persistent=False
+        )
+
+    def test_pool_none_persistent_none(self):
+        self.assertEqual(
+            default_pool(self.reactor, None, None),
+            self.HTTPConnectionPool.return_value
+        )
+
+        self.HTTPConnectionPool.assert_called_once_with(
+            self.reactor, persistent=True
+        )
+
+    def test_pool_none_persistent_true(self):
+        self.assertEqual(
+            default_pool(self.reactor, None, True),
+            self.HTTPConnectionPool.return_value
+        )
+
+        self.HTTPConnectionPool.assert_called_once_with(
+            self.reactor, persistent=True
+        )
+
+    def test_cached_global_pool(self):
+        pool1 = default_pool(self.reactor, None, None)
+
+        self.HTTPConnectionPool.return_value = mock.Mock()
+
+        pool2 = default_pool(self.reactor, None, True)
+
+        self.assertEqual(pool1, pool2)
+
+    def test_specified_pool(self):
+        pool = mock.Mock()
+
+        self.assertEqual(
+            default_pool(self.reactor, pool, None),
+            pool
+        )
+
+        self.HTTPConnectionPool.assert_not_called()
diff --git a/src/treq/test/test_utils.py b/src/treq/test/test_utils.py
deleted file mode 100644
index 2023ddc4..00000000
--- a/src/treq/test/test_utils.py
+++ /dev/null
@@ -1,77 +0,0 @@
-import mock
-
-from twisted.trial.unittest import TestCase
-
-from treq._utils import default_reactor, default_pool, set_global_pool
-
-
-class DefaultReactorTests(TestCase):
-    def test_passes_reactor(self):
-        mock_reactor = mock.Mock()
-
-        self.assertEqual(default_reactor(mock_reactor), mock_reactor)
-
-    def test_uses_default_reactor(self):
-        from twisted.internet import reactor
-        self.assertEqual(default_reactor(None), reactor)
-
-
-class DefaultPoolTests(TestCase):
-    def setUp(self):
-        set_global_pool(None)
-
-        pool_patcher = mock.patch('treq._utils.HTTPConnectionPool')
-
-        self.HTTPConnectionPool = pool_patcher.start()
-        self.addCleanup(pool_patcher.stop)
-
-        self.reactor = mock.Mock()
-
-    def test_persistent_false(self):
-        self.assertEqual(
-            default_pool(self.reactor, None, False),
-            self.HTTPConnectionPool.return_value
-        )
-
-        self.HTTPConnectionPool.assert_called_once_with(
-            self.reactor, persistent=False
-        )
-
-    def test_pool_none_persistent_none(self):
-        self.assertEqual(
-            default_pool(self.reactor, None, None),
-            self.HTTPConnectionPool.return_value
-        )
-
-        self.HTTPConnectionPool.assert_called_once_with(
-            self.reactor, persistent=True
-        )
-
-    def test_pool_none_persistent_true(self):
-        self.assertEqual(
-            default_pool(self.reactor, None, True),
-            self.HTTPConnectionPool.return_value
-        )
-
-        self.HTTPConnectionPool.assert_called_once_with(
-            self.reactor, persistent=True
-        )
-
-    def test_cached_global_pool(self):
-        pool1 = default_pool(self.reactor, None, None)
-
-        self.HTTPConnectionPool.return_value = mock.Mock()
-
-        pool2 = default_pool(self.reactor, None, True)
-
-        self.assertEqual(pool1, pool2)
-
-    def test_specified_pool(self):
-        pool = mock.Mock()
-
-        self.assertEqual(
-            default_pool(self.reactor, pool, None),
-            pool
-        )
-
-        self.HTTPConnectionPool.assert_not_called()

From c7872cd98df2b1e3315c727ac2ae39b1a1663b79 Mon Sep 17 00:00:00 2001
From: Tom Most <tmost@blueplanet.com>
Date: Tue, 29 Sep 2020 18:26:45 -0700
Subject: [PATCH 08/19] Demock pool tests

---
 src/treq/test/test_api.py | 97 ++++++++++++++++++++++++---------------
 1 file changed, 59 insertions(+), 38 deletions(-)

diff --git a/src/treq/test/test_api.py b/src/treq/test/test_api.py
index 85cd704a..e06e3f66 100644
--- a/src/treq/test/test_api.py
+++ b/src/treq/test/test_api.py
@@ -2,10 +2,12 @@
 
 import mock
 
+from twisted.web.client import HTTPConnectionPool
 from twisted.trial.unittest import TestCase
+from twisted.internet.testing import MemoryReactorClock
 
 import treq
-from treq.api import default_reactor, default_pool, set_global_pool
+from treq.api import default_reactor, default_pool, set_global_pool, get_global_pool
 
 
 class TreqAPITests(TestCase):
@@ -68,61 +70,80 @@ def test_uses_default_reactor(self):
 
 
 class DefaultPoolTests(TestCase):
+    """
+    Test `treq.api.default_pool`.
+    """
     def setUp(self):
         set_global_pool(None)
+        self.reactor = MemoryReactorClock()
 
-        pool_patcher = mock.patch('treq.api.HTTPConnectionPool')
+    def test_persistent_false(self):
+        """
+        When *persistent=False* is passed a non-persistent pool is created.
+        """
+        pool = default_pool(self.reactor, None, False)
 
-        self.HTTPConnectionPool = pool_patcher.start()
-        self.addCleanup(pool_patcher.stop)
+        self.assertTrue(isinstance(pool, HTTPConnectionPool))
+        self.assertFalse(pool.persistent)
+
+    def test_persistent_false_not_stored(self):
+        """
+        When *persistent=False* is passed the resulting pool is not stored as
+        the global pool.
+        """
+        pool = default_pool(self.reactor, None, persistent=False)
 
-        self.reactor = mock.Mock()
+        self.assertIsNot(pool, get_global_pool())
 
-    def test_persistent_false(self):
-        self.assertEqual(
-            default_pool(self.reactor, None, False),
-            self.HTTPConnectionPool.return_value
-        )
+    def test_persistent_false_new(self):
+        """
+        When *persistent=False* is passed a new pool is returned each time.
+        """
+        pool1 = default_pool(self.reactor, None, persistent=False)
+        pool2 = default_pool(self.reactor, None, persistent=False)
 
-        self.HTTPConnectionPool.assert_called_once_with(
-            self.reactor, persistent=False
-        )
+        self.assertIsNot(pool1, pool2)
 
     def test_pool_none_persistent_none(self):
-        self.assertEqual(
-            default_pool(self.reactor, None, None),
-            self.HTTPConnectionPool.return_value
-        )
+        """
+        When *persistent=None* is passed a _persistent_ pool is created for
+        backwards compatibility.
+        """
+        pool = default_pool(self.reactor, None, None)
 
-        self.HTTPConnectionPool.assert_called_once_with(
-            self.reactor, persistent=True
-        )
+        self.assertTrue(pool.persistent)
 
     def test_pool_none_persistent_true(self):
-        self.assertEqual(
-            default_pool(self.reactor, None, True),
-            self.HTTPConnectionPool.return_value
-        )
+        """
+        When *persistent=True* is passed a persistent pool is created and
+        stored as the global pool.
+        """
+        pool = default_pool(self.reactor, None, True)
 
-        self.HTTPConnectionPool.assert_called_once_with(
-            self.reactor, persistent=True
-        )
+        self.assertTrue(isinstance(pool, HTTPConnectionPool))
+        self.assertTrue(pool.persistent)
 
     def test_cached_global_pool(self):
+        """
+        When *persistent=True* or *persistent=None* is passed the pool created
+        is cached as the global pool.
+        """
         pool1 = default_pool(self.reactor, None, None)
-
-        self.HTTPConnectionPool.return_value = mock.Mock()
-
         pool2 = default_pool(self.reactor, None, True)
 
         self.assertEqual(pool1, pool2)
 
     def test_specified_pool(self):
-        pool = mock.Mock()
-
-        self.assertEqual(
-            default_pool(self.reactor, pool, None),
-            pool
-        )
-
-        self.HTTPConnectionPool.assert_not_called()
+        """
+        When the user passes a pool it is returned directly. The *persistent*
+        argument is ignored. It is not cached as the global pool.
+        """
+        user_pool = HTTPConnectionPool(self.reactor, persistent=True)
+        pool1 = default_pool(self.reactor, user_pool, None)
+        pool2 = default_pool(self.reactor, user_pool, True)
+        pool3 = default_pool(self.reactor, user_pool, False)
+
+        self.assertIs(pool1, user_pool)
+        self.assertIs(pool2, user_pool)
+        self.assertIs(pool3, user_pool)
+        self.assertIsNot(get_global_pool(), user_pool)

From 66e72282655744869ca677e1b839a64274b9a69f Mon Sep 17 00:00:00 2001
From: Tom Most <tmost@blueplanet.com>
Date: Tue, 29 Sep 2020 18:28:51 -0700
Subject: [PATCH 09/19] Demock default_reactor tests

---
 src/treq/test/test_api.py | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/treq/test/test_api.py b/src/treq/test/test_api.py
index e06e3f66..ac8eac64 100644
--- a/src/treq/test/test_api.py
+++ b/src/treq/test/test_api.py
@@ -59,12 +59,21 @@ def test_custom_agent(self):
 
 
 class DefaultReactorTests(TestCase):
+    """
+    Test `treq.api.default_reactor()`
+    """
     def test_passes_reactor(self):
-        mock_reactor = mock.Mock()
+        """
+        `default_reactor()` returns any reactor passed.
+        """
+        reactor = MemoryReactorClock()
 
-        self.assertEqual(default_reactor(mock_reactor), mock_reactor)
+        self.assertIs(default_reactor(reactor), reactor)
 
     def test_uses_default_reactor(self):
+        """
+        `default_reactor()` returns the global reactor when passed ``None``.
+        """
         from twisted.internet import reactor
         self.assertEqual(default_reactor(None), reactor)
 

From 3b84ff2d5e3e986c8100d8c708a5ecfacf3744fe Mon Sep 17 00:00:00 2001
From: Tom Most <tmost@blueplanet.com>
Date: Tue, 29 Sep 2020 19:06:49 -0700
Subject: [PATCH 10/19] Demock module-level API tests

---
 src/treq/test/test_api.py | 92 ++++++++++++++++++++++++++-------------
 1 file changed, 61 insertions(+), 31 deletions(-)

diff --git a/src/treq/test/test_api.py b/src/treq/test/test_api.py
index ac8eac64..b23c1310 100644
--- a/src/treq/test/test_api.py
+++ b/src/treq/test/test_api.py
@@ -1,61 +1,91 @@
 from __future__ import absolute_import, division
 
-import mock
-
+from twisted.web.iweb import IAgent
 from twisted.web.client import HTTPConnectionPool
 from twisted.trial.unittest import TestCase
+from twisted.internet import defer
 from twisted.internet.testing import MemoryReactorClock
+from zope.interface import implementer
 
 import treq
 from treq.api import default_reactor, default_pool, set_global_pool, get_global_pool
 
 
-class TreqAPITests(TestCase):
-    def setUp(self):
-        set_global_pool(None)
-
-        agent_patcher = mock.patch('treq.api.Agent')
-        self.Agent = agent_patcher.start()
-        self.addCleanup(agent_patcher.stop)
-
-        client_patcher = mock.patch('treq.api.HTTPClient')
-        self.HTTPClient = client_patcher.start()
-        self.addCleanup(client_patcher.stop)
+class SyntacticAbominationHTTPConnectionPool(object):
+    """
+    A HTTP connection pool that always fails to return a connection,
+    but counts the number of requests made.
+    """
+    requests = 0
 
-        pool_patcher = mock.patch('treq.api.HTTPConnectionPool')
-        self.HTTPConnectionPool = pool_patcher.start()
-        self.addCleanup(pool_patcher.stop)
+    def getConnection(self, key, endpoint):
+        """
+        Count each request, then fail with `IndentationError`.
+        """
+        self.requests += 1
+        return defer.fail(TabError())
 
-        self.client = self.HTTPClient.return_value
 
+class TreqAPITests(TestCase):
     def test_default_pool(self):
-        resp = treq.get('http://test.com')
+        """
+        The module-level API uses the global connection pool by default.
+        """
+        pool = SyntacticAbominationHTTPConnectionPool()
+        set_global_pool(pool)
 
-        self.Agent.assert_called_once_with(
-            mock.ANY,
-            pool=self.HTTPConnectionPool.return_value
-        )
+        d = treq.get('http://test.com')
 
-        self.assertEqual(self.client.get.return_value, resp)
+        self.assertEqual(pool.requests, 1)
+        self.failureResultOf(d, TabError)
 
     def test_cached_pool(self):
-        pool = self.HTTPConnectionPool.return_value
+        """
+        The first use of the module-level API populates the global connection
+        pool, which is used for all subsequent requests.
+        """
+        pool = SyntacticAbominationHTTPConnectionPool()
+        self.patch(treq.api, 'HTTPConnectionPool', lambda reactor, persistent: pool)
 
-        treq.get('http://test.com')
+        self.failureResultOf(treq.head("http://test.com"), TabError)
+        self.failureResultOf(treq.get("http://test.com"), TabError)
+        self.failureResultOf(treq.post("http://test.com"), TabError)
+        self.failureResultOf(treq.put("http://test.com"), TabError)
+        self.failureResultOf(treq.delete("http://test.com"), TabError)
+        self.failureResultOf(treq.request("OPTIONS", "http://test.com"), TabError)
 
-        self.HTTPConnectionPool.return_value = mock.Mock()
+        self.assertEqual(pool.requests, 6)
 
-        treq.get('http://test.com')
+    def test_custom_pool(self):
+        """
+        `treq.post()` accepts a *pool* argument to use for the request. The
+        global pool is unaffected.
+        """
+        pool = SyntacticAbominationHTTPConnectionPool()
 
-        self.Agent.assert_called_with(mock.ANY, pool=pool)
+        d = treq.post('http://foo', data=b'bar', pool=pool)
+
+        self.assertEqual(pool.requests, 1)
+        self.failureResultOf(d, TabError)
+        self.assertIsNot(pool, get_global_pool())
 
     def test_custom_agent(self):
         """
         A custom Agent is used if specified.
         """
-        custom_agent = mock.Mock()
-        treq.get('https://www.example.org/', agent=custom_agent)
-        self.HTTPClient.assert_called_once_with(custom_agent)
+        @implementer(IAgent)
+        class CounterAgent(object):
+            requests = 0
+
+            def request(self, method, uri, headers=None, bodyProducer=None):
+                self.requests += 1
+                return defer.Deferred()
+
+        custom_agent = CounterAgent()
+        d = treq.get('https://www.example.org/', agent=custom_agent)
+
+        self.assertNoResult(d)
+        self.assertEqual(1, custom_agent.requests)
 
 
 class DefaultReactorTests(TestCase):

From c432fe08efeaacc28948634f39edb664b5f379d4 Mon Sep 17 00:00:00 2001
From: Tom Most <tmost@blueplanet.com>
Date: Tue, 29 Sep 2020 19:14:57 -0700
Subject: [PATCH 11/19] Fade to black

---
 src/treq/test/test_api.py | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/treq/test/test_api.py b/src/treq/test/test_api.py
index b23c1310..412799fa 100644
--- a/src/treq/test/test_api.py
+++ b/src/treq/test/test_api.py
@@ -16,6 +16,7 @@ class SyntacticAbominationHTTPConnectionPool(object):
     A HTTP connection pool that always fails to return a connection,
     but counts the number of requests made.
     """
+
     requests = 0
 
     def getConnection(self, key, endpoint):
@@ -34,7 +35,7 @@ def test_default_pool(self):
         pool = SyntacticAbominationHTTPConnectionPool()
         set_global_pool(pool)
 
-        d = treq.get('http://test.com')
+        d = treq.get("http://test.com")
 
         self.assertEqual(pool.requests, 1)
         self.failureResultOf(d, TabError)
@@ -45,7 +46,7 @@ def test_cached_pool(self):
         pool, which is used for all subsequent requests.
         """
         pool = SyntacticAbominationHTTPConnectionPool()
-        self.patch(treq.api, 'HTTPConnectionPool', lambda reactor, persistent: pool)
+        self.patch(treq.api, "HTTPConnectionPool", lambda reactor, persistent: pool)
 
         self.failureResultOf(treq.head("http://test.com"), TabError)
         self.failureResultOf(treq.get("http://test.com"), TabError)
@@ -63,7 +64,7 @@ def test_custom_pool(self):
         """
         pool = SyntacticAbominationHTTPConnectionPool()
 
-        d = treq.post('http://foo', data=b'bar', pool=pool)
+        d = treq.post("http://foo", data=b"bar", pool=pool)
 
         self.assertEqual(pool.requests, 1)
         self.failureResultOf(d, TabError)
@@ -73,6 +74,7 @@ def test_custom_agent(self):
         """
         A custom Agent is used if specified.
         """
+
         @implementer(IAgent)
         class CounterAgent(object):
             requests = 0
@@ -82,7 +84,7 @@ def request(self, method, uri, headers=None, bodyProducer=None):
                 return defer.Deferred()
 
         custom_agent = CounterAgent()
-        d = treq.get('https://www.example.org/', agent=custom_agent)
+        d = treq.get("https://www.example.org/", agent=custom_agent)
 
         self.assertNoResult(d)
         self.assertEqual(1, custom_agent.requests)
@@ -92,6 +94,7 @@ class DefaultReactorTests(TestCase):
     """
     Test `treq.api.default_reactor()`
     """
+
     def test_passes_reactor(self):
         """
         `default_reactor()` returns any reactor passed.
@@ -105,6 +108,7 @@ def test_uses_default_reactor(self):
         `default_reactor()` returns the global reactor when passed ``None``.
         """
         from twisted.internet import reactor
+
         self.assertEqual(default_reactor(None), reactor)
 
 
@@ -112,6 +116,7 @@ class DefaultPoolTests(TestCase):
     """
     Test `treq.api.default_pool`.
     """
+
     def setUp(self):
         set_global_pool(None)
         self.reactor = MemoryReactorClock()

From fea41a1637d46cfb6b70bf07f618c1b6f77c71dc Mon Sep 17 00:00:00 2001
From: Tom Most <tmost@blueplanet.com>
Date: Tue, 29 Sep 2020 19:19:29 -0700
Subject: [PATCH 12/19] Test module-level API deprecations

---
 src/treq/test/test_api.py | 52 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/src/treq/test/test_api.py b/src/treq/test/test_api.py
index 412799fa..9e83cb84 100644
--- a/src/treq/test/test_api.py
+++ b/src/treq/test/test_api.py
@@ -89,6 +89,58 @@ def request(self, method, uri, headers=None, bodyProducer=None):
         self.assertNoResult(d)
         self.assertEqual(1, custom_agent.requests)
 
+    def test_request_invalid_param(self):
+        """
+        `treq.request()` warns that it ignores unknown keyword arguments, but
+        this is deprecated.
+
+        This test verifies that stacklevel is set appropriately when issuing
+        the warning.
+        """
+        self.failureResultOf(
+            treq.request(
+                "GET",
+                "https://foo.bar",
+                invalid=True,
+                pool=SyntacticAbominationHTTPConnectionPool(),
+            )
+        )
+
+        [w] = self.flushWarnings([self.test_request_invalid_param])
+        self.assertEqual(
+            (
+                "Got unexpected keyword argument: 'invalid'."
+                " treq will ignore this argument,"
+                " but will raise TypeError in the next treq release."
+            ),
+            w["message"],
+        )
+
+    def test_post_json_with_data(self):
+        """
+        `treq.post()` warns that mixing *data* and *json* is deprecated.
+
+        This test verifies that stacklevel is set appropriately when issuing
+        the warning.
+        """
+        self.failureResultOf(
+            treq.post(
+                "https://test.example/",
+                data={"hello": "world"},
+                json={"goodnight": "moon"},
+                pool=SyntacticAbominationHTTPConnectionPool(),
+            )
+        )
+
+        [w] = self.flushWarnings([self.test_post_json_with_data])
+        self.assertEqual(
+            (
+                "Argument 'json' will be ignored because 'data' was also passed."
+                " This will raise TypeError in the next treq release."
+            ),
+            w["message"],
+        )
+
 
 class DefaultReactorTests(TestCase):
     """

From 148f009ed883cf7623f18916720972a0fdded534 Mon Sep 17 00:00:00 2001
From: Tom Most <tmost@blueplanet.com>
Date: Tue, 29 Sep 2020 19:38:12 -0700
Subject: [PATCH 13/19] Fix stacklevel for module-level API

---
 src/treq/api.py              | 27 ++++++++++++++-------------
 src/treq/client.py           | 15 ++++++++++++---
 src/treq/test/test_api.py    |  2 ++
 src/treq/test/test_client.py |  2 ++
 4 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/src/treq/api.py b/src/treq/api.py
index cc8ccb85..e1232e10 100644
--- a/src/treq/api.py
+++ b/src/treq/api.py
@@ -11,7 +11,7 @@ def head(url, **kwargs):
 
     See :py:func:`treq.request`
     """
-    return _client(kwargs).head(url, **kwargs)
+    return _client(kwargs).head(url, _stacklevel=4, **kwargs)
 
 
 def get(url, headers=None, **kwargs):
@@ -20,7 +20,7 @@ def get(url, headers=None, **kwargs):
 
     See :py:func:`treq.request`
     """
-    return _client(kwargs).get(url, headers=headers, **kwargs)
+    return _client(kwargs).get(url, headers=headers, _stacklevel=4, **kwargs)
 
 
 def post(url, data=None, **kwargs):
@@ -29,7 +29,7 @@ def post(url, data=None, **kwargs):
 
     See :py:func:`treq.request`
     """
-    return _client(kwargs).post(url, data=data, **kwargs)
+    return _client(kwargs).post(url, data=data, _stacklevel=4, **kwargs)
 
 
 def put(url, data=None, **kwargs):
@@ -38,7 +38,7 @@ def put(url, data=None, **kwargs):
 
     See :py:func:`treq.request`
     """
-    return _client(kwargs).put(url, data=data, **kwargs)
+    return _client(kwargs).put(url, data=data, _stacklevel=4, **kwargs)
 
 
 def patch(url, data=None, **kwargs):
@@ -47,7 +47,7 @@ def patch(url, data=None, **kwargs):
 
     See :py:func:`treq.request`
     """
-    return _client(kwargs).patch(url, data=data, **kwargs)
+    return _client(kwargs).patch(url, data=data, _stacklevel=4, **kwargs)
 
 
 def delete(url, **kwargs):
@@ -56,7 +56,7 @@ def delete(url, **kwargs):
 
     See :py:func:`treq.request`
     """
-    return _client(kwargs).delete(url, **kwargs)
+    return _client(kwargs).delete(url, _stacklevel=4, **kwargs)
 
 
 def request(method, url, **kwargs):
@@ -122,7 +122,7 @@ def request(method, url, **kwargs):
         The *url* param now accepts :class:`hyperlink.DecodedURL` and
         :class:`hyperlink.EncodedURL` objects.
     """
-    return _client(kwargs).request(method, url, **kwargs)
+    return _client(kwargs).request(method, url, _stacklevel=3, **kwargs)
 
 
 #
@@ -153,7 +153,7 @@ def set_global_pool(pool):
 
 def default_pool(reactor, pool, persistent):
     """
-    Return the specified pool or a a pool with the specified reactor and
+    Return the specified pool or a pool with the specified reactor and
     persistence.
     """
     reactor = default_reactor(reactor)
@@ -171,11 +171,12 @@ def default_pool(reactor, pool, persistent):
 
 
 def _client(kwargs):
-    agent = kwargs.get('agent')
+    agent = kwargs.pop("agent", None)
+    reactor = kwargs.pop("reactor", None)
+    pool = kwargs.pop("pool", None)
+    persistent = kwargs.pop("persistent", None)
     if agent is None:
-        reactor = default_reactor(kwargs.get('reactor'))
-        pool = default_pool(reactor,
-                            kwargs.get('pool'),
-                            kwargs.get('persistent'))
+        reactor = default_reactor(reactor)
+        pool = default_pool(reactor, pool, persistent)
         agent = Agent(reactor, pool=pool)
     return HTTPClient(agent)
diff --git a/src/treq/client.py b/src/treq/client.py
index 9bf2e5d1..113d6bb6 100644
--- a/src/treq/client.py
+++ b/src/treq/client.py
@@ -107,36 +107,42 @@ def get(self, url, **kwargs):
         """
         See :func:`treq.get()`.
         """
+        kwargs.setdefault('_stacklevel', 3)
         return self.request('GET', url, **kwargs)
 
     def put(self, url, data=None, **kwargs):
         """
         See :func:`treq.put()`.
         """
+        kwargs.setdefault('_stacklevel', 3)
         return self.request('PUT', url, data=data, **kwargs)
 
     def patch(self, url, data=None, **kwargs):
         """
         See :func:`treq.patch()`.
         """
+        kwargs.setdefault('_stacklevel', 3)
         return self.request('PATCH', url, data=data, **kwargs)
 
     def post(self, url, data=None, **kwargs):
         """
         See :func:`treq.post()`.
         """
+        kwargs.setdefault('_stacklevel', 3)
         return self.request('POST', url, data=data, **kwargs)
 
     def head(self, url, **kwargs):
         """
         See :func:`treq.head()`.
         """
+        kwargs.setdefault('_stacklevel', 3)
         return self.request('HEAD', url, **kwargs)
 
     def delete(self, url, **kwargs):
         """
         See :func:`treq.delete()`.
         """
+        kwargs.setdefault('_stacklevel', 3)
         return self.request('DELETE', url, **kwargs)
 
     def request(self, method, url, **kwargs):
@@ -144,6 +150,7 @@ def request(self, method, url, **kwargs):
         See :func:`treq.request()`.
         """
         method = method.encode('ascii').upper()
+        stacklevel = kwargs.pop('_stacklevel', 2)
 
         if isinstance(url, DecodedURL):
             parsed_url = url
@@ -184,6 +191,7 @@ def request(self, method, url, **kwargs):
             data=kwargs.pop('data', None),
             files=kwargs.pop('files', None),
             json=kwargs.pop('json', _NOTHING),
+            stacklevel=stacklevel,
         )
         if contentType is not None:
             headers.setRawHeaders(b'Content-Type', [contentType])
@@ -239,12 +247,12 @@ def gotResult(result):
                     " but will raise TypeError in the next treq release."
                 ).format(", ".join(repr(k) for k in kwargs)),
                 DeprecationWarning,
-                stacklevel=2,
+                stacklevel=stacklevel,
             )
 
         return d.addCallback(_Response, cookies)
 
-    def _request_body(self, data, files, json):
+    def _request_body(self, data, files, json, stacklevel):
         """
         Here we choose a right producer based on the parameters passed in.
 
@@ -278,7 +286,8 @@ def _request_body(self, data, files, json):
                     "Argument 'json' will be ignored because '{}' was also passed."
                     " This will raise TypeError in the next treq release."
                 ).format("data" if data else "files"),
-                stacklevel=3,
+                DeprecationWarning,
+                stacklevel=stacklevel + 1,
             )
 
         if files:
diff --git a/src/treq/test/test_api.py b/src/treq/test/test_api.py
index 9e83cb84..f1a010d9 100644
--- a/src/treq/test/test_api.py
+++ b/src/treq/test/test_api.py
@@ -107,6 +107,7 @@ def test_request_invalid_param(self):
         )
 
         [w] = self.flushWarnings([self.test_request_invalid_param])
+        self.assertEqual(DeprecationWarning, w["category"])
         self.assertEqual(
             (
                 "Got unexpected keyword argument: 'invalid'."
@@ -133,6 +134,7 @@ def test_post_json_with_data(self):
         )
 
         [w] = self.flushWarnings([self.test_post_json_with_data])
+        self.assertEqual(DeprecationWarning, w["category"])
         self.assertEqual(
             (
                 "Argument 'json' will be ignored because 'data' was also passed."
diff --git a/src/treq/test/test_client.py b/src/treq/test/test_client.py
index 6f850731..d66d3a60 100644
--- a/src/treq/test/test_client.py
+++ b/src/treq/test/test_client.py
@@ -473,6 +473,7 @@ def test_request_json_with_data(self):
         )
 
         [w] = self.flushWarnings([self.test_request_json_with_data])
+        self.assertEqual(DeprecationWarning, w["category"])
         self.assertEqual(
             (
                 "Argument 'json' will be ignored because 'data' was also passed."
@@ -494,6 +495,7 @@ def test_request_json_with_files(self):
         )
 
         [w] = self.flushWarnings([self.test_request_json_with_files])
+        self.assertEqual(DeprecationWarning, w["category"])
         self.assertEqual(
             (
                 "Argument 'json' will be ignored because 'files' was also passed."

From 14e07d7c72dd7fb3a38183b0a70eeeac101164e6 Mon Sep 17 00:00:00 2001
From: Tom Most <tmost@blueplanet.com>
Date: Tue, 29 Sep 2020 19:40:30 -0700
Subject: [PATCH 14/19] Update changelog

---
 changelog.d/287.bugfix.rst | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/changelog.d/287.bugfix.rst b/changelog.d/287.bugfix.rst
index e42675a5..43191d12 100644
--- a/changelog.d/287.bugfix.rst
+++ b/changelog.d/287.bugfix.rst
@@ -1,2 +1,3 @@
-treq request functions and methods like :func:`treq.get()` and :meth:`HTTPClient.post()` now issue a ``DeprecationWarning``.
-This will change to a ``TypeError`` in the next treq release.
+treq request functions and methods like :func:`treq.get()` and :meth:`HTTPClient.post()` now issue a ``DeprecationWarning`` when passed unknown keyword arguments, rather than ignoring them.
+Mixing the *json* argument with *files* or *data* is also deprecated.
+These warnings will change to a ``TypeError`` in the next treq release.

From 6abb976446d8af4bf217c5c47c77e3218f7dc452 Mon Sep 17 00:00:00 2001
From: Tom Most <tmost@blueplanet.com>
Date: Tue, 29 Sep 2020 21:06:56 -0700
Subject: [PATCH 15/19] Update changelog

---
 changelog.d/{287.bugfix.rst => 297.removal.rst} | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename changelog.d/{287.bugfix.rst => 297.removal.rst} (100%)

diff --git a/changelog.d/287.bugfix.rst b/changelog.d/297.removal.rst
similarity index 100%
rename from changelog.d/287.bugfix.rst
rename to changelog.d/297.removal.rst

From 92175b7dccacbb63355459830991e9b193e90596 Mon Sep 17 00:00:00 2001
From: Tom Most <tmost@blueplanet.com>
Date: Tue, 29 Sep 2020 21:18:57 -0700
Subject: [PATCH 16/19] Configure flake8 for Black

---
 tox.ini | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tox.ini b/tox.ini
index 9e6a8aca..942321d2 100644
--- a/tox.ini
+++ b/tox.ini
@@ -60,3 +60,9 @@ commands =
 changedir = docs
 commands =
     sphinx-build -b html . html
+
+[flake8]
+# This is a minimal Black-compatible config.
+# See https://black.readthedocs.io/en/stable/compatible_configs.html#flake8
+max-line-length = 88
+extend-ignore = E203, W503

From 9fc9ee1a8968ec57d2f602ed5addb6182f3d0b7a Mon Sep 17 00:00:00 2001
From: Tom Most <tmost@blueplanet.com>
Date: Tue, 29 Sep 2020 21:21:16 -0700
Subject: [PATCH 17/19] Backward-compatible import

---
 src/treq/test/test_api.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/treq/test/test_api.py b/src/treq/test/test_api.py
index f1a010d9..782a35f4 100644
--- a/src/treq/test/test_api.py
+++ b/src/treq/test/test_api.py
@@ -4,12 +4,16 @@
 from twisted.web.client import HTTPConnectionPool
 from twisted.trial.unittest import TestCase
 from twisted.internet import defer
-from twisted.internet.testing import MemoryReactorClock
 from zope.interface import implementer
 
 import treq
 from treq.api import default_reactor, default_pool, set_global_pool, get_global_pool
 
+try:
+    from twisted.internet.testing import MemoryReactorClock
+except ImportError:
+    from twisted.test.proto_helpers import MemoryReactorClock
+
 
 class SyntacticAbominationHTTPConnectionPool(object):
     """

From 7a0cd40d9bf9c810d7566f9a2fc311df9033e2ad Mon Sep 17 00:00:00 2001
From: Tom Most <tmost@blueplanet.com>
Date: Tue, 29 Sep 2020 22:17:26 -0700
Subject: [PATCH 18/19] Pass down reactor

---
 src/treq/api.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/treq/api.py b/src/treq/api.py
index e1232e10..9439d418 100644
--- a/src/treq/api.py
+++ b/src/treq/api.py
@@ -172,11 +172,12 @@ def default_pool(reactor, pool, persistent):
 
 def _client(kwargs):
     agent = kwargs.pop("agent", None)
-    reactor = kwargs.pop("reactor", None)
     pool = kwargs.pop("pool", None)
     persistent = kwargs.pop("persistent", None)
     if agent is None:
-        reactor = default_reactor(reactor)
+        # "reactor" isn't removed from kwargs because it must also be passed
+        # down for use in the timeout logic.
+        reactor = default_reactor(kwargs.get("reactor"))
         pool = default_pool(reactor, pool, persistent)
         agent = Agent(reactor, pool=pool)
     return HTTPClient(agent)

From 5fbc08cd136b16534a6fc62dfeb701994acfc36e Mon Sep 17 00:00:00 2001
From: Tom Most <tmost@blueplanet.com>
Date: Tue, 29 Sep 2020 22:20:07 -0700
Subject: [PATCH 19/19] Juice coverage

---
 src/treq/test/test_client.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/treq/test/test_client.py b/src/treq/test/test_client.py
index d66d3a60..51a316d2 100644
--- a/src/treq/test/test_client.py
+++ b/src/treq/test/test_client.py
@@ -522,7 +522,7 @@ def test_request_invalid_param(self):
         `HTTPClient.request()` warns that invalid parameters are ignored and
         that this is deprecated.
         """
-        self.client.request('GET', 'http://example.com', invalid=True)
+        self.client.request('GET', b'http://example.com', invalid=True)
 
         [w] = self.flushWarnings([self.test_request_invalid_param])
         self.assertEqual(