Testing: Replace @patch decorators (#182)

* Testing: Replace @patch decorators

The @patch decorator clashes with py.test's fixture support because py.test
tries to interpret the argument names as fixtures and doesn't know about
@patch filling the slots.

Rather than performing deep hackery, teaching either of these tools to work
that way, this patch elects to simply replace these decorators with
"with patch() …" constructs.

* fix flake8 errors

including two that were not my fault ;-)

8 files changed, 170 insertions, 171 deletions

diff --git a/amqp/serialization.py b/amqp/serialization.py
index 7cf0ad3..8b8210d 100644
--- a/amqp/serialization.py
+++ b/amqp/serialization.py
@@ -242,7 +242,7 @@ def _flushbits(bits, write, pack=pack):
 
 
 def dumps(format, values):
-    """"Serialize AMQP arguments.
+    """Serialize AMQP arguments.
 
     Notes:
         bit = b
diff --git a/amqp/utils.py b/amqp/utils.py
index 2f785df..1f312c9 100644
--- a/amqp/utils.py
+++ b/amqp/utils.py
@@ -85,7 +85,7 @@ else:
 
     def str_to_bytes(s):                # noqa
         """Convert str to bytes."""
-        if isinstance(s, unicode):
+        if isinstance(s, string_t):
             return s.encode('utf-8')
         return s
 
diff --git a/t/unit/test_abstract_channel.py b/t/unit/test_abstract_channel.py
index e9283af..ca38e2d 100644
--- a/t/unit/test_abstract_channel.py
+++ b/t/unit/test_abstract_channel.py
@@ -64,23 +64,23 @@ class test_AbstractChannel:
         with pytest.raises(NotImplementedError):
             self.c.close()
 
-    @patch('amqp.abstract_channel.ensure_promise')
-    def test_wait(self, ensure_promise):
-        p = ensure_promise.return_value
-        p.ready = False
+    def test_wait(self):
+        with patch('amqp.abstract_channel.ensure_promise') as ensure_promise:
+            p = ensure_promise.return_value
+            p.ready = False
 
-        def on_drain(*args, **kwargs):
-            p.ready = True
-        self.conn.drain_events.side_effect = on_drain
+            def on_drain(*args, **kwargs):
+                p.ready = True
+            self.conn.drain_events.side_effect = on_drain
 
-        p.value = (1,), {'arg': 2}
-        self.c.wait((50, 61), timeout=1)
-        self.conn.drain_events.assert_called_with(timeout=1)
+            p.value = (1,), {'arg': 2}
+            self.c.wait((50, 61), timeout=1)
+            self.conn.drain_events.assert_called_with(timeout=1)
 
-        prev = self.c._pending[(50, 61)] = Mock(name='p2')
-        p.value = None
-        self.c.wait([(50, 61)])
-        assert self.c._pending[(50, 61)] is prev
+            prev = self.c._pending[(50, 61)] = Mock(name='p2')
+            p.value = None
+            self.c.wait([(50, 61)])
+            assert self.c._pending[(50, 61)] is prev
 
     def test_dispatch_method__content_encoding(self):
         self.c.auto_decode = True
@@ -107,20 +107,20 @@ class test_AbstractChannel:
         p.assert_called_with()
         assert not self.c._pending
 
-    @patch('amqp.abstract_channel.loads')
-    def test_dispatch_method__listeners(self, loads):
-        loads.return_value = [1, 2, 3], 'foo'
-        p = self.c._callbacks[(50, 61)] = Mock(name='p')
-        self.c.dispatch_method((50, 61), 'payload', self.content)
-        p.assert_called_with(1, 2, 3, self.content)
-
-    @patch('amqp.abstract_channel.loads')
-    def test_dispatch_method__listeners_and_one_shot(self, loads):
-        loads.return_value = [1, 2, 3], 'foo'
-        p1 = self.c._callbacks[(50, 61)] = Mock(name='p')
-        p2 = self.c._pending[(50, 61)] = Mock(name='oneshot')
-        self.c.dispatch_method((50, 61), 'payload', self.content)
-        p1.assert_called_with(1, 2, 3, self.content)
-        p2.assert_called_with(1, 2, 3, self.content)
-        assert not self.c._pending
-        assert self.c._callbacks[(50, 61)]
+    def test_dispatch_method__listeners(self):
+        with patch('amqp.abstract_channel.loads') as loads:
+            loads.return_value = [1, 2, 3], 'foo'
+            p = self.c._callbacks[(50, 61)] = Mock(name='p')
+            self.c.dispatch_method((50, 61), 'payload', self.content)
+            p.assert_called_with(1, 2, 3, self.content)
+
+    def test_dispatch_method__listeners_and_one_shot(self):
+        with patch('amqp.abstract_channel.loads') as loads:
+            loads.return_value = [1, 2, 3], 'foo'
+            p1 = self.c._callbacks[(50, 61)] = Mock(name='p')
+            p2 = self.c._pending[(50, 61)] = Mock(name='oneshot')
+            self.c.dispatch_method((50, 61), 'payload', self.content)
+            p1.assert_called_with(1, 2, 3, self.content)
+            p2.assert_called_with(1, 2, 3, self.content)
+            assert not self.c._pending
+            assert self.c._callbacks[(50, 61)]
diff --git a/t/unit/test_channel.py b/t/unit/test_channel.py
index 2b3c53f..7fc0aac 100644
--- a/t/unit/test_channel.py
+++ b/t/unit/test_channel.py
@@ -138,13 +138,13 @@ class test_Channel:
             wait=spec.Exchange.DeclareOk,
         )
 
-    @patch('amqp.channel.warn')
-    def test_exchange_declare__auto_delete(self, warn):
-        self.c.exchange_declare(
-            'foo', 'direct', False, True,
-            auto_delete=True, nowait=False, arguments={'x': 1},
-        )
-        warn.assert_called()
+    def test_exchange_declare__auto_delete(self):
+        with patch('amqp.channel.warn') as warn:
+            self.c.exchange_declare(
+                'foo', 'direct', False, True,
+                auto_delete=True, nowait=False, arguments={'x': 1},
+            )
+            warn.assert_called()
 
     def test_exchange_delete(self):
         self.c.exchange_delete('foo')
@@ -366,14 +366,14 @@ class test_Channel:
         with pytest.raises(NotFound):
             self.c._on_basic_return(404, 'text', 'ex', 'rkey', 'msg')
 
-    @patch('amqp.channel.error_for_code')
-    def test_on_basic_return__handled(self, error_for_code):
-        callback = Mock(name='callback')
-        self.c.events['basic_return'].add(callback)
-        self.c._on_basic_return(404, 'text', 'ex', 'rkey', 'msg')
-        callback.assert_called_with(
-            error_for_code(), 'ex', 'rkey', 'msg',
-        )
+    def test_on_basic_return__handled(self):
+        with patch('amqp.channel.error_for_code') as error_for_code:
+            callback = Mock(name='callback')
+            self.c.events['basic_return'].add(callback)
+            self.c._on_basic_return(404, 'text', 'ex', 'rkey', 'msg')
+            callback.assert_called_with(
+                error_for_code(), 'ex', 'rkey', 'msg',
+            )
 
     def test_tx_commit(self):
         self.c.tx_commit()
diff --git a/t/unit/test_platform.py b/t/unit/test_platform.py
index fb48d64..fd6ed2d 100644
--- a/t/unit/test_platform.py
+++ b/t/unit/test_platform.py
@@ -13,7 +13,7 @@ def reload_module(module):
         import importlib
         importlib.reload(module)
     except Exception:
-        reload(module)
+        reload(module) # noqa -- does not exist on Python3
 
 
 def test_struct_argument_type():
diff --git a/t/unit/test_sasl.py b/t/unit/test_sasl.py
index 84ef1d5..06c9dc1 100644
--- a/t/unit/test_sasl.py
+++ b/t/unit/test_sasl.py
@@ -88,9 +88,9 @@ class test_SASL:
             else:
                 sys.modules['gssapi.raw.misc'] = orig_gssapi_raw_misc
 
-    @patch('socket.gethostbyaddr')
-    def test_gssapi_rdns(self, gethostbyaddr):
-        with self.fake_gssapi() as gssapi:
+    def test_gssapi_rdns(self):
+        with self.fake_gssapi() as gssapi, \
+                patch('socket.gethostbyaddr') as gethostbyaddr:
             connection = Mock()
             connection.transport.sock.getpeername.return_value = ('192.0.2.0',
                                                                   5672)
diff --git a/t/unit/test_transport.py b/t/unit/test_transport.py
index ad50600..3e2223a 100644
--- a/t/unit/test_transport.py
+++ b/t/unit/test_transport.py
@@ -346,113 +346,111 @@ class test_AbstractTransport_connect:
         if fcntl is not None:
             patching('fcntl.fcntl')
 
-    @patch('socket.socket', side_effect=socket.error)
-    def test_connect_socket_fails(self, sock_mock):
-        with pytest.raises(socket.error):
-            self.t.connect()
-
-    @patch('socket.socket', side_effect=socket.error)
-    @patch('socket.getaddrinfo',
-           return_value=[
-               (socket.AF_INET, 1, socket.IPPROTO_TCP,
-                '', ('127.0.0.1', 5672)),
-               (socket.AF_INET, 1, socket.IPPROTO_TCP,
-                '', ('127.0.0.2', 5672))
-           ])
-    def test_connect_socket_initialization_fails(self, getaddrinfo, sock_mock):
-        with pytest.raises(socket.error):
-            self.t.connect()
-
-    @patch('socket.socket', return_value=MockSocket())
-    @patch('socket.getaddrinfo',
-           return_value=[
-               (socket.AF_INET, 1, socket.IPPROTO_TCP,
-                '', ('127.0.0.1', 5672)),
-               (socket.AF_INET, 1, socket.IPPROTO_TCP,
-                '', ('127.0.0.2', 5672))
-           ])
-    def test_connect_multiple_addr_entries_fails(self, getaddrinfo, sock_mock):
-        self.t.sock = Mock()
-        self.t.close()
-        with patch.object(sock_mock.return_value, 'connect',
-                          side_effect=socket.error):
+    def test_connect_socket_fails(self):
+        with patch('socket.socket', side_effect=socket.error):
             with pytest.raises(socket.error):
                 self.t.connect()
 
-    @patch('socket.socket', return_value=MockSocket())
-    @patch('socket.getaddrinfo',
-           return_value=[
-               (socket.AF_INET, 1, socket.IPPROTO_TCP,
-                '', ('127.0.0.1', 5672)),
-               (socket.AF_INET, 1, socket.IPPROTO_TCP,
-                '', ('127.0.0.2', 5672))
-           ])
-    def test_connect_multiple_addr_entries_succeed(self, getaddrinfo,
-                                                   sock_mock):
-        self.t.sock = Mock()
-        self.t.close()
-        with patch.object(sock_mock.return_value, 'connect',
-                          side_effect=(socket.error, None)):
-            self.t.connect()
+    def test_connect_socket_initialization_fails(self):
+        with patch('socket.socket', side_effect=socket.error), \
+            patch('socket.getaddrinfo',
+                  return_value=[
+                      (socket.AF_INET, 1, socket.IPPROTO_TCP,
+                          '', ('127.0.0.1', 5672)),
+                      (socket.AF_INET, 1, socket.IPPROTO_TCP,
+                          '', ('127.0.0.2', 5672))
+                  ]):
+            with pytest.raises(socket.error):
+                self.t.connect()
 
-    @patch('socket.socket', return_value=MockSocket())
-    @patch('socket.getaddrinfo',
-           side_effect=[
-               [(socket.AF_INET, 1, socket.IPPROTO_TCP,
-                 '', ('127.0.0.1', 5672))],
-               [(socket.AF_INET6, 1, socket.IPPROTO_TCP,
-                 '', ('::1', 5672))]
-           ])
-    def test_connect_short_curcuit_on_INET_succeed(self, getaddrinfo,
-                                                   sock_mock):
-        self.t.sock = Mock()
-        self.t.close()
-        self.t.connect()
-        getaddrinfo.assert_called_with(
-            'localhost', 5672, socket.AF_INET, ANY, ANY)
-
-    @patch('socket.socket', return_value=MockSocket())
-    @patch('socket.getaddrinfo',
-           side_effect=[
-               [(socket.AF_INET, 1, socket.IPPROTO_TCP,
-                 '', ('127.0.0.1', 5672))],
-               [(socket.AF_INET6, 1, socket.IPPROTO_TCP,
-                 '', ('::1', 5672))]
-           ])
-    def test_connect_short_curcuit_on_INET_fails(self, getaddrinfo, sock_mock):
-        self.t.sock = Mock()
-        self.t.close()
-        with patch.object(sock_mock.return_value, 'connect',
-                          side_effect=(socket.error, None)):
-            self.t.connect()
-        getaddrinfo.assert_has_calls(
-            [call('localhost', 5672, addr_type, ANY, ANY)
-             for addr_type in (socket.AF_INET, socket.AF_INET6)])
+    def test_connect_multiple_addr_entries_fails(self):
+        with patch('socket.socket', return_value=MockSocket()) as sock_mock, \
+            patch('socket.getaddrinfo',
+                  return_value=[
+                      (socket.AF_INET, 1, socket.IPPROTO_TCP,
+                          '', ('127.0.0.1', 5672)),
+                      (socket.AF_INET, 1, socket.IPPROTO_TCP,
+                          '', ('127.0.0.2', 5672))
+                  ]):
+            self.t.sock = Mock()
+            self.t.close()
+            with patch.object(sock_mock.return_value, 'connect',
+                              side_effect=socket.error):
+                with pytest.raises(socket.error):
+                    self.t.connect()
+
+    def test_connect_multiple_addr_entries_succeed(self):
+        with patch('socket.socket', return_value=MockSocket()) as sock_mock, \
+            patch('socket.getaddrinfo',
+                  return_value=[
+                      (socket.AF_INET, 1, socket.IPPROTO_TCP,
+                          '', ('127.0.0.1', 5672)),
+                      (socket.AF_INET, 1, socket.IPPROTO_TCP,
+                          '', ('127.0.0.2', 5672))
+                  ]):
+            self.t.sock = Mock()
+            self.t.close()
+            with patch.object(sock_mock.return_value, 'connect',
+                              side_effect=(socket.error, None)):
+                self.t.connect()
 
-    @patch('socket.getaddrinfo', side_effect=socket.gaierror)
-    def test_connect_getaddrinfo_raises_gaierror(self, getaddrinfo):
-        with pytest.raises(socket.error):
+    def test_connect_short_curcuit_on_INET_succeed(self):
+        with patch('socket.socket', return_value=MockSocket()), \
+            patch('socket.getaddrinfo',
+                  side_effect=[
+                      [(socket.AF_INET, 1, socket.IPPROTO_TCP,
+                          '', ('127.0.0.1', 5672))],
+                      [(socket.AF_INET6, 1, socket.IPPROTO_TCP,
+                          '', ('::1', 5672))]
+                  ]) as getaddrinfo:
+            self.t.sock = Mock()
+            self.t.close()
             self.t.connect()
+            getaddrinfo.assert_called_with(
+                'localhost', 5672, socket.AF_INET, ANY, ANY)
+
+    def test_connect_short_curcuit_on_INET_fails(self):
+        with patch('socket.socket', return_value=MockSocket()) as sock_mock, \
+            patch('socket.getaddrinfo',
+                  side_effect=[
+                      [(socket.AF_INET, 1, socket.IPPROTO_TCP,
+                          '', ('127.0.0.1', 5672))],
+                      [(socket.AF_INET6, 1, socket.IPPROTO_TCP,
+                          '', ('::1', 5672))]
+                  ]) as getaddrinfo:
+            self.t.sock = Mock()
+            self.t.close()
+            with patch.object(sock_mock.return_value, 'connect',
+                              side_effect=(socket.error, None)):
+                self.t.connect()
+            getaddrinfo.assert_has_calls(
+                [call('localhost', 5672, addr_type, ANY, ANY)
+                 for addr_type in (socket.AF_INET, socket.AF_INET6)])
 
-    @patch('socket.socket', return_value=MockSocket())
-    @patch('socket.getaddrinfo',
-           side_effect=[
-               socket.gaierror,
-               [(socket.AF_INET6, 1, socket.IPPROTO_TCP,
-                 '', ('::1', 5672))]
-           ])
-    def test_connect_getaddrinfo_raises_gaierror_once_recovers(self, *mocks):
-        self.t.connect()
+    def test_connect_getaddrinfo_raises_gaierror(self):
+        with patch('socket.getaddrinfo', side_effect=socket.gaierror):
+            with pytest.raises(socket.error):
+                self.t.connect()
 
-    @patch('socket.socket', return_value=MockSocket())
-    @patch('socket.getaddrinfo',
-           return_value=[(socket.AF_INET, 1, socket.IPPROTO_TCP,
-                          '', ('127.0.0.1', 5672))])
-    def test_connect_survives_not_implemented_set_cloexec(self, *mocks):
-        with patch('amqp.transport.set_cloexec',
-                   side_effect=NotImplementedError) as cloexec_mock:
+    def test_connect_getaddrinfo_raises_gaierror_once_recovers(self):
+        with patch('socket.socket', return_value=MockSocket()), \
+            patch('socket.getaddrinfo',
+                  side_effect=[
+                      socket.gaierror,
+                      [(socket.AF_INET6, 1, socket.IPPROTO_TCP,
+                          '', ('::1', 5672))]
+                  ]):
             self.t.connect()
-        assert cloexec_mock.called
+
+    def test_connect_survives_not_implemented_set_cloexec(self):
+        with patch('socket.socket', return_value=MockSocket()), \
+            patch('socket.getaddrinfo',
+                  return_value=[(socket.AF_INET, 1, socket.IPPROTO_TCP,
+                                 '', ('127.0.0.1', 5672))]):
+            with patch('amqp.transport.set_cloexec',
+                       side_effect=NotImplementedError) as cloexec_mock:
+                self.t.connect()
+            assert cloexec_mock.called
 
 
 class test_SSLTransport:
@@ -489,14 +487,15 @@ class test_SSLTransport:
         self.t._wrap_socket(sock, {'c': 2}, foo=1)
         self.t._wrap_context.assert_called_with(sock, {'foo': 1}, c=2)
 
-    @patch('ssl.create_default_context', create=True)
-    def test_wrap_context(self, create_default_context):
-        sock = Mock()
-        self.t._wrap_context(sock, {'f': 1}, check_hostname=True, bar=3)
-        create_default_context.assert_called_with(bar=3)
-        ctx = create_default_context()
-        assert ctx.check_hostname
-        ctx.wrap_socket.assert_called_with(sock, f=1)
+    def test_wrap_context(self):
+        with patch('ssl.create_default_context', create=True) \
+                as create_default_context:
+            sock = Mock()
+            self.t._wrap_context(sock, {'f': 1}, check_hostname=True, bar=3)
+            create_default_context.assert_called_with(bar=3)
+            ctx = create_default_context()
+            assert ctx.check_hostname
+            ctx.wrap_socket.assert_called_with(sock, f=1)
 
     def test_shutdown_transport(self):
         self.t.sock = None
diff --git a/t/unit/test_utils.py b/t/unit/test_utils.py
index af1d5b5..b429970 100644
--- a/t/unit/test_utils.py
+++ b/t/unit/test_utils.py
@@ -71,16 +71,16 @@ class test_NullHandler:
 
 class test_get_logger:
 
-    @patch('logging.getLogger')
-    def test_as_str(self, getLogger):
-        x = get_logger('foo.bar')
-        getLogger.assert_called_with('foo.bar')
-        assert x is getLogger()
-
-    @patch('amqp.utils.NullHandler')
-    def test_as_logger(self, _NullHandler):
-        m = Mock(name='logger')
-        m.handlers = None
-        x = get_logger(m)
-        assert x is m
-        x.addHandler.assert_called_with(_NullHandler())
+    def test_as_str(self):
+        with patch('logging.getLogger') as getLogger:
+            x = get_logger('foo.bar')
+            getLogger.assert_called_with('foo.bar')
+            assert x is getLogger()
+
+    def test_as_logger(self):
+        with patch('amqp.utils.NullHandler') as _NullHandler:
+            m = Mock(name='logger')
+            m.handlers = None
+            x = get_logger(m)
+            assert x is m
+            x.addHandler.assert_called_with(_NullHandler())