Skip to content

Commit cb49561

Browse files
committed
Say out loud that these cells need a case sensitive filesystem
The Hub branch of check_model_quantization_status is only reachable when os.path.exists(name) misses while check_local_model_exists(name) hits, and a case difference is the only thing that produces that: case insensitive matching is the entire purpose of that helper, so there is no filesystem independent way to write the shape. `Outputs/MyModel` against an on-disk `outputs/mymodel` is it. On macOS APFS and Windows NTFS os.path.exists answers True for `Outputs/MyModel`, so the local branch is taken and the Hub round trip these five cells are named for never happens. They would then pass while exercising nothing, which is the same kind of quiet false confidence as a green check on tests CI never executed. So probe the filesystem and skip with the reason, rather than leaving the assumption implicit. On a case sensitive filesystem nothing changes: 28 passed, no skips. Checked the guard both ways by substituting a case insensitive os.path.exists, so it is not decorative. Found by running the branch on macos-14 and windows-latest, which nothing in CI does today.
1 parent d117647 commit cb49561

1 file changed

Lines changed: 30 additions & 0 deletions

File tree

tests/test_quant_status_transport_errors.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@
4141
the two Hub entry points, so nothing touches the network.
4242
"""
4343

44+
import os
45+
4446
import huggingface_hub
4547
import pytest
4648
import requests
@@ -328,6 +330,29 @@ def test_an_absent_local_config_still_reports_unquantized(tmp_path):
328330
_ON_DISK = ("outputs", "mymodel")
329331

330332

333+
def _require_a_case_sensitive_filesystem(tmp_path):
334+
"""State the precondition the shape above depends on, rather than assume it.
335+
336+
A case difference is the only thing that makes `os.path.exists(name)` miss
337+
while `check_local_model_exists(name)` hits, because case insensitive matching
338+
is the whole purpose of that helper. So there is no filesystem independent way
339+
to write this shape, and on a case insensitive filesystem (macOS APFS, Windows
340+
NTFS) `os.path.exists("Outputs/MyModel")` answers True, the local branch is
341+
taken, and the Hub round trip these tests are named for never happens.
342+
343+
Left implicit, that turns them into tests that pass while exercising nothing.
344+
Skipping says so out loud instead.
345+
"""
346+
probe = tmp_path / "case_probe"
347+
probe.mkdir(exist_ok = True)
348+
if os.path.exists(str(tmp_path / "CASE_PROBE")):
349+
pytest.skip(
350+
"case insensitive filesystem: os.path.exists would resolve "
351+
f"{_REQUESTED!r} locally and skip the Hub branch under test"
352+
)
353+
pass
354+
355+
331356
@pytest.mark.parametrize("save_method", ["merged_4bit", "forced_merged_4bit"])
332357
def test_local_4bit_still_resolves_when_only_the_config_fetch_fails(
333358
monkeypatch, tmp_path, save_method,
@@ -339,6 +364,7 @@ def test_local_4bit_still_resolves_when_only_the_config_fetch_fails(
339364
tmp_path.joinpath(*_ON_DISK),
340365
quant_config = {"load_in_4bit": True, "bnb_4bit_quant_type": "nf4"},
341366
)
367+
_require_a_case_sensitive_filesystem(tmp_path)
342368
monkeypatch.chdir(tmp_path)
343369
_patch_ls_present(monkeypatch)
344370
_patch_config_fetch(monkeypatch, _RateLimited())
@@ -357,6 +383,7 @@ def test_local_4bit_plus_16bit_merge_still_raises(monkeypatch, tmp_path):
357383
tmp_path.joinpath(*_ON_DISK),
358384
quant_config = {"load_in_4bit": True, "bnb_4bit_quant_type": "nf4"},
359385
)
386+
_require_a_case_sensitive_filesystem(tmp_path)
360387
monkeypatch.chdir(tmp_path)
361388
_patch_ls_present(monkeypatch)
362389
_patch_config_fetch(monkeypatch, _RateLimited())
@@ -372,6 +399,7 @@ def test_local_fp8_resolves_for_a_16bit_merge(monkeypatch, tmp_path):
372399
tmp_path.joinpath(*_ON_DISK),
373400
quant_config = {"quant_method": "fp8"},
374401
)
402+
_require_a_case_sensitive_filesystem(tmp_path)
375403
monkeypatch.chdir(tmp_path)
376404
_patch_ls_present(monkeypatch)
377405
_patch_config_fetch(monkeypatch, _RateLimited())
@@ -385,6 +413,7 @@ def test_local_fp8_resolves_for_a_16bit_merge(monkeypatch, tmp_path):
385413
def test_no_local_copy_propagates(monkeypatch, tmp_path):
386414
"""Nothing local can answer, so the outage is the whole story and must be
387415
what the caller sees, rather than a `(None, ...)` that writes no files."""
416+
_require_a_case_sensitive_filesystem(tmp_path)
388417
monkeypatch.chdir(tmp_path)
389418
_patch_ls_present(monkeypatch)
390419
_patch_config_fetch(monkeypatch, _RateLimited())
@@ -403,6 +432,7 @@ def test_an_exactly_named_local_directory_never_reaches_the_config_fetch(
403432
tmp_path.joinpath(*_ON_DISK),
404433
quant_config = {"load_in_4bit": True, "bnb_4bit_quant_type": "nf4"},
405434
)
435+
_require_a_case_sensitive_filesystem(tmp_path)
406436
monkeypatch.chdir(tmp_path)
407437
_patch_ls_present(monkeypatch)
408438

0 commit comments

Comments
 (0)