From 0f6ec20d774e5e8b2b93ba5aa2e41132282e9c86 Mon Sep 17 00:00:00 2001 From: MrAnkleManic Date: Tue, 7 Jul 2026 18:04:28 +0100 Subject: [PATCH] fix(tests): exclude blue-named .pth files from green skip-download property Since the CorridorKeyBlue filename convention (#241), _discover_checkpoint classifies any checkpoint whose basename contains 'blue' (case-insensitive) as a blue checkpoint. Property 2's strategy could generate such names (e.g. 'blue.pth'), so green discovery correctly found no match and entered the auto-download path the test forbids. The failure surfaced as OSError: [Errno 9] Bad file descriptor rather than an assertion: the bare hf_hub_download mock returned a MagicMock, which shutil.copy2 -> open() treated as file descriptor 1 (MagicMock.__index__() returns 1), reading from and then closing pytest's captured stdout. - Restrict Property 2's green strategy to names without the blue token, mirroring the backend's case-insensitive check. - Add a companion property: blue-named .pth files skip the download when screen_color='blue'. - Give the must-not-download mocks (Properties 2 and 3) a loud AssertionError side effect so any regression fails with a clear message instead of fd corruption. Co-Authored-By: Claude Fable 5 --- tests/test_pbt_auto_download.py | 64 +++++++++++++++++++++++++-------- 1 file changed, 49 insertions(+), 15 deletions(-) diff --git a/tests/test_pbt_auto_download.py b/tests/test_pbt_auto_download.py index e2e081ff2..1481b826c 100644 --- a/tests/test_pbt_auto_download.py +++ b/tests/test_pbt_auto_download.py @@ -18,6 +18,7 @@ from hypothesis import strategies as st from CorridorKeyModule.backend import ( + BLUE_FILENAME_TOKEN, HF_CHECKPOINT_FILENAME_SAFETENSORS, HF_REPO_ID, TORCH_EXT, @@ -128,12 +129,26 @@ def test_missing_pth_triggers_download_and_returns_valid_path( # Strategies for Property 2 # --------------------------------------------------------------------------- -# Strategy: valid .pth filenames (alphanumeric + underscore/dash, non-empty) -_pth_basenames = st.text( +# Strategy: valid .pth filenames (alphanumeric + underscore/dash, non-empty). +# Names containing the (case-insensitive) blue token are excluded: the backend +# classifies them as blue checkpoints, so they do NOT satisfy green discovery +# and would legitimately trigger the auto-download this property forbids. +_green_pth_basenames = ( + st.text( + alphabet=st.characters(whitelist_categories=("L", "N"), whitelist_characters="_-"), + min_size=1, + max_size=20, + ) + .map(lambda s: f"{s}.pth") + .filter(lambda name: BLUE_FILENAME_TOKEN not in name.lower()) +) + +# Strategy: .pth filenames carrying the blue token, as the backend detects it +_blue_pth_basenames = st.text( alphabet=st.characters(whitelist_categories=("L", "N"), whitelist_characters="_-"), - min_size=1, - max_size=20, -).map(lambda s: f"{s}.pth") + min_size=0, + max_size=14, +).map(lambda s: f"{BLUE_FILENAME_TOKEN}{s}.pth") # --------------------------------------------------------------------------- @@ -143,21 +158,17 @@ def test_missing_pth_triggers_download_and_returns_valid_path( class TestExistingCheckpointSkipsDownload: """Property 2: For any checkpoint directory that already contains a .pth - file, calling _discover_checkpoint(TORCH_EXT) returns the existing file's - path without invoking hf_hub_download. + file matching the requested screen color (filenames containing 'blue', + case-insensitively, are blue; everything else is green), calling + _discover_checkpoint(TORCH_EXT) returns the existing file's path without + invoking hf_hub_download. Feature: auto-model-download, Property 2: Existing checkpoint skips download **Validates: Requirements 1.3** """ - @settings(max_examples=100) - @given(pth_name=_pth_basenames) - def test_existing_pth_skips_download(self, pth_name: str) -> None: - """Feature: auto-model-download, Property 2: Existing checkpoint skips download - - **Validates: Requirements 1.3** - """ + def _assert_skips_download(self, pth_name: str, screen_color: str) -> None: with tempfile.TemporaryDirectory() as tmp: ckpt_dir = Path(tmp) / "checkpoints" ckpt_dir.mkdir() @@ -170,9 +181,13 @@ def test_existing_pth_skips_download(self, pth_name: str) -> None: mock.patch("CorridorKeyModule.backend.CHECKPOINT_DIR", str(ckpt_dir)), mock.patch( "huggingface_hub.hf_hub_download", + # Fail loudly if called: a bare MagicMock return value + # would be fed to shutil.copy2, which opens it as fd 1 + # (MagicMock.__index__() == 1) and closes stdout. + side_effect=AssertionError("hf_hub_download must not be called"), ) as mock_dl, ): - result = _discover_checkpoint(TORCH_EXT) + result = _discover_checkpoint(TORCH_EXT, screen_color=screen_color) # hf_hub_download must NOT have been called mock_dl.assert_not_called() @@ -180,6 +195,24 @@ def test_existing_pth_skips_download(self, pth_name: str) -> None: # The returned path must match the existing file assert result == existing_file, f"Expected {existing_file}, got {result}" + @settings(max_examples=100) + @given(pth_name=_green_pth_basenames) + def test_existing_pth_skips_download(self, pth_name: str) -> None: + """Feature: auto-model-download, Property 2: Existing checkpoint skips download + + **Validates: Requirements 1.3** + """ + self._assert_skips_download(pth_name, "green") + + @settings(max_examples=100) + @given(pth_name=_blue_pth_basenames) + def test_existing_blue_pth_skips_download(self, pth_name: str) -> None: + """Feature: auto-model-download, Property 2: Existing checkpoint skips download + + **Validates: Requirements 1.3** + """ + self._assert_skips_download(pth_name, "blue") + # --------------------------------------------------------------------------- # Property 3: Auto-download is Torch-only @@ -211,6 +244,7 @@ def test_non_pth_extension_raises_without_download(self, ext: str) -> None: mock.patch("CorridorKeyModule.backend.CHECKPOINT_DIR", str(ckpt_dir)), mock.patch( "huggingface_hub.hf_hub_download", + side_effect=AssertionError("hf_hub_download must not be called"), ) as mock_dl, ): with pytest.raises(FileNotFoundError):