Fix lint blocker, false-green tests and offline defaults for PR #7482
Drop the two unused _env_says_offline imports that fail the Source lint import-hoist check. test_has_tokenizer_model_offline_skips_model_info and its local_files_only twin set model_info.side_effect = AssertionError, but _has_tokenizer_model wraps that call in "except Exception: return False", so the AssertionError was swallowed and both passed on the merge base with the fix absent. Assert model_info.call_count == 0 instead; both now fail on the base with assert 1 == 0. The real-cache integration tests called hf_hub_download and PreTrainedTokenizerFast directly, so they exercised plain huggingface_hub and passed identically on both trees. Route them through the resolver this PR adds, and gate the file at module level since importing unsloth needs a GPU host either way. _resolve_hub_repo_local_dir and _resolve_hub_repo_cached_file defaulted to local_files_only = False, so a helper named "resolve local dir" would download with backoff retries when called without the flag. Every caller already passes it explicitly, so default it closed. Use tempfile.gettempdir() rather than a hardcoded /tmp, which silently skipped both files on Windows, the platform in the bug report. Patch socket.socket connect rather than replacing the class, which broke isinstance checks. Wire the unit tests into the Bucket-A CI list; Repo tests (CPU) ignores tests/saving, so none of these ran anywhere.
This commit is contained in:
parent
b41bbbef7d
commit
5f854f2b50
6 changed files with 70 additions and 42 deletions
6
.github/workflows/consolidated-tests-ci.yml
vendored
6
.github/workflows/consolidated-tests-ci.yml
vendored
|
|
@ -7,7 +7,7 @@
|
|||
#
|
||||
# Why a separate workflow:
|
||||
# - studio-backend-ci.yml's "Repo tests (CPU)" job already auto-discovers
|
||||
# tests/ minus tests/qlora, tests/saving, tests/utils, tests/sh. The 16
|
||||
# tests/ minus tests/qlora, tests/saving, tests/utils, tests/sh. The 17
|
||||
# Bucket-A tests below live inside those --ignore dirs (CPU-runnable but
|
||||
# historically excluded with their GPU siblings); pulling them out into
|
||||
# a sibling job keeps the existing 760-passed baseline stable while we
|
||||
|
|
@ -274,6 +274,7 @@ jobs:
|
|||
tests/saving/test_export_dispatch.py \
|
||||
tests/saving/test_imatrix_export.py \
|
||||
tests/saving/test_gguf_single_pass_export.py \
|
||||
tests/saving/test_offline_gguf_vlm_tokenizer_7481.py \
|
||||
tests/utils/test_attention_masks.py \
|
||||
tests/utils/test_trunc_normal_patch.py \
|
||||
tests/python/test_fast_language_model_text_only.py
|
||||
|
|
@ -365,6 +366,7 @@ jobs:
|
|||
tests/saving/test_export_dispatch.py \
|
||||
tests/saving/test_imatrix_export.py \
|
||||
tests/saving/test_gguf_single_pass_export.py \
|
||||
tests/saving/test_offline_gguf_vlm_tokenizer_7481.py \
|
||||
tests/utils/test_attention_masks.py \
|
||||
tests/utils/test_trunc_normal_patch.py \
|
||||
tests/python/test_fast_language_model_text_only.py \
|
||||
|
|
@ -2129,7 +2131,7 @@ jobs:
|
|||
pip show unsloth_zoo
|
||||
echo "::endgroup::"
|
||||
echo "Consolidated job done. Coverage:"
|
||||
echo " - 16 unsloth Bucket-A tests under tests/saving/ + tests/utils/"
|
||||
echo " - 17 unsloth Bucket-A tests under tests/saving/ + tests/utils/"
|
||||
echo " - unsloth_zoo @ ${UNSLOTH_ZOO_REF} pytest tests/ (5 GPU cases deselected)"
|
||||
echo " - unsloth_zoo.compiler.test_apply_fused_lm_head"
|
||||
|
||||
|
|
|
|||
|
|
@ -1,4 +1,7 @@
|
|||
#!/usr/bin/env python3
|
||||
# SPDX-License-Identifier: AGPL-3.0-only
|
||||
# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved.
|
||||
|
||||
"""Download real Gemma weights and run offline integration tests for #7481.
|
||||
|
||||
Example:
|
||||
|
|
@ -11,10 +14,13 @@ from __future__ import annotations
|
|||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
REPO = "unsloth/gemma-3-270m-it-bnb-4bit"
|
||||
CACHE_ROOT = Path(os.environ.get("HF_HOME", "/tmp/hf_offline_test_cache"))
|
||||
CACHE_ROOT = Path(
|
||||
os.environ.get("HF_HOME") or os.path.join(tempfile.gettempdir(), "hf_offline_test_cache")
|
||||
)
|
||||
|
||||
|
||||
def download():
|
||||
|
|
|
|||
|
|
@ -1,22 +1,43 @@
|
|||
# SPDX-License-Identifier: AGPL-3.0-only
|
||||
# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved.
|
||||
|
||||
"""Integration tests for #7481 using real cached Gemma weights.
|
||||
|
||||
Requires a one-time online download:
|
||||
HF_HOME=/tmp/hf_offline_test_cache python -c \\
|
||||
"from huggingface_hub import snapshot_download; snapshot_download('unsloth/gemma-3-270m-it-bnb-4bit', cache_dir='/tmp/hf_offline_test_cache/hub')"
|
||||
Requires a one-time online download into ``$HF_HOME`` (defaults to a
|
||||
``hf_offline_test_cache`` directory under the platform temp dir):
|
||||
|
||||
No GPU. No full unsloth import (CPU-only hosts cannot import the package graph).
|
||||
HF_HOME=<cache> python -c \\
|
||||
"from huggingface_hub import snapshot_download; snapshot_download('unsloth/gemma-3-270m-it-bnb-4bit', cache_dir='<cache>/hub')"
|
||||
|
||||
Every test here drives unsloth's own resolver. Resolving through
|
||||
``hf_hub_download`` directly would pass with the fix reverted, since that is
|
||||
plain huggingface_hub behaviour rather than anything this change touches.
|
||||
|
||||
Importing unsloth pulls the whole package graph, which CPU-only hosts cannot
|
||||
do, so the suite is gated behind ``UNSLOTH_INTEGRATION_IMPORT=1``.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import socket
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
REPO = "unsloth/gemma-3-270m-it-bnb-4bit"
|
||||
CACHE_ROOT = Path(os.environ.get("HF_HOME", "/tmp/hf_offline_test_cache"))
|
||||
CACHE_ROOT = Path(
|
||||
os.environ.get("HF_HOME") or os.path.join(tempfile.gettempdir(), "hf_offline_test_cache")
|
||||
)
|
||||
|
||||
pytestmark = [
|
||||
pytest.mark.integration,
|
||||
pytest.mark.skipif(
|
||||
os.environ.get("UNSLOTH_INTEGRATION_IMPORT") != "1",
|
||||
reason = "full unsloth import needs a GPU host; set UNSLOTH_INTEGRATION_IMPORT=1 to enable",
|
||||
),
|
||||
]
|
||||
|
||||
|
||||
def _require_cached_repo():
|
||||
|
|
@ -34,7 +55,9 @@ def _block_network(monkeypatch):
|
|||
def _guard(*args, **kwargs):
|
||||
raise OSError("network blocked for offline integration test")
|
||||
|
||||
monkeypatch.setattr(socket, "socket", _guard)
|
||||
# Patch the method, not the class: replacing socket.socket itself breaks any
|
||||
# isinstance(x, socket.socket) in the stack under test.
|
||||
monkeypatch.setattr(socket.socket, "connect", _guard)
|
||||
monkeypatch.setattr(socket, "create_connection", _guard)
|
||||
monkeypatch.setattr(socket, "getaddrinfo", _guard)
|
||||
|
||||
|
|
@ -45,51 +68,43 @@ def _offline_env(monkeypatch):
|
|||
monkeypatch.setenv("HF_HOME", str(CACHE_ROOT))
|
||||
|
||||
|
||||
def _resolve_snapshot(cache_dir: Path) -> Path:
|
||||
from huggingface_hub import hf_hub_download
|
||||
path = hf_hub_download(
|
||||
REPO,
|
||||
"tokenizer_config.json",
|
||||
cache_dir = str(cache_dir),
|
||||
local_files_only = True,
|
||||
)
|
||||
return Path(path).parent
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
def test_real_cached_snapshot_resolves_offline(monkeypatch):
|
||||
_require_cached_repo()
|
||||
_offline_env(monkeypatch)
|
||||
_block_network(monkeypatch)
|
||||
|
||||
snap = _resolve_snapshot(CACHE_ROOT / "hub")
|
||||
from unsloth.models.loader_utils import _resolve_hub_repo_local_dir
|
||||
|
||||
snap = Path(
|
||||
_resolve_hub_repo_local_dir(
|
||||
REPO,
|
||||
cache_dir = str(CACHE_ROOT / "hub"),
|
||||
local_files_only = True,
|
||||
)
|
||||
)
|
||||
assert (snap / "tokenizer.json").is_file()
|
||||
assert (snap / "tokenizer.model").is_file()
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
def test_real_cached_tokenizer_loads_from_snapshot_not_repo_id(monkeypatch):
|
||||
"""Mirrors the #7481 fix: load from snapshot dir, not Hub repo id."""
|
||||
"""The #7481 fix: the loader hands transformers a snapshot dir, not a repo id."""
|
||||
_require_cached_repo()
|
||||
_offline_env(monkeypatch)
|
||||
_block_network(monkeypatch)
|
||||
|
||||
from transformers import PreTrainedTokenizerFast
|
||||
from unsloth.models.loader_utils import _load_pretrained_tokenizer_fast
|
||||
|
||||
snap = _resolve_snapshot(CACHE_ROOT / "hub")
|
||||
tok = PreTrainedTokenizerFast.from_pretrained(str(snap), local_files_only = True)
|
||||
tok = _load_pretrained_tokenizer_fast(
|
||||
REPO,
|
||||
local_files_only = True,
|
||||
cache_dir = str(CACHE_ROOT / "hub"),
|
||||
)
|
||||
assert tok.vocab_size > 0
|
||||
|
||||
# Repo-id path is what triggered model_info() offline in #7481; snapshot path is the fix.
|
||||
assert str(snap) != REPO
|
||||
assert "/" not in Path(str(snap)).name
|
||||
# A repo id here means the Hub metadata probe was reached, which is the bug.
|
||||
assert tok.name_or_path != REPO
|
||||
assert Path(tok.name_or_path).is_dir()
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
@pytest.mark.skipif(
|
||||
os.environ.get("UNSLOTH_INTEGRATION_IMPORT") != "1",
|
||||
reason = "full unsloth import needs GPU host; set UNSLOTH_INTEGRATION_IMPORT=1 to enable",
|
||||
)
|
||||
def test_real_cached_unsloth_helpers_offline(monkeypatch):
|
||||
_require_cached_repo()
|
||||
_offline_env(monkeypatch)
|
||||
|
|
|
|||
|
|
@ -1,3 +1,6 @@
|
|||
# SPDX-License-Identifier: AGPL-3.0-only
|
||||
# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved.
|
||||
|
||||
"""Offline GGUF export must not probe the Hub for VLM tokenizer metadata (issue #7481).
|
||||
|
||||
Regression for ``PreTrainedTokenizerFast.from_pretrained`` on a repo id calling
|
||||
|
|
@ -177,9 +180,11 @@ def test_has_tokenizer_model_offline_skips_model_info(tmp_path, monkeypatch):
|
|||
|
||||
tok = SimpleNamespace(name_or_path = _REPO)
|
||||
|
||||
# A raising side_effect proves nothing: _has_tokenizer_model wraps the call
|
||||
# in `except Exception: return False`, so it passes with the fix reverted.
|
||||
with patch("huggingface_hub.HfApi.model_info") as model_info:
|
||||
model_info.side_effect = AssertionError("model_info must not run offline")
|
||||
assert _has_tokenizer_model(tok, token = None) is False
|
||||
assert model_info.call_count == 0
|
||||
|
||||
|
||||
def test_has_tokenizer_model_probes_cache_before_model_info(tmp_path, monkeypatch):
|
||||
|
|
@ -214,5 +219,5 @@ def test_has_tokenizer_model_local_files_only_skips_model_info(tmp_path, monkeyp
|
|||
)
|
||||
|
||||
with patch("huggingface_hub.HfApi.model_info") as model_info:
|
||||
model_info.side_effect = AssertionError("model_info must not run with local_files_only")
|
||||
assert _has_tokenizer_model(tok, token = None) is False
|
||||
assert model_info.call_count == 0
|
||||
|
|
|
|||
|
|
@ -1139,7 +1139,9 @@ def _resolve_hub_repo_local_dir(
|
|||
*,
|
||||
token = None,
|
||||
cache_dir = None,
|
||||
local_files_only = False,
|
||||
# Default closed: a "resolve local dir" helper must not download. False here
|
||||
# means five filenames each retried with backoff before it gives up.
|
||||
local_files_only = True,
|
||||
filenames = (
|
||||
"tokenizer_config.json",
|
||||
"config.json",
|
||||
|
|
@ -1184,7 +1186,7 @@ def _resolve_hub_repo_cached_file(
|
|||
*,
|
||||
token = None,
|
||||
cache_dir = None,
|
||||
local_files_only = False,
|
||||
local_files_only = True,
|
||||
):
|
||||
"""Return a cached file path under a Hub snapshot, or None if absent."""
|
||||
local_dir = _resolve_hub_repo_local_dir(
|
||||
|
|
|
|||
|
|
@ -54,7 +54,6 @@ import re
|
|||
from transformers.models.llama.modeling_llama import logger
|
||||
from .models.loader_utils import (
|
||||
get_model_name,
|
||||
_env_says_offline,
|
||||
_resolve_hub_repo_cached_file,
|
||||
_tokenizer_wants_local_only,
|
||||
)
|
||||
|
|
@ -3839,7 +3838,6 @@ def unsloth_convert_lora_to_ggml_and_save_locally(
|
|||
|
||||
from .models.loader_utils import (
|
||||
get_model_name,
|
||||
_env_says_offline,
|
||||
_resolve_hub_repo_cached_file,
|
||||
_tokenizer_wants_local_only,
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue