Fix version checker tests

src/tribler-core/tribler_core/modules/tests/test_versioncheck.py

@@ -9,27 +9,33 @@
 from tribler_core.modules.versioncheck_manager import VersionCheckManager
 from tribler_core.restapi.rest_endpoint import RESTResponse
 
+# Assuming this is always a newer version id
+NEW_VERSION_ID = 'v1337.0'
 
-@pytest.fixture
-async def version_check_manager(free_port, session):
-    versioncheck_manager.VERSION_CHECK_URL = 'http://localhost:%s' % free_port
+
+@pytest.fixture(name='version_check_manager')
+async def fixture_version_check_manager(free_port, session):
+    versioncheck_manager.VERSION_CHECK_URLS = [f"http://localhost:{free_port}"]
     version_check_manager = VersionCheckManager(session)
     yield version_check_manager
     await version_check_manager.stop()
 
 
 response = None
 response_code = 200
+response_lag = 0  # in seconds
 
 
-def handle_version_request(_):
-    global response, response_code
+async def handle_version_request(_):
+    global response, response_code, response_lag  # pylint: disable=global-statement
+    if response_lag > 0:
+        await sleep(response_lag)
     return RESTResponse(response, status=response_code)
 
 
-@pytest.fixture
-async def version_server(free_port):
-    global response_code
+@pytest.fixture(name='version_server')
+async def fixture_version_server(free_port):
+    global response_code  # pylint: disable=global-statement
     response_code = 200
     app = web.Application()
     app.add_routes([web.get('/{tail:.*}', handle_version_request)])
@@ -46,14 +52,14 @@ async def test_start(version_check_manager, version_server):
     """
     Test whether the periodic version lookup works as expected
     """
-    global response
+    global response  # pylint: disable=global-statement
     response = json.dumps({'name': 'v1.0'})
 
     version_check_manager.start()
     # We only start the version check if GIT is not in the version ID.
     assert not version_check_manager.is_pending_task_active("tribler version check")
 
-    import tribler_core.modules.versioncheck_manager as vcm
+    import tribler_core.modules.versioncheck_manager as vcm  # pylint: disable=reimported, import-outside-toplevel
     old_id = vcm.version_id
     vcm.version_id = "7.0.0"
     version_check_manager.start()
@@ -64,24 +70,23 @@ async def test_start(version_check_manager, version_server):
 
 @pytest.mark.asyncio
 async def test_old_version(version_check_manager, version_server):
-    global response
+    global response  # pylint: disable=global-statement
     response = json.dumps({'name': 'v1.0'})
     has_new_version = await version_check_manager.check_new_version()
     assert not has_new_version
 
 
-@pytest.mark.skip
 @pytest.mark.asyncio
 async def test_new_version(version_check_manager, version_server):
-    global response
-    response = json.dumps({'name': 'v1337.0'})
+    global response  # pylint: disable=global-statement
+    response = json.dumps({'name': NEW_VERSION_ID})
     has_new_version = await version_check_manager.check_new_version()
     assert has_new_version
 
 
 @pytest.mark.asyncio
 async def test_bad_request(version_check_manager, version_server):
-    global response, response_code
+    global response, response_code  # pylint: disable=global-statement
     response = json.dumps({'name': 'v1.0'})
     response_code = 500
     has_new_version = await version_check_manager.check_new_version()
@@ -90,48 +95,73 @@ async def test_bad_request(version_check_manager, version_server):
 
 @pytest.mark.asyncio
 async def test_connection_error(version_check_manager):
-    global response
+    global response  # pylint: disable=global-statement
     response = json.dumps({'name': 'v1.0'})
     versioncheck_manager.VERSION_CHECK_URLS = ["http://this.will.not.exist"]
     has_new_version = await version_check_manager.check_new_version()
     assert not has_new_version
 
 
-@pytest.mark.skip
 @pytest.mark.asyncio
 async def test_non_json_response(version_check_manager, version_server):
-    global response
+    global response  # pylint: disable=global-statement
     response = 'hello world - not json'
 
     versioncheck_manager.check_failed = False
     with pytest.raises(ValueError):
         await version_check_manager.check_new_version()
 
 
-@pytest.mark.skip
 @pytest.mark.asyncio
 async def test_version_check_timeout(version_check_manager, version_server):
-    #await setup_version_server(json.dumps({'name': 'v1337.0'}))
+    global response  # pylint: disable=global-statement
+    response = json.dumps({'name': NEW_VERSION_ID})
+
+    import tribler_core.modules.versioncheck_manager as vcm  # pylint: disable=reimported, import-outside-toplevel
+    old_timeout = vcm.VERSION_CHECK_TIMEOUT
+    vcm.VERSION_CHECK_TIMEOUT = 0.001
+
+    has_new_version = await version_check_manager.check_new_version()
+    assert not has_new_version
+
+    vcm.VERSION_CHECK_TIMEOUT = old_timeout
+
+
+@pytest.mark.asyncio
+async def test_version_check_api_timeout(free_port, version_check_manager, version_server):
+    global response, response_lag  # pylint: disable=global-statement
+    response = json.dumps({'name': NEW_VERSION_ID})
+    response_lag = 2  # Ensures that it takes 2 seconds to send a response
+
+    import tribler_core.modules.versioncheck_manager as vcm  # pylint: disable=reimported, import-outside-toplevel
+    old_timeout = vcm.VERSION_CHECK_TIMEOUT
+    vcm.VERSION_CHECK_TIMEOUT = 1  # version checker will wait for 1 second to get response
+
+    version_check_url = f"http://localhost:{free_port}"
+    # Since the time to respond is higher than the time version checker waits for response,
+    # it should cancel the request and return False
+    has_new_version = await version_check_manager.check_new_version_api(version_check_url)
+    assert not has_new_version
+
+    vcm.VERSION_CHECK_TIMEOUT = old_timeout
 
-    # Setting a timeout of 1ms, version checks should fail
-    versioncheck_manager.VERSION_CHECK_TIMEOUT = 0.001
-    version_check_manager.should_call_new_version_callback = False
-    await version_check_manager.check_version()
 
-@pytest.mark.skip
 @pytest.mark.asyncio
-async def test_fallback_on_multiple_urls(version_check_manager, version_server):
+async def test_fallback_on_multiple_urls(free_port, version_check_manager, version_server):
     """
     Scenario: Two release API URLs. First one is a non-existing URL so is expected to fail.
     The second one is of a local webserver (http://localhost:{port}) which is configured to
     return a new version available response. Here we test if the version checking still works
     if the first URL fails.
     """
-    versioncheck_manager.VERSION_CHECK_URLS = ["http://this.will.not.exist",
-                                               f"http://localhost:{version_check_manager.port}"]
+    global response  # pylint: disable=global-statement
+    response = json.dumps({'name': NEW_VERSION_ID})
 
-    # Local server which responds with a new version available on the API response
-    #await self.setup_version_server(json.dumps({'name': 'v1337.0'}))
+    import tribler_core.modules.versioncheck_manager as vcm  # pylint: disable=reimported, import-outside-toplevel
+    vcm_old_urls = vcm.VERSION_CHECK_URLS
+    vcm.VERSION_CHECK_URLS = ["http://this.will.not.exist", f"http://localhost:{free_port}"]
+
+    has_new_version = await version_check_manager.check_new_version()
+    assert has_new_version
 
-    #self.should_call_new_version_callback = True
-    #await self.check_version()
+    vcm.VERSION_CHECK_URLS = vcm_old_urls

src/tribler-core/tribler_core/modules/versioncheck_manager.py

@@ -2,7 +2,8 @@
 import logging
 from distutils.version import LooseVersion
 
-from aiohttp import ClientConnectionError, ClientResponseError, ClientSession, ContentTypeError, ServerConnectionError
+from aiohttp import ClientConnectionError, ClientResponseError, ClientSession, ClientTimeout, \
+    ContentTypeError, ServerConnectionError
 
 from ipv8.taskmanager import TaskManager
 
@@ -35,14 +36,15 @@ async def check_new_version(self):
         for version_check_url in VERSION_CHECK_URLS:
             try:
                 if await asyncio.wait_for(self.check_new_version_api(version_check_url), VERSION_CHECK_TIMEOUT):
-                    break
+                    return True
             except asyncio.TimeoutError:
                 self._logger.warning("Checking for new version failed for %s", version_check_url)
+        return False
 
     async def check_new_version_api(self, version_check_url):
         try:
             async with ClientSession(raise_for_status=True) as session:
-                response = await session.get(version_check_url)
+                response = await session.get(version_check_url, timeout=ClientTimeout(total=VERSION_CHECK_TIMEOUT))
                 response_dict = await response.json(content_type=None)
         except (ServerConnectionError, ClientConnectionError) as e:
             self._logger.error("Error when performing version check request: %s", e)
@@ -55,6 +57,7 @@ async def check_new_version_api(self, version_check_url):
             return False
         except asyncio.TimeoutError:
             self._logger.warning("Checking for new version failed for %s", version_check_url)
+            return False
 
         try:
             version = response_dict['name'][1:]
@@ -64,4 +67,3 @@ async def check_new_version_api(self, version_check_url):
             return False
         except ValueError as ve:
             raise ValueError("Failed to parse Tribler version response.\nError:%s" % ve)
-        return True