[Tests] Autorun async tests with pytest_twisted.

Since all of our async tests should be run with twisted, it's
annoying to have to decorate them with pytest_twisted.ensureDeferred.
Forgetting to do this will cause the test to pass without actually
running. This changes the behavior to detect all coroutine function
tests and mark them to be run by pytest_twisted the same way the
decorator does.

Closes: https://github.com/deluge-torrent/deluge/pull/414

deluge/conftest.py

@@ -3,7 +3,7 @@
 # the additional special exception to link portions of this program with the OpenSSL library.
 # See LICENSE for more details.
 #
-
+import asyncio
 import tempfile
 import warnings
 from unittest.mock import Mock, patch
@@ -192,3 +192,18 @@ def mock_mkstemp(tmp_path):
     tmp_file = tempfile.mkstemp(dir=tmp_path)
     with patch('tempfile.mkstemp', return_value=tmp_file):
         yield tmp_file
+
+
+def pytest_collection_modifyitems(session, config, items) -> None:
+    """
+    Automatically runs async tests with pytest_twisted.ensureDeferred
+    """
+    function_items = (item for item in items if isinstance(item, pytest.Function))
+    for function_item in function_items:
+        function = function_item.obj
+        if hasattr(function, '__func__'):
+            # methods need to be unwrapped.
+            function = function.__func__
+        if asyncio.iscoroutinefunction(function):
+            # This is how pytest_twisted marks ensureDeferred tests
+            setattr(function, '_pytest_twisted_mark', 'async_test')

deluge/tests/test_component.py

@@ -3,7 +3,6 @@
 # the additional special exception to link portions of this program with the OpenSSL library.
 # See LICENSE for more details.
 #
-import inspect
 import time
 from unittest.mock import Mock
 
@@ -32,15 +31,6 @@ class ComponentTesterDelayStart(ComponentTester):
         yield threads.deferToThread(time.sleep, 0.5)
 
 
-def pytest_twisted_ensuredeferred_for_class(cls):
-    """Applies ensureDeferred to all async test_ methods in class"""
-    for name, method in inspect.getmembers(cls, inspect.iscoroutinefunction):
-        if name.startswith('test'):
-            setattr(cls, name, pytest_twisted.ensureDeferred(method))
-    return cls
-
-
-@pytest_twisted_ensuredeferred_for_class
 @pytest.mark.usefixtures('component')
 class TestComponent:
     async def test_start_component(self):

deluge/tests/test_config.py

@@ -10,7 +10,6 @@ import os
 from codecs import getwriter
 
 import pytest
-import pytest_twisted
 from twisted.internet import task
 
 from deluge.common import JSON_FORMAT
@@ -84,7 +83,6 @@ class TestConfig:
 
         config._save_timer.cancel()
 
-    @pytest_twisted.ensureDeferred
     async def test_on_changed_callback(self, mock_callback):
         config = Config('test.conf', config_dir=self.config_dir)
         config.register_change_callback(mock_callback)
@@ -93,7 +91,6 @@ class TestConfig:
         await mock_callback.deferred
         mock_callback.assert_called_once_with('foo', 1)
 
-    @pytest_twisted.ensureDeferred
     async def test_key_function_callback(self, mock_callback):
         config = Config(
             'test.conf', defaults={'foo': 1, 'bar': 1}, config_dir=self.config_dir

deluge/tests/test_core.py

@@ -188,7 +188,6 @@ class TestCore(BaseTestCase):
         assert torrent_id == info_hash
         assert not os.path.isfile(mock_mkstemp[1])
 
-    @pytest_twisted.ensureDeferred
     async def test_add_torrent_url_with_cookie(self):
         url = 'http://localhost:%d/cookie' % self.listen_port
         options = {}
@@ -201,7 +200,6 @@ class TestCore(BaseTestCase):
         result = await self.core.add_torrent_url(url, options, headers)
         assert result == info_hash
 
-    @pytest_twisted.ensureDeferred
     async def test_add_torrent_url_with_redirect(self):
         url = 'http://localhost:%d/redirect' % self.listen_port
         options = {}
@@ -210,7 +208,6 @@ class TestCore(BaseTestCase):
         result = await self.core.add_torrent_url(url, options)
         assert result == info_hash
 
-    @pytest_twisted.ensureDeferred
     async def test_add_torrent_url_with_partial_download(self):
         url = 'http://localhost:%d/partial' % self.listen_port
         options = {}

deluge/tests/test_httpdownloader.py

@@ -176,18 +176,15 @@ class TestDownloadFile:
                 pytest.fail(ex)
         return filename
 
-    @pytest_twisted.ensureDeferred
     async def test_download(self):
         filename = await download_file(self.get_url(), fname('index.html'))
         assert filename == fname('index.html')
 
-    @pytest_twisted.ensureDeferred
     async def test_download_without_required_cookies(self):
         url = self.get_url('cookie')
         filename = await download_file(url, fname('none'))
         self.assert_contains(filename, 'Password cookie not set!')
 
-    @pytest_twisted.ensureDeferred
     async def test_download_with_required_cookies(self):
         url = self.get_url('cookie')
         cookie = {'cookie': 'password=deluge'}
@@ -195,14 +192,12 @@ class TestDownloadFile:
         assert filename == fname('monster')
         self.assert_contains(filename, 'COOKIE MONSTER!')
 
-    @pytest_twisted.ensureDeferred
     async def test_download_with_rename(self):
         url = self.get_url('rename?filename=renamed')
         filename = await download_file(url, fname('original'))
         assert filename == fname('renamed')
         self.assert_contains(filename, 'This file should be called renamed')
 
-    @pytest_twisted.ensureDeferred
     async def test_download_with_rename_exists(self):
         open(fname('renamed'), 'w').close()
         url = self.get_url('rename?filename=renamed')
@@ -210,34 +205,29 @@ class TestDownloadFile:
         assert filename == fname('renamed-1')
         self.assert_contains(filename, 'This file should be called renamed')
 
-    @pytest_twisted.ensureDeferred
     async def test_download_with_rename_sanitised(self):
         url = self.get_url('rename?filename=/etc/passwd')
         filename = await download_file(url, fname('original'))
         assert filename == fname('passwd')
         self.assert_contains(filename, 'This file should be called /etc/passwd')
 
-    @pytest_twisted.ensureDeferred
     async def test_download_with_attachment_no_filename(self):
         url = self.get_url('attachment')
         filename = await download_file(url, fname('original'))
         assert filename == fname('original')
         self.assert_contains(filename, 'Attachment with no filename set')
 
-    @pytest_twisted.ensureDeferred
     async def test_download_with_rename_prevented(self):
         url = self.get_url('rename?filename=spam')
         filename = await download_file(url, fname('forced'), force_filename=True)
         assert filename == fname('forced')
         self.assert_contains(filename, 'This file should be called spam')
 
-    @pytest_twisted.ensureDeferred
     async def test_download_with_gzip_encoding(self):
         url = self.get_url('gzip?msg=success')
         filename = await download_file(url, fname('gzip_encoded'))
         self.assert_contains(filename, 'success')
 
-    @pytest_twisted.ensureDeferred
     async def test_download_with_gzip_encoding_disabled(self):
         url = self.get_url('gzip?msg=unzip')
         filename = await download_file(
@@ -245,32 +235,27 @@ class TestDownloadFile:
         )
         self.assert_contains(filename, 'unzip')
 
-    @pytest_twisted.ensureDeferred
     async def test_page_redirect_unhandled(self):
         url = self.get_url('redirect')
         with pytest.raises(PageRedirect):
             await download_file(url, fname('none'), handle_redirects=False)
 
-    @pytest_twisted.ensureDeferred
     async def test_page_redirect(self):
         url = self.get_url('redirect')
         filename = await download_file(url, fname('none'), handle_redirects=True)
         assert filename == fname('none')
 
-    @pytest_twisted.ensureDeferred
     async def test_page_not_found(self):
         with pytest.raises(Error):
             await download_file(self.get_url('page/not/found'), fname('none'))
 
     @pytest.mark.xfail(reason="Doesn't seem like httpdownloader ever implemented this.")
-    @pytest_twisted.ensureDeferred
     async def test_page_not_modified(self):
         headers = {'If-Modified-Since': formatdate(usegmt=True)}
         with pytest.raises(Error) as exc_info:
             await download_file(self.get_url(), fname('index.html'), headers=headers)
         assert exc_info.value.status == NOT_MODIFIED
 
-    @pytest_twisted.ensureDeferred
     async def test_download_text_reencode_charset(self):
         """Re-encode as UTF-8 specified charset for text content-type header"""
         url = self.get_url('attachment')
@@ -280,7 +265,6 @@ class TestDownloadFile:
         assert filename == filepath
         self.assert_contains(filename, 'Attachment with no filename setбвгде')
 
-    @pytest_twisted.ensureDeferred
     async def test_download_binary_ignore_charset(self):
         """Ignore charset for binary content-type header e.g. torrent files"""
         url = self.get_url('torrent')

deluge/tests/test_json_api.py

@@ -30,7 +30,6 @@ common.disable_new_release_check()
 
 @pytest.mark.usefixtures('daemon', 'client', 'component')
 class TestJSON:
-    @pytest_twisted.ensureDeferred
     async def test_get_remote_methods(self):
         json = JSON()
         methods = await json.get_remote_methods()
@@ -151,7 +150,6 @@ class TestRPCRaiseDelugeErrorJSON:
     daemon.rpcserver.register_object(test)
 """
 
-    @pytest_twisted.ensureDeferred
     async def test_handle_request_method_raise_delugeerror(self):
         json = JSON()
 

deluge/tests/test_maybe_coroutine.py

@@ -139,7 +139,6 @@ def test_error_from_inline(function):
         inline_func_from_coro,
     ],
 )
-@pytest_twisted.ensureDeferred
 async def test_from_coro(function):
     """Test our coroutines wrapped with maybe_coroutine work from another coroutine."""
     result = await function()
@@ -156,14 +155,12 @@ async def test_from_coro(function):
         inline_error_from_coro,
     ],
 )
-@pytest_twisted.ensureDeferred
 async def test_error_from_coro(function):
     """Test our coroutines wrapped with maybe_coroutine work from another coroutine with errors."""
     with pytest.raises(Exception, match='function_error'):
         await function()
 
 
-@pytest_twisted.ensureDeferred
 async def test_tracebacks_preserved():
     with pytest.raises(Exception) as exc:
         await coro_error_from_coro()
@@ -178,13 +175,11 @@ async def test_tracebacks_preserved():
         assert expected in str(actual)
 
 
-@pytest_twisted.ensureDeferred
 async def test_maybe_deferred_coroutine():
     result = await maybeDeferred(coro_func)
     assert result == 'function_result'
 
 
-@pytest_twisted.ensureDeferred
 async def test_callback_before_await():
     def cb(res):
         assert res == 'function_result'
@@ -196,7 +191,6 @@ async def test_callback_before_await():
     assert result == 'function_result'
 
 
-@pytest_twisted.ensureDeferred
 async def test_callback_after_await():
     """If it has already been used as a coroutine, can't be retroactively turned into a Deferred.
     This limitation could be fixed, but the extra complication doesn't feel worth it.

deluge/tests/test_sessionproxy.py

@@ -5,7 +5,6 @@
 # the additional special exception to link portions of this program with the OpenSSL library.
 # See LICENSE for more details.
 #
-import pytest_twisted
 from twisted.internet.defer import maybeDeferred, succeed
 from twisted.internet.task import Clock
 
@@ -125,25 +124,21 @@ class TestSessionProxy(BaseTestCase):
     def test_startup(self):
         assert client.core.torrents['a'] == self.sp.torrents['a'][1]
 
-    @pytest_twisted.ensureDeferred
     async def test_get_torrent_status_no_change(self):
         result = await self.sp.get_torrent_status('a', [])
         assert result == client.core.torrents['a']
 
-    @pytest_twisted.ensureDeferred
     async def test_get_torrent_status_change_with_cache(self):
         client.core.torrents['a']['key1'] = 2
         result = await self.sp.get_torrent_status('a', ['key1'])
         assert result == {'key1': 1}
 
-    @pytest_twisted.ensureDeferred
     async def test_get_torrent_status_change_without_cache(self):
         client.core.torrents['a']['key1'] = 2
         self.clock.advance(self.sp.cache_time + 0.1)
         result = await self.sp.get_torrent_status('a', [])
         assert result == client.core.torrents['a']
 
-    @pytest_twisted.ensureDeferred
     async def test_get_torrent_status_key_not_updated(self):
         self.clock.advance(self.sp.cache_time + 0.1)
         self.sp.get_torrent_status('a', ['key1'])
@@ -151,7 +146,6 @@ class TestSessionProxy(BaseTestCase):
         result = await self.sp.get_torrent_status('a', ['key2'])
         assert result == {'key2': 99}
 
-    @pytest_twisted.ensureDeferred
     async def test_get_torrents_status_key_not_updated(self):
         self.clock.advance(self.sp.cache_time + 0.1)
         self.sp.get_torrents_status({'id': ['a']}, ['key1'])

deluge/tests/test_torrent.py

@@ -10,7 +10,6 @@ from base64 import b64encode
 from unittest import mock
 
 import pytest
-import pytest_twisted
 from twisted.internet import defer, reactor
 from twisted.internet.task import deferLater
 
@@ -85,7 +84,6 @@ class TestTorrent(BaseTestCase):
         }
         return atp
 
-    @pytest_twisted.ensureDeferred
     async def test_set_file_priorities(self):
         if getattr(lt, 'file_prio_alert', None):
             # Libtorrent 2.0.3 and later has a file_prio_alert

deluge/tests/test_torrentmanager.py

@@ -64,7 +64,6 @@ class TestTorrentmanager(BaseTestCase):
         torrent_id = yield self.core.add_torrent_magnet(magnet, options)
         assert self.tm.remove(torrent_id, False)
 
-    @pytest_twisted.ensureDeferred
     async def test_prefetch_metadata(self):
         from deluge._libtorrent import lt
 
@@ -119,7 +118,6 @@ class TestTorrentmanager(BaseTestCase):
         )
         assert expected == await d
 
-    @pytest_twisted.ensureDeferred
     async def test_prefetch_metadata_timeout(self):
         magnet = 'magnet:?xt=urn:btih:ab570cdd5a17ea1b61e970bb72047de141bce173'
         d = self.tm.prefetch_metadata(magnet, 30)

deluge/tests/test_tracker_icons.py

@@ -6,7 +6,6 @@
 import os.path
 
 import pytest
-import pytest_twisted
 
 import deluge.component as component
 import deluge.ui.tracker_icons
@@ -28,7 +27,6 @@ class TestTrackerIcons(BaseTestCase):
     def tear_down(self):
         return component.shutdown()
 
-    @pytest_twisted.ensureDeferred
     async def test_get_deluge_png(self, mock_mkstemp):
         # Deluge has a png favicon link
         icon = TrackerIcon(common.get_test_data_file('deluge.png'))
@@ -36,7 +34,6 @@ class TestTrackerIcons(BaseTestCase):
         assert result == icon
         assert not os.path.isfile(mock_mkstemp[1])
 
-    @pytest_twisted.ensureDeferred
     async def test_get_google_ico(self):
         # Google doesn't have any icon links
         # So instead we'll grab its favicon.ico
@@ -44,14 +41,12 @@ class TestTrackerIcons(BaseTestCase):
         result = await self.icons.fetch('www.google.com')
         assert result == icon
 
-    @pytest_twisted.ensureDeferred
     async def test_get_google_ico_hebrew(self):
         """Test that Google.co.il page is read as UTF-8"""
         icon = TrackerIcon(common.get_test_data_file('google.ico'))
         result = await self.icons.fetch('www.google.co.il')
         assert result == icon
 
-    @pytest_twisted.ensureDeferred
     async def test_get_google_ico_with_redirect(self):
         # google.com redirects to www.google.com
         icon = TrackerIcon(common.get_test_data_file('google.ico'))
@@ -59,19 +54,16 @@ class TestTrackerIcons(BaseTestCase):
         assert result == icon
 
     @pytest.mark.skip(reason='Site removed favicon, new SNI test will be needed')
-    @pytest_twisted.ensureDeferred
     async def test_get_seo_svg_with_sni(self):
         # seo using certificates with SNI support only
         icon = TrackerIcon(common.get_test_data_file('seo.svg'))
         result = await self.icons.fetch('www.seo.com')
         assert result == icon
 
-    @pytest_twisted.ensureDeferred
     async def test_get_empty_string_tracker(self):
         result = await self.icons.fetch('')
         assert result is None
 
-    @pytest_twisted.ensureDeferred
     async def test_invalid_host(self, mock_mkstemp):
         """Test that TrackerIcon can handle invalid hostname"""
         result = await self.icons.fetch('deluge.example.com')

deluge/tests/test_ui_entry.py

@@ -386,7 +386,6 @@ class ConsoleUIWithDaemonBaseTestCase(UIWithDaemonBaseTestCase):
             f'move_completed_path: {tmp_path}\nmove_completed: True\n'
         )
 
-    @pytest_twisted.ensureDeferred
     async def test_console_command_status(self):
         fd = StringFileDescriptor(sys.stdout)
         self.patch_arg_command(['status'])

deluge/tests/test_web_api.py

@@ -26,7 +26,6 @@ common.disable_new_release_check()
 
 class TestWebAPI(WebServerTestBase):
     @pytest.mark.xfail(reason='This just logs an error at the moment.')
-    @pytest_twisted.ensureDeferred
     async def test_connect_invalid_host(self):
         with pytest.raises(Exception):
             await self.deluge_web.web_api.connect('id')

deluge/tests/test_webserver.py

@@ -10,7 +10,6 @@ import json as json_lib
 from io import BytesIO
 
 import pytest
-import pytest_twisted
 import twisted.web.client
 from twisted.internet import reactor
 from twisted.web.client import Agent, FileBodyProducer
@@ -24,7 +23,6 @@ common.disable_new_release_check()
 
 
 class TestWebServer(WebServerTestBase, WebServerMockBase):
-    @pytest_twisted.ensureDeferred
     async def test_get_torrent_info(self):
         agent = Agent(reactor)
 
@@ -60,7 +58,6 @@ class TestWebServer(WebServerTestBase, WebServerMockBase):
         assert 'torrent_filehash' == json['result']['name']
 
     @pytest.mark.parametrize('base', ['', '/', 'deluge'])
-    @pytest_twisted.ensureDeferred
     async def test_base_with_config(self, base):
         agent = Agent(reactor)
         root_url = f'http://127.0.0.1:{self.deluge_web.port}'
@@ -77,7 +74,6 @@ class TestWebServer(WebServerTestBase, WebServerMockBase):
         assert response.code == 200
 
     @pytest.mark.parametrize('base', ['/', 'deluge'])
-    @pytest_twisted.ensureDeferred
     async def test_base_with_config_recurring_basepath(self, base):
         agent = Agent(reactor)
         base_url = f'http://127.0.0.1:{self.deluge_web.port}/{base}'
@@ -95,7 +91,6 @@ class TestWebServer(WebServerTestBase, WebServerMockBase):
         response = await agent.request(b'GET', recursive_url.encode())
         assert response.code == 404 if base.strip('/') else 200
 
-    @pytest_twisted.ensureDeferred
     async def test_base_with_deluge_header(self):
         """Ensure base path is set and HTML contains path"""
         agent = Agent(reactor)