From 53cceb770bd2429126b6df9061479d8628d9165f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 7 Aug 2026 05:51:06 +0000 Subject: [PATCH 1/5] feat: forward HF_ENDPOINT env var to huggingface_hub calls in download_files_from_huggingface Co-authored-by: pangwangshu <1851324+pangwangshu@users.noreply.github.com> --- fastembed/common/model_management.py | 9 ++- tests/test_common.py | 89 ++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 2 deletions(-) diff --git a/fastembed/common/model_management.py b/fastembed/common/model_management.py index 35e682366..3bff18af8 100644 --- a/fastembed/common/model_management.py +++ b/fastembed/common/model_management.py @@ -214,6 +214,9 @@ def _save_file_metadata(model_dir: Path, meta: dict[str, dict[str, int | str]]) snapshot_dir = Path(cache_dir) / f"models--{hf_source_repo.replace('/', '--')}" metadata_file = snapshot_dir / cls.METADATA_FILE + hf_endpoint = os.environ.get("HF_ENDPOINT") + endpoint_kwargs: dict[str, Any] = {"endpoint": hf_endpoint} if hf_endpoint else {} + if local_files_only: disable_progress_bars() if metadata_file.exists(): @@ -228,12 +231,13 @@ def _save_file_metadata(model_dir: Path, meta: dict[str, dict[str, int | str]]) allow_patterns=allow_patterns, cache_dir=cache_dir, local_files_only=local_files_only, + **endpoint_kwargs, **kwargs, ) return result - repo_revision = model_info(hf_source_repo).sha - repo_tree = list(list_repo_tree(hf_source_repo, revision=repo_revision, repo_type="model")) + repo_revision = model_info(hf_source_repo, **endpoint_kwargs).sha + repo_tree = list(list_repo_tree(hf_source_repo, revision=repo_revision, repo_type="model", **endpoint_kwargs)) allowed_extensions = {".json", ".onnx", ".txt"} repo_files = ( @@ -260,6 +264,7 @@ def _save_file_metadata(model_dir: Path, meta: dict[str, dict[str, int | str]]) allow_patterns=allow_patterns, cache_dir=cache_dir, local_files_only=local_files_only, + **endpoint_kwargs, **kwargs, ) diff --git a/tests/test_common.py b/tests/test_common.py index 372c2e01f..a4a573ac8 100644 --- a/tests/test_common.py +++ b/tests/test_common.py @@ -1,3 +1,6 @@ +import os +from unittest.mock import patch, MagicMock + from fastembed import ( TextEmbedding, SparseTextEmbedding, @@ -5,6 +8,7 @@ LateInteractionMultimodalEmbedding, LateInteractionTextEmbedding, ) +from fastembed.common.model_management import ModelManagement def test_text_list_supported_models(): @@ -28,3 +32,88 @@ def test_text_list_supported_models(): assert "model_file" in description and description["model_file"] assert "sources" in description and description["sources"] assert "hf" in description["sources"] or "url" in description["sources"] + + +def test_hf_endpoint_forwarded_to_hub_calls(tmp_path): + """HF_ENDPOINT env var must be forwarded to model_info, list_repo_tree, and snapshot_download.""" + custom_endpoint = "https://hf-mirror.example.com" + + mock_model_info = MagicMock() + mock_model_info.sha = "abc123" + + mock_repo_file = MagicMock() + mock_repo_file.path = "model.onnx" + mock_repo_file.__class__ = __import__( + "huggingface_hub.hf_api", fromlist=["RepoFile"] + ).RepoFile + + with ( + patch.dict(os.environ, {"HF_ENDPOINT": custom_endpoint}), + patch("fastembed.common.model_management.model_info", return_value=mock_model_info) as mock_mi, + patch("fastembed.common.model_management.list_repo_tree", return_value=[]) as mock_lrt, + patch( + "fastembed.common.model_management.snapshot_download", + return_value=str(tmp_path), + ) as mock_sd, + ): + try: + ModelManagement.download_files_from_huggingface( + hf_source_repo="test-org/test-model", + cache_dir=str(tmp_path), + extra_patterns=["*.onnx"], + ) + except Exception: + pass # we only care that the calls received the right kwargs + + mock_mi.assert_called_once() + _, mi_kwargs = mock_mi.call_args + assert mi_kwargs.get("endpoint") == custom_endpoint, ( + f"model_info should receive endpoint={custom_endpoint!r}, got {mi_kwargs}" + ) + + mock_lrt.assert_called_once() + _, lrt_kwargs = mock_lrt.call_args + assert lrt_kwargs.get("endpoint") == custom_endpoint, ( + f"list_repo_tree should receive endpoint={custom_endpoint!r}, got {lrt_kwargs}" + ) + + mock_sd.assert_called_once() + _, sd_kwargs = mock_sd.call_args + assert sd_kwargs.get("endpoint") == custom_endpoint, ( + f"snapshot_download should receive endpoint={custom_endpoint!r}, got {sd_kwargs}" + ) + + +def test_no_hf_endpoint_no_extra_kwarg(tmp_path): + """When HF_ENDPOINT is not set, endpoint kwarg must NOT be passed to hub calls.""" + mock_model_info = MagicMock() + mock_model_info.sha = "abc123" + + env_without_hf_endpoint = {k: v for k, v in os.environ.items() if k != "HF_ENDPOINT"} + + with ( + patch.dict(os.environ, env_without_hf_endpoint, clear=True), + patch("fastembed.common.model_management.model_info", return_value=mock_model_info) as mock_mi, + patch("fastembed.common.model_management.list_repo_tree", return_value=[]) as mock_lrt, + patch( + "fastembed.common.model_management.snapshot_download", + return_value=str(tmp_path), + ) as mock_sd, + ): + try: + ModelManagement.download_files_from_huggingface( + hf_source_repo="test-org/test-model", + cache_dir=str(tmp_path), + extra_patterns=["*.onnx"], + ) + except Exception: + pass + + _, mi_kwargs = mock_mi.call_args + assert "endpoint" not in mi_kwargs, "endpoint kwarg should not be present when HF_ENDPOINT is unset" + + _, lrt_kwargs = mock_lrt.call_args + assert "endpoint" not in lrt_kwargs, "endpoint kwarg should not be present when HF_ENDPOINT is unset" + + _, sd_kwargs = mock_sd.call_args + assert "endpoint" not in sd_kwargs, "endpoint kwarg should not be present when HF_ENDPOINT is unset" From ebf8097d3e018eca0dcfaf1042477e57f260d049 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 7 Aug 2026 06:13:40 +0000 Subject: [PATCH 2/5] =?UTF-8?q?fix:=20clean=20up=20tests=20per=20ruff=20?= =?UTF-8?q?=E2=80=94=20remove=20bare=20except,=20fix=20import=20order?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: pangwangshu <1851324+pangwangshu@users.noreply.github.com> --- fastembed/common/model_management.py | 9 ++- tests/test_common.py | 108 +++++++++++---------------- 2 files changed, 47 insertions(+), 70 deletions(-) diff --git a/fastembed/common/model_management.py b/fastembed/common/model_management.py index 3bff18af8..48856df83 100644 --- a/fastembed/common/model_management.py +++ b/fastembed/common/model_management.py @@ -1,14 +1,14 @@ -import os -import time import json +import os import shutil import tarfile +import time from copy import deepcopy from pathlib import Path -from typing import Any, TypeVar, Generic +from typing import Any, Generic, TypeVar import requests -from huggingface_hub import snapshot_download, model_info, list_repo_tree +from huggingface_hub import list_repo_tree, model_info, snapshot_download from huggingface_hub.hf_api import RepoFile from huggingface_hub.utils import ( RepositoryNotFoundError, @@ -17,6 +17,7 @@ ) from loguru import logger from tqdm import tqdm + from fastembed.common.model_description import BaseModelDescription T = TypeVar("T", bound=BaseModelDescription) diff --git a/tests/test_common.py b/tests/test_common.py index a4a573ac8..415ca9cd7 100644 --- a/tests/test_common.py +++ b/tests/test_common.py @@ -1,12 +1,12 @@ import os -from unittest.mock import patch, MagicMock +from unittest.mock import MagicMock, patch from fastembed import ( - TextEmbedding, - SparseTextEmbedding, ImageEmbedding, LateInteractionMultimodalEmbedding, LateInteractionTextEmbedding, + SparseTextEmbedding, + TextEmbedding, ) from fastembed.common.model_management import ModelManagement @@ -34,86 +34,62 @@ def test_text_list_supported_models(): assert "hf" in description["sources"] or "url" in description["sources"] -def test_hf_endpoint_forwarded_to_hub_calls(tmp_path): - """HF_ENDPOINT env var must be forwarded to model_info, list_repo_tree, and snapshot_download.""" - custom_endpoint = "https://hf-mirror.example.com" - +def _run_download_with_mocks(tmp_path, extra_env): + """Helper: run download_files_from_huggingface with all network calls mocked out.""" mock_model_info = MagicMock() mock_model_info.sha = "abc123" - mock_repo_file = MagicMock() - mock_repo_file.path = "model.onnx" - mock_repo_file.__class__ = __import__( - "huggingface_hub.hf_api", fromlist=["RepoFile"] - ).RepoFile - with ( - patch.dict(os.environ, {"HF_ENDPOINT": custom_endpoint}), + patch.dict(os.environ, extra_env), patch("fastembed.common.model_management.model_info", return_value=mock_model_info) as mock_mi, patch("fastembed.common.model_management.list_repo_tree", return_value=[]) as mock_lrt, patch( "fastembed.common.model_management.snapshot_download", return_value=str(tmp_path), ) as mock_sd, + # skip post-download metadata verification so the function completes cleanly + patch.object(ModelManagement, "METADATA_FILE", "__nonexistent__"), ): - try: - ModelManagement.download_files_from_huggingface( - hf_source_repo="test-org/test-model", - cache_dir=str(tmp_path), - extra_patterns=["*.onnx"], - ) - except Exception: - pass # we only care that the calls received the right kwargs - - mock_mi.assert_called_once() - _, mi_kwargs = mock_mi.call_args - assert mi_kwargs.get("endpoint") == custom_endpoint, ( - f"model_info should receive endpoint={custom_endpoint!r}, got {mi_kwargs}" + ModelManagement.download_files_from_huggingface( + hf_source_repo="test-org/test-model", + cache_dir=str(tmp_path), + extra_patterns=["*.onnx"], ) + return mock_mi, mock_lrt, mock_sd - mock_lrt.assert_called_once() - _, lrt_kwargs = mock_lrt.call_args - assert lrt_kwargs.get("endpoint") == custom_endpoint, ( - f"list_repo_tree should receive endpoint={custom_endpoint!r}, got {lrt_kwargs}" - ) - mock_sd.assert_called_once() - _, sd_kwargs = mock_sd.call_args - assert sd_kwargs.get("endpoint") == custom_endpoint, ( - f"snapshot_download should receive endpoint={custom_endpoint!r}, got {sd_kwargs}" - ) +def test_hf_endpoint_forwarded_to_hub_calls(tmp_path): + """HF_ENDPOINT env var must be forwarded to model_info, list_repo_tree, and snapshot_download.""" + custom_endpoint = "https://hf-mirror.example.com" + mock_mi, mock_lrt, mock_sd = _run_download_with_mocks(tmp_path, {"HF_ENDPOINT": custom_endpoint}) + + _, mi_kwargs = mock_mi.call_args + assert mi_kwargs.get("endpoint") == custom_endpoint, ( + f"model_info should receive endpoint={custom_endpoint!r}, got {mi_kwargs}" + ) + + _, lrt_kwargs = mock_lrt.call_args + assert lrt_kwargs.get("endpoint") == custom_endpoint, ( + f"list_repo_tree should receive endpoint={custom_endpoint!r}, got {lrt_kwargs}" + ) + + _, sd_kwargs = mock_sd.call_args + assert sd_kwargs.get("endpoint") == custom_endpoint, ( + f"snapshot_download should receive endpoint={custom_endpoint!r}, got {sd_kwargs}" + ) def test_no_hf_endpoint_no_extra_kwarg(tmp_path): """When HF_ENDPOINT is not set, endpoint kwarg must NOT be passed to hub calls.""" - mock_model_info = MagicMock() - mock_model_info.sha = "abc123" - env_without_hf_endpoint = {k: v for k, v in os.environ.items() if k != "HF_ENDPOINT"} + with patch.dict(os.environ, env_without_hf_endpoint, clear=True): + mock_mi, mock_lrt, mock_sd = _run_download_with_mocks(tmp_path, {}) - with ( - patch.dict(os.environ, env_without_hf_endpoint, clear=True), - patch("fastembed.common.model_management.model_info", return_value=mock_model_info) as mock_mi, - patch("fastembed.common.model_management.list_repo_tree", return_value=[]) as mock_lrt, - patch( - "fastembed.common.model_management.snapshot_download", - return_value=str(tmp_path), - ) as mock_sd, - ): - try: - ModelManagement.download_files_from_huggingface( - hf_source_repo="test-org/test-model", - cache_dir=str(tmp_path), - extra_patterns=["*.onnx"], - ) - except Exception: - pass - - _, mi_kwargs = mock_mi.call_args - assert "endpoint" not in mi_kwargs, "endpoint kwarg should not be present when HF_ENDPOINT is unset" - - _, lrt_kwargs = mock_lrt.call_args - assert "endpoint" not in lrt_kwargs, "endpoint kwarg should not be present when HF_ENDPOINT is unset" - - _, sd_kwargs = mock_sd.call_args - assert "endpoint" not in sd_kwargs, "endpoint kwarg should not be present when HF_ENDPOINT is unset" + _, mi_kwargs = mock_mi.call_args + assert "endpoint" not in mi_kwargs, "endpoint kwarg should not be present when HF_ENDPOINT is unset" + + _, lrt_kwargs = mock_lrt.call_args + assert "endpoint" not in lrt_kwargs, "endpoint kwarg should not be present when HF_ENDPOINT is unset" + + _, sd_kwargs = mock_sd.call_args + assert "endpoint" not in sd_kwargs, "endpoint kwarg should not be present when HF_ENDPOINT is unset" From 1c05361dcdbb7752acd18ec73f9d5b971e99fcd0 Mon Sep 17 00:00:00 2001 From: Wangshu Pang Date: Thu, 6 Aug 2026 23:30:35 -0700 Subject: [PATCH 3/5] fix: forward HF_ENDPOINT through HfApi instance, not module-level functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit model_info and list_repo_tree, as imported from huggingface_hub, are bound methods of a module-level HfApi() singleton and don't accept an `endpoint` kwarg — only snapshot_download does. Passing `endpoint=` to them raised TypeError at runtime whenever HF_ENDPOINT was set. Construct an HfApi instance with the endpoint instead, and use autospec=True in the tests so mocks are checked against the real huggingface_hub signatures. Co-Authored-By: Claude Sonnet 5 --- fastembed/common/model_management.py | 14 ++++--- tests/test_common.py | 57 +++++++++++++++++----------- 2 files changed, 42 insertions(+), 29 deletions(-) diff --git a/fastembed/common/model_management.py b/fastembed/common/model_management.py index 48856df83..9459c3f20 100644 --- a/fastembed/common/model_management.py +++ b/fastembed/common/model_management.py @@ -8,7 +8,7 @@ from typing import Any, Generic, TypeVar import requests -from huggingface_hub import list_repo_tree, model_info, snapshot_download +from huggingface_hub import HfApi, snapshot_download from huggingface_hub.hf_api import RepoFile from huggingface_hub.utils import ( RepositoryNotFoundError, @@ -216,7 +216,7 @@ def _save_file_metadata(model_dir: Path, meta: dict[str, dict[str, int | str]]) metadata_file = snapshot_dir / cls.METADATA_FILE hf_endpoint = os.environ.get("HF_ENDPOINT") - endpoint_kwargs: dict[str, Any] = {"endpoint": hf_endpoint} if hf_endpoint else {} + hf_api = HfApi(endpoint=hf_endpoint) if local_files_only: disable_progress_bars() @@ -232,13 +232,15 @@ def _save_file_metadata(model_dir: Path, meta: dict[str, dict[str, int | str]]) allow_patterns=allow_patterns, cache_dir=cache_dir, local_files_only=local_files_only, - **endpoint_kwargs, + endpoint=hf_endpoint, **kwargs, ) return result - repo_revision = model_info(hf_source_repo, **endpoint_kwargs).sha - repo_tree = list(list_repo_tree(hf_source_repo, revision=repo_revision, repo_type="model", **endpoint_kwargs)) + repo_revision = hf_api.model_info(hf_source_repo).sha + repo_tree = list( + hf_api.list_repo_tree(hf_source_repo, revision=repo_revision, repo_type="model") + ) allowed_extensions = {".json", ".onnx", ".txt"} repo_files = ( @@ -265,7 +267,7 @@ def _save_file_metadata(model_dir: Path, meta: dict[str, dict[str, int | str]]) allow_patterns=allow_patterns, cache_dir=cache_dir, local_files_only=local_files_only, - **endpoint_kwargs, + endpoint=hf_endpoint, **kwargs, ) diff --git a/tests/test_common.py b/tests/test_common.py index 415ca9cd7..b2dcd0f97 100644 --- a/tests/test_common.py +++ b/tests/test_common.py @@ -35,16 +35,27 @@ def test_text_list_supported_models(): def _run_download_with_mocks(tmp_path, extra_env): - """Helper: run download_files_from_huggingface with all network calls mocked out.""" - mock_model_info = MagicMock() - mock_model_info.sha = "abc123" + """Run download_files_from_huggingface with all network calls mocked out. + + Uses autospec=True so mocks are checked against the real huggingface_hub + signatures - a kwarg that the real API doesn't accept (e.g. passing + `endpoint` to a bound HfApi method instead of its constructor) raises a + TypeError here, the same as it would against the real library. + """ + mock_hf_api_instance = MagicMock() + mock_hf_api_instance.model_info.return_value = MagicMock(sha="abc123") + mock_hf_api_instance.list_repo_tree.return_value = [] with ( patch.dict(os.environ, extra_env), - patch("fastembed.common.model_management.model_info", return_value=mock_model_info) as mock_mi, - patch("fastembed.common.model_management.list_repo_tree", return_value=[]) as mock_lrt, + patch( + "fastembed.common.model_management.HfApi", + autospec=True, + return_value=mock_hf_api_instance, + ) as mock_hf_api_cls, patch( "fastembed.common.model_management.snapshot_download", + autospec=True, return_value=str(tmp_path), ) as mock_sd, # skip post-download metadata verification so the function completes cleanly @@ -55,23 +66,22 @@ def _run_download_with_mocks(tmp_path, extra_env): cache_dir=str(tmp_path), extra_patterns=["*.onnx"], ) - return mock_mi, mock_lrt, mock_sd + return mock_hf_api_cls, mock_hf_api_instance, mock_sd def test_hf_endpoint_forwarded_to_hub_calls(tmp_path): - """HF_ENDPOINT env var must be forwarded to model_info, list_repo_tree, and snapshot_download.""" + """HF_ENDPOINT env var must be forwarded to HfApi and snapshot_download.""" custom_endpoint = "https://hf-mirror.example.com" - mock_mi, mock_lrt, mock_sd = _run_download_with_mocks(tmp_path, {"HF_ENDPOINT": custom_endpoint}) - - _, mi_kwargs = mock_mi.call_args - assert mi_kwargs.get("endpoint") == custom_endpoint, ( - f"model_info should receive endpoint={custom_endpoint!r}, got {mi_kwargs}" + mock_hf_api_cls, mock_hf_api_instance, mock_sd = _run_download_with_mocks( + tmp_path, {"HF_ENDPOINT": custom_endpoint} ) - _, lrt_kwargs = mock_lrt.call_args - assert lrt_kwargs.get("endpoint") == custom_endpoint, ( - f"list_repo_tree should receive endpoint={custom_endpoint!r}, got {lrt_kwargs}" + _, api_kwargs = mock_hf_api_cls.call_args + assert api_kwargs.get("endpoint") == custom_endpoint, ( + f"HfApi should be constructed with endpoint={custom_endpoint!r}, got {api_kwargs}" ) + mock_hf_api_instance.model_info.assert_called_once() + mock_hf_api_instance.list_repo_tree.assert_called_once() _, sd_kwargs = mock_sd.call_args assert sd_kwargs.get("endpoint") == custom_endpoint, ( @@ -80,16 +90,17 @@ def test_hf_endpoint_forwarded_to_hub_calls(tmp_path): def test_no_hf_endpoint_no_extra_kwarg(tmp_path): - """When HF_ENDPOINT is not set, endpoint kwarg must NOT be passed to hub calls.""" + """When HF_ENDPOINT is not set, endpoint must be None for HfApi and snapshot_download.""" env_without_hf_endpoint = {k: v for k, v in os.environ.items() if k != "HF_ENDPOINT"} with patch.dict(os.environ, env_without_hf_endpoint, clear=True): - mock_mi, mock_lrt, mock_sd = _run_download_with_mocks(tmp_path, {}) + mock_hf_api_cls, _, mock_sd = _run_download_with_mocks(tmp_path, {}) - _, mi_kwargs = mock_mi.call_args - assert "endpoint" not in mi_kwargs, "endpoint kwarg should not be present when HF_ENDPOINT is unset" - - _, lrt_kwargs = mock_lrt.call_args - assert "endpoint" not in lrt_kwargs, "endpoint kwarg should not be present when HF_ENDPOINT is unset" + _, api_kwargs = mock_hf_api_cls.call_args + assert api_kwargs.get("endpoint") is None, ( + f"HfApi endpoint should be None when HF_ENDPOINT is unset, got {api_kwargs}" + ) _, sd_kwargs = mock_sd.call_args - assert "endpoint" not in sd_kwargs, "endpoint kwarg should not be present when HF_ENDPOINT is unset" + assert sd_kwargs.get("endpoint") is None, ( + f"snapshot_download endpoint should be None when HF_ENDPOINT is unset, got {sd_kwargs}" + ) From b83174459669a1553e4415ee2ec0bcd4835d0d06 Mon Sep 17 00:00:00 2001 From: Wangshu Pang Date: Fri, 7 Aug 2026 01:11:14 -0700 Subject: [PATCH 4/5] fix: normalize empty HF_ENDPOINT and avoid duplicate endpoint kwarg Addresses CodeRabbit review on PR #667: - Treat an empty-string HF_ENDPOINT as unset instead of forwarding "" as the endpoint. - Pop endpoint out of kwargs before resolving hf_endpoint, so callers who pass endpoint= directly no longer collide with the explicit endpoint= kwarg passed to snapshot_download. - Stop overwriting the autospecced HfApi instance mock with a plain MagicMock, so method calls on it stay signature-checked. - Add test coverage for the above plus the local_files_only download path. Co-Authored-By: Claude Sonnet 5 --- fastembed/common/model_management.py | 2 +- tests/test_common.py | 71 +++++++++++++++++++++++----- 2 files changed, 61 insertions(+), 12 deletions(-) diff --git a/fastembed/common/model_management.py b/fastembed/common/model_management.py index 9459c3f20..96b9fb33f 100644 --- a/fastembed/common/model_management.py +++ b/fastembed/common/model_management.py @@ -215,7 +215,7 @@ def _save_file_metadata(model_dir: Path, meta: dict[str, dict[str, int | str]]) snapshot_dir = Path(cache_dir) / f"models--{hf_source_repo.replace('/', '--')}" metadata_file = snapshot_dir / cls.METADATA_FILE - hf_endpoint = os.environ.get("HF_ENDPOINT") + hf_endpoint = kwargs.pop("endpoint", None) or os.environ.get("HF_ENDPOINT") or None hf_api = HfApi(endpoint=hf_endpoint) if local_files_only: diff --git a/tests/test_common.py b/tests/test_common.py index b2dcd0f97..16a350791 100644 --- a/tests/test_common.py +++ b/tests/test_common.py @@ -34,25 +34,19 @@ def test_text_list_supported_models(): assert "hf" in description["sources"] or "url" in description["sources"] -def _run_download_with_mocks(tmp_path, extra_env): +def _run_download_with_mocks(tmp_path, extra_env, local_files_only=False, download_kwargs=None): """Run download_files_from_huggingface with all network calls mocked out. Uses autospec=True so mocks are checked against the real huggingface_hub signatures - a kwarg that the real API doesn't accept (e.g. passing `endpoint` to a bound HfApi method instead of its constructor) raises a - TypeError here, the same as it would against the real library. + TypeError here, the same as it would against the real library. The + instance returned by the autospecced HfApi mock (mock_hf_api_cls.return_value) + is kept as-is rather than replaced, so its methods stay spec-checked too. """ - mock_hf_api_instance = MagicMock() - mock_hf_api_instance.model_info.return_value = MagicMock(sha="abc123") - mock_hf_api_instance.list_repo_tree.return_value = [] - with ( patch.dict(os.environ, extra_env), - patch( - "fastembed.common.model_management.HfApi", - autospec=True, - return_value=mock_hf_api_instance, - ) as mock_hf_api_cls, + patch("fastembed.common.model_management.HfApi", autospec=True) as mock_hf_api_cls, patch( "fastembed.common.model_management.snapshot_download", autospec=True, @@ -61,10 +55,16 @@ def _run_download_with_mocks(tmp_path, extra_env): # skip post-download metadata verification so the function completes cleanly patch.object(ModelManagement, "METADATA_FILE", "__nonexistent__"), ): + mock_hf_api_instance = mock_hf_api_cls.return_value + mock_hf_api_instance.model_info.return_value = MagicMock(sha="abc123") + mock_hf_api_instance.list_repo_tree.return_value = [] + ModelManagement.download_files_from_huggingface( hf_source_repo="test-org/test-model", cache_dir=str(tmp_path), extra_patterns=["*.onnx"], + local_files_only=local_files_only, + **(download_kwargs or {}), ) return mock_hf_api_cls, mock_hf_api_instance, mock_sd @@ -104,3 +104,52 @@ def test_no_hf_endpoint_no_extra_kwarg(tmp_path): assert sd_kwargs.get("endpoint") is None, ( f"snapshot_download endpoint should be None when HF_ENDPOINT is unset, got {sd_kwargs}" ) + + +def test_empty_hf_endpoint_normalized_to_none(tmp_path): + """An empty-string HF_ENDPOINT must be treated as unset, not forwarded as ''.""" + mock_hf_api_cls, _, mock_sd = _run_download_with_mocks(tmp_path, {"HF_ENDPOINT": ""}) + + _, api_kwargs = mock_hf_api_cls.call_args + assert api_kwargs.get("endpoint") is None, ( + f"empty HF_ENDPOINT should normalize to None, got {api_kwargs}" + ) + + _, sd_kwargs = mock_sd.call_args + assert sd_kwargs.get("endpoint") is None, ( + f"empty HF_ENDPOINT should normalize to None, got {sd_kwargs}" + ) + + +def test_explicit_endpoint_kwarg_takes_precedence(tmp_path): + """An explicit `endpoint` kwarg must win over HF_ENDPOINT and not collide with it.""" + explicit_endpoint = "https://explicit.example.com" + mock_hf_api_cls, _, mock_sd = _run_download_with_mocks( + tmp_path, + {"HF_ENDPOINT": "https://from-env.example.com"}, + download_kwargs={"endpoint": explicit_endpoint}, + ) + + _, api_kwargs = mock_hf_api_cls.call_args + assert api_kwargs.get("endpoint") == explicit_endpoint, ( + f"explicit endpoint kwarg should take precedence, got {api_kwargs}" + ) + + _, sd_kwargs = mock_sd.call_args + assert sd_kwargs.get("endpoint") == explicit_endpoint, ( + f"explicit endpoint kwarg should take precedence, got {sd_kwargs}" + ) + + +def test_hf_endpoint_forwarded_on_local_files_only_path(tmp_path): + """HF_ENDPOINT must also be forwarded on the local_files_only snapshot_download call.""" + custom_endpoint = "https://hf-mirror.example.com" + _, _, mock_sd = _run_download_with_mocks( + tmp_path, {"HF_ENDPOINT": custom_endpoint}, local_files_only=True + ) + + _, sd_kwargs = mock_sd.call_args + assert sd_kwargs.get("endpoint") == custom_endpoint, ( + f"snapshot_download should receive endpoint={custom_endpoint!r} on the local_files_only path, " + f"got {sd_kwargs}" + ) From c9fb9e3ef8a4da5d84fbdc86e00d47465955c701 Mon Sep 17 00:00:00 2001 From: Wangshu Pang Date: Fri, 7 Aug 2026 01:17:07 -0700 Subject: [PATCH 5/5] fix: isolate test HF_ENDPOINT env var from the runner environment Addresses CodeRabbit review on PR #667. patch.dict(os.environ, extra_env) only adds/overrides keys, it never removes ones already present. If HF_ENDPOINT happens to be set in the runner's environment, tests calling _run_download_with_mocks with an extra_env that omits it would silently inherit that value instead of exercising the unset case. Co-Authored-By: Claude Sonnet 5 --- tests/test_common.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_common.py b/tests/test_common.py index 16a350791..550547a9c 100644 --- a/tests/test_common.py +++ b/tests/test_common.py @@ -55,6 +55,8 @@ def _run_download_with_mocks(tmp_path, extra_env, local_files_only=False, downlo # skip post-download metadata verification so the function completes cleanly patch.object(ModelManagement, "METADATA_FILE", "__nonexistent__"), ): + if "HF_ENDPOINT" not in extra_env: + os.environ.pop("HF_ENDPOINT", None) mock_hf_api_instance = mock_hf_api_cls.return_value mock_hf_api_instance.model_info.return_value = MagicMock(sha="abc123") mock_hf_api_instance.list_repo_tree.return_value = []