Skip to content

fix(tests): exclude blue-named .pth files from green skip-download property - #263

Open
MrAnkleManic wants to merge 1 commit into
nikopueringer:mainfrom
MrAnkleManic:fix/pbt-existing-checkpoint-blue-names
Open

fix(tests): exclude blue-named .pth files from green skip-download property#263
MrAnkleManic wants to merge 1 commit into
nikopueringer:mainfrom
MrAnkleManic:fix/pbt-existing-checkpoint-blue-names

Conversation

@MrAnkleManic

Copy link
Copy Markdown

Problem

tests/test_pbt_auto_download.py::TestExistingCheckpointSkipsDownload::test_existing_pth_skips_download fails once Hypothesis generates a .pth filename containing the blue token, e.g. blue.pth (after which the falsifying example is cached in .hypothesis/ and the failure becomes deterministic):

OSError: [Errno 9] Bad file descriptor
Falsifying example: test_existing_pth_skips_download(..., pth_name='blue.pth')

Root cause

Two layers:

  1. The property is stale since feat: dedicated blue-screen model (CorridorKeyBlue) with auto-detection #241. _discover_checkpoint now classifies any checkpoint whose basename contains blue (case-insensitive) as a blue checkpoint. The test calls _discover_checkpoint(TORCH_EXT) with the default screen_color="green", so a generated name like blue.pth is (correctly) filtered out of green discovery and the auto-download path runs — which the test forbids. The property "any existing .pth skips download" stopped being true when the filename convention landed.

  2. Why it surfaces as EBADF instead of a clean assertion: the test patches hf_hub_download with a bare mock.patch (no return_value), so the "downloaded path" is a MagicMock. That flows into shutil.copy2()open(src, 'rb'), and since MagicMock.__index__() returns 1, FileIO treats the mock as file descriptor 1 — opening pytest's captured stdout for reading and closing it on exit, which raises OSError: [Errno 9] Bad file descriptor and poisons subsequent examples in the run.

Fix (test-only)

  • Restrict Property 2's strategy to green-classified names by filtering out the blue token (imported as BLUE_FILENAME_TOKEN, matched case-insensitively exactly as the backend does).
  • Add a companion property: a blue-named .pth skips the download when screen_color="blue".
  • Give the must-not-download mocks (Properties 2 and 3) side_effect=AssertionError(...) so any future regression fails with a clear message instead of fd corruption.

Verification

All 5 tests in the file pass, including replaying the previously cached blue.pth falsifying example from .hypothesis/. ruff check and ruff format --check are clean. uv.lock untouched.

🤖 Generated with Claude Code

…operty

Since the CorridorKeyBlue filename convention (nikopueringer#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 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant