fix(tests): Mock HF in integration tests - #259
Merged
matthewgrossman merged 8 commits intoJun 15, 2026
Merged
Conversation
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
The conftest.py patches hf_hub_download and AutoConfig.from_pretrained to serve from local fixtures, preventing HuggingFace API calls. The download_fixtures.py script regenerates fixtures when new models are added to tests. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Contributor
|
The 5K lines of HF config fixtures were unnecessary — the parallelism tests are always skipped in CI (torch not installed). Stripped down to only the change that fixes the actual CI failures: mocking HfApi in TestTrustRemoteCodePermission. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
…ts-hit-hugging-face-rate-limits Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Replace the broad HfApi class mock with the no_hf_network fixture (defined in conftest.py) which patches only validate_storage and resolve_config on HuggingfaceStorageImpl. This is the same pattern used by the files service tests and avoids breaking downstream calls like list_files that also use HfApi. All 6 TestTrustRemoteCodePermission tests pass locally. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
matthewgrossman
marked this pull request as ready for review
June 15, 2026 18:46
Add a safety net: if validate_storage or resolve_config patches ever stop working, HfApi.repo_info will raise RuntimeError instead of silently making real HuggingFace API calls. Verified locally: all 6 TestTrustRemoteCodePermission tests pass with the poison active, proving no real HF egress occurs. Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesHuggingFace Network Isolation
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
mckornfield
reviewed
Jun 15, 2026
Signed-off-by: Matthew Grossman <mgrossman@nvidia.com>
matthewgrossman
enabled auto-merge
June 15, 2026 19:55
matthewgrossman
deleted the
mgrossman/aircore-744-python-integration-tests-hit-hugging-face-rate-limits
branch
June 15, 2026 20:06
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
TestTrustRemoteCodePermissionintegration tests create HuggingFace-backed filesets (storage.type=huggingface), which triggers real HF Hub API calls (repo_info,get_hf_file_metadata) duringvalidate_storage()andresolve_config(). These calls are rate-limited by HuggingFace, causing all 6 tests to fail with:Setting
HF_TOKENin CI (PR #202) reduced the frequency but didn't eliminate the issue — rate limits still apply to authenticated users, and concurrent CI runs sharing the same token compound the problem.Strategy
Mock at the Files service layer, not in individual tests. These tests verify authorization behavior (who can set
trust_remote_code), not HuggingFace connectivity. The real HF API calls are an unnecessary side effect of fileset creation.A
no_hf_networkfixture (defined inconftest.py) patchesHuggingfaceStorageImpl.validate_storageandresolve_configto no-ops. This is the same pattern already used by the Files service auth tests intests_filesets_with_auth_secrets.py. It's better than mockingHfApidirectly because it only disables the two methods that egress to HF, leaving everything else (likelist_files) intact.TestTrustRemoteCodePermissionuses this fixture via an autouse wrapper so every test in the class gets the mock automatically.What about other tests?
test_huggingface_endpoints.py) — already safe, use ASGI transport (no real network)test_huggingface_storage.py) — gated behindRUN_EXTERNAL_STORAGE_TESTS=1, skipped by defaulttests_filesets_with_auth_secrets.py) — already useno_hf_networkpytest.importorskip("torch"), skipped in CI (torch not installed)The only unprotected tests hitting real HF in CI were the 6
TestTrustRemoteCodePermissiontests fixed here.Test plan
TestTrustRemoteCodePermissiontests pass locally (verified withHF_HUB_OFFLINE=1)Fixes AIRCORE-744
Summary by CodeRabbit
Release Notes