Make unit tests run offline and add HTTP request timeouts

CHANGELOG.rst

@@ -1,6 +1,10 @@
 Changelog
 =========
 
+Unreleased
+----------
+- Make unit tests independent of external HTTP services and add default HTTP timeouts (#188).
+
 v0.8.5
 --------
 - Update pypa gh-action-pypi-publish tag to fixed commit SHA (https://github.com/aboutcode-org/fetchcode/pull/221)

README.rst

@@ -38,10 +38,16 @@ To run test suite use::
 
     pytest -vvs
 
+Unit tests use recorded data and mocks and must run without an internet
+connection. Unexpected HTTP requests fail the test suite.
+
 
 Usage
 --------
 
+HTTP requests use a 30-second timeout for connection setup and read inactivity.
+This is not a limit on the total time needed to download a file.
+
 Fetch a code archive and get a ``fetchcode.fetch.Response`` object back::
 
     >>> from fetchcode import fetch

src/fetchcode/__init__.py

@@ -23,6 +23,7 @@
 import requests
 from packageurl.contrib import purl2url
 
+from fetchcode.utils import DEFAULT_TIMEOUT
 from fetchcode.utils import _http_exists
 
 
@@ -46,7 +47,7 @@ def fetch_http(url, location):
     Return a `Response` object built from fetching the content at a HTTP/HTTPS based
     `url` URL string saving the content in a file at `location`
     """
-    r = requests.get(url)
+    r = requests.get(url, timeout=DEFAULT_TIMEOUT)
 
     with open(location, "wb") as f:
         f.write(r.content)
@@ -162,7 +163,7 @@ def fetch_json_response(url):
     """
     Fetch a JSON response from the given URL and return the parsed JSON data.
     """
-    response = requests.get(url)
+    response = requests.get(url, timeout=DEFAULT_TIMEOUT)
     if response.status_code != 200:
         raise Exception(f"Failed to fetch {url}: {response.status_code} {response.reason}")
 

src/fetchcode/package_versions.py

@@ -30,6 +30,7 @@
 from packageurl.contrib.route import NoRouteAvailable
 from packageurl.contrib.route import Router
 
+from fetchcode.utils import DEFAULT_TIMEOUT
 from fetchcode.utils import fetch_github_tags_gql
 
 logger = logging.getLogger(__name__)
@@ -515,7 +516,7 @@ def get_response(url, content_type="json", headers=None):
     one of binary, text, yaml or json.
     """
     try:
-        resp = requests.get(url)
+        resp = requests.get(url, timeout=DEFAULT_TIMEOUT)
         resp.raise_for_status()
     except requests.HTTPError as http_err:
         logger.error(f"Error while fetching {url!r}: {resp.status_code!r}")

src/fetchcode/utils.py

@@ -23,6 +23,9 @@
 from dateutil import parser as dateparser
 from dateutil.parser import ParserError
 
+# Bound connection setup and read inactivity, not the total download duration.
+DEFAULT_TIMEOUT = 30
+
 
 def fetch_github_tags_gql(purl):
     """
@@ -145,7 +148,9 @@ def github_response(graphql_query):
     headers = {"Authorization": f"bearer {gh_token}"}
 
     endpoint = "https://api.github.com/graphql"
-    response = requests.post(endpoint, headers=headers, json=graphql_query).json()
+    response = requests.post(
+        endpoint, headers=headers, json=graphql_query, timeout=DEFAULT_TIMEOUT
+    ).json()
 
     message = response.get("message")
     if message and message == "Bad credentials":
@@ -170,15 +175,15 @@ def get_github_rest(url):
 
 
 def get_response(url, headers=None):
-    resp = requests.get(url, headers=headers)
+    resp = requests.get(url, headers=headers, timeout=DEFAULT_TIMEOUT)
     if resp.status_code == 200:
         return resp.json()
 
     raise Exception(f"Failed to fetch: {url}")
 
 
 def get_text_response(url, headers=None):
-    resp = requests.get(url, headers=headers)
+    resp = requests.get(url, headers=headers, timeout=DEFAULT_TIMEOUT)
     if resp.status_code == 200:
         return resp.text
 
@@ -187,7 +192,7 @@ def get_text_response(url, headers=None):
 
 def make_head_request(url, headers=None):
     try:
-        resp = requests.head(url, headers=headers)
+        resp = requests.head(url, headers=headers, timeout=DEFAULT_TIMEOUT)
         return resp
     except requests.RequestException:
         raise Exception(f"Failed to fetch: {url}")

tests/conftest.py

@@ -0,0 +1,15 @@
+# Copyright (c) nexB Inc. and others. All rights reserved.
+# SPDX-License-Identifier: Apache-2.0
+# See https://github.com/aboutcode-org/fetchcode for support or download.
+
+import pytest
+
+
+@pytest.fixture(autouse=True)
+def no_live_http_requests(monkeypatch):
+    """Require HTTP responses to be mocked so unit tests also run offline."""
+
+    def fail_on_request(self, method, url, **kwargs):
+        pytest.fail(f"Unexpected live HTTP request: {method} {url}")
+
+    monkeypatch.setattr("requests.sessions.Session.request", fail_on_request)

tests/test_fetch.py

@@ -143,9 +143,12 @@ def test_resolve_purl_invalid():
     assert resolve_purl("pkg:invalid/invalid-package@1.0.0") is None
 
 
-def test_resolve_purl_using_purl2url():
+@mock.patch("fetchcode._http_exists")
+def test_resolve_purl_using_purl2url(mock_http_exists):
+    mock_http_exists.return_value = True
     url = resolve_purl("pkg:pub/http@0.13.3")
     assert url == "https://pub.dev/api/archives/http-0.13.3.tar.gz"
+    mock_http_exists.assert_called_once_with("https://pub.dev/api/archives/http-0.13.3.tar.gz")
 
 
 @mock.patch("fetchcode._http_exists")

tests/test_http_timeouts.py

@@ -0,0 +1,143 @@
+# Copyright (c) nexB Inc. and others. All rights reserved.
+# SPDX-License-Identifier: Apache-2.0
+# See https://github.com/aboutcode-org/fetchcode for support or download.
+
+from unittest import mock
+
+import pytest
+import requests
+
+from fetchcode import fetch_http
+from fetchcode import fetch_json_response
+from fetchcode import package_versions
+from fetchcode import utils
+
+
+@pytest.fixture
+def http_response():
+    response = mock.Mock()
+    response.status_code = 200
+    response.json.return_value = {"name": "example"}
+    response.text = "name: example"
+    response.content = b"name: example"
+    response.headers = {"content-type": "text/plain", "content-length": "13"}
+    return response
+
+
+@pytest.mark.parametrize(
+    "helper, method, expected",
+    [
+        (utils.get_response, "get", {"name": "example"}),
+        (utils.get_text_response, "get", "name: example"),
+        (utils.make_head_request, "head", None),
+    ],
+)
+def test_http_helpers_use_timeout(helper, method, expected, http_response):
+    url = "https://example.com/metadata"
+    headers = {"Accept": "application/json"}
+    with mock.patch(f"requests.{method}", return_value=http_response) as request:
+        result = helper(url, headers=headers)
+
+    request.assert_called_once_with(url, headers=headers, timeout=30)
+    assert result == (http_response if method == "head" else expected)
+
+
+def test_github_graphql_uses_timeout(http_response):
+    query = {"query": "query { viewer { login } }"}
+    with (
+        mock.patch("fetchcode.utils.get_github_token", return_value="test-token"),
+        mock.patch("requests.post", return_value=http_response) as request,
+    ):
+        result = utils.github_response(query)
+
+    request.assert_called_once_with(
+        "https://api.github.com/graphql",
+        headers={"Authorization": "bearer test-token"},
+        json=query,
+        timeout=30,
+    )
+    assert result == {"name": "example"}
+
+
+def test_fetch_http_uses_timeout(tmp_path, http_response):
+    url = "https://example.com/archive"
+    location = tmp_path / "download"
+    with mock.patch("requests.get", return_value=http_response) as request:
+        result = fetch_http(url, location)
+
+    request.assert_called_once_with(url, timeout=30)
+    assert location.read_bytes() == http_response.content
+    assert result.url == url
+    assert result.size == 13
+    assert result.content_type == "text/plain"
+
+
+def test_fetch_json_response_uses_timeout(http_response):
+    url = "https://example.com/metadata.json"
+    with mock.patch("requests.get", return_value=http_response) as request:
+        result = fetch_json_response(url)
+
+    request.assert_called_once_with(url, timeout=30)
+    assert result == {"name": "example"}
+
+
+@pytest.mark.parametrize(
+    "content_type, expected",
+    [
+        ("json", {"name": "example"}),
+        ("yaml", {"name": "example"}),
+        ("text", "name: example"),
+        ("binary", b"name: example"),
+    ],
+)
+def test_package_versions_response_uses_timeout(content_type, expected, http_response):
+    url = "https://example.com/versions"
+    with mock.patch("requests.get", return_value=http_response) as request:
+        result = package_versions.get_response(url, content_type=content_type)
+
+    request.assert_called_once_with(url, timeout=30)
+    http_response.raise_for_status.assert_called_once_with()
+    assert result == expected
+
+
+@pytest.mark.parametrize("error_type", [requests.ConnectTimeout, requests.ReadTimeout])
+@pytest.mark.parametrize(
+    "helper",
+    [
+        utils.get_response,
+        utils.get_text_response,
+        fetch_json_response,
+        package_versions.get_response,
+    ],
+)
+def test_response_helpers_propagate_timeouts(helper, error_type):
+    with mock.patch("requests.get", side_effect=error_type("request timed out")):
+        with pytest.raises(error_type, match="request timed out"):
+            helper("https://example.com/metadata")
+
+
+def test_timed_out_download_does_not_overwrite_file(tmp_path):
+    location = tmp_path / "download"
+    location.write_bytes(b"existing content")
+    with mock.patch("requests.get", side_effect=requests.ReadTimeout("request timed out")):
+        with pytest.raises(requests.ReadTimeout, match="request timed out"):
+            fetch_http("https://example.com/archive", location)
+
+    assert location.read_bytes() == b"existing content"
+
+
+def test_timed_out_github_graphql_request_propagates():
+    with (
+        mock.patch("fetchcode.utils.get_github_token", return_value="test-token"),
+        mock.patch("requests.post", side_effect=requests.ReadTimeout("request timed out")),
+    ):
+        with pytest.raises(requests.ReadTimeout, match="request timed out"):
+            utils.github_response({"query": "query { viewer { login } }"})
+
+
+def test_timed_out_head_request_reports_failure():
+    url = "https://example.com/archive"
+    with mock.patch("requests.head", side_effect=requests.ConnectTimeout("request timed out")):
+        with pytest.raises(Exception, match=f"Failed to fetch: {url}"):
+            utils.make_head_request(url)
+        assert utils._http_exists(url) is False

tests/test_package.py

@@ -200,7 +200,9 @@ def test_tuby_package_with_invalid_url(mock_get):
 @mock.patch("fetchcode.package_util.utils.get_response")
 @mock.patch("fetchcode.package.get_cocoapod_tags")
 @mock.patch("fetchcode.package.get_hashed_path")
+@mock.patch("fetchcode.package.get_response")
 def test_cocoapods_packages(
+    mock_get_podspec,
     mock_get_hashed_path,
     mock_get_cocoapod_tags,
     mock_get_response,
@@ -221,6 +223,9 @@ def test_cocoapods_packages(
     mock_get_response.side_effect = file_json(
         "tests/data/cocoapods/mock_get_response_side_effect.json"
     )
+    mock_get_podspec.side_effect = file_json(
+        "tests/data/cocoapods/mock_get_response_side_effect.json"
+    )
     mock_get_github_rest.return_value = load_json(
         "tests/data/cocoapods/mock_get_github_rest_return_value.json"
     )
@@ -240,7 +245,9 @@ def test_cocoapods_packages(
 @mock.patch("fetchcode.package_util.utils.get_response")
 @mock.patch("fetchcode.package.get_cocoapod_tags")
 @mock.patch("fetchcode.package.get_hashed_path")
+@mock.patch("fetchcode.package.get_response")
 def test_get_cocoapods_data_from_purl(
+    mock_get_podspec,
     mock_get_hashed_path,
     mock_get_cocoapod_tags,
     mock_get_response,
@@ -266,6 +273,9 @@ def test_get_cocoapods_data_from_purl(
     mock_get_response.side_effect = file_json(
         "tests/data/cocoapods/mock_get_response_side_effect.json"
     )
+    mock_get_podspec.side_effect = file_json(
+        "tests/data/cocoapods/mock_get_response_side_effect.json"
+    )
     mock_get_github_rest.return_value = load_json(
         "tests/data/cocoapods/mock_get_github_rest_return_value.json"
     )