From 8b81a9a310f5705b7107f65a7908f6d2335d13a2 Mon Sep 17 00:00:00 2001 From: tejasng053 Date: Sat, 31 Oct 2026 16:14:39 +0530 Subject: [PATCH] Make unit tests run offline and add HTTP request timeouts Fixes #188 Signed-off-by: tejasng053 --- CHANGELOG.rst | 4 + README.rst | 6 ++ src/fetchcode/__init__.py | 5 +- src/fetchcode/package_versions.py | 3 +- src/fetchcode/utils.py | 13 ++- tests/conftest.py | 15 ++++ tests/test_fetch.py | 5 +- tests/test_http_timeouts.py | 143 ++++++++++++++++++++++++++++++ tests/test_package.py | 10 +++ 9 files changed, 196 insertions(+), 8 deletions(-) create mode 100644 tests/conftest.py create mode 100644 tests/test_http_timeouts.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index e3df22f..e9a5ae5 100644 --- a/CHANGELOG.rst +++ b/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) diff --git a/README.rst b/README.rst index cd2139e..bb5e7fa 100644 --- a/README.rst +++ b/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 diff --git a/src/fetchcode/__init__.py b/src/fetchcode/__init__.py index d4403c5..45c05d8 100644 --- a/src/fetchcode/__init__.py +++ b/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}") diff --git a/src/fetchcode/package_versions.py b/src/fetchcode/package_versions.py index 5fa8116..df89d06 100644 --- a/src/fetchcode/package_versions.py +++ b/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}") diff --git a/src/fetchcode/utils.py b/src/fetchcode/utils.py index 13f1421..0e4bf97 100644 --- a/src/fetchcode/utils.py +++ b/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,7 +175,7 @@ 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() @@ -178,7 +183,7 @@ def get_response(url, headers=None): 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}") diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..37d8cce --- /dev/null +++ b/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) diff --git a/tests/test_fetch.py b/tests/test_fetch.py index 74300ba..bd24495 100644 --- a/tests/test_fetch.py +++ b/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") diff --git a/tests/test_http_timeouts.py b/tests/test_http_timeouts.py new file mode 100644 index 0000000..88c5430 --- /dev/null +++ b/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 diff --git a/tests/test_package.py b/tests/test_package.py index e1e22b5..b64539e 100644 --- a/tests/test_package.py +++ b/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" )