fix(tests): skip tokenizer tests for only gated model in fork prs #2268
Merged
lianakoleva merged 9 commits intoJun 23, 2026
Merged
Conversation
bhimrazy
requested review from
andyland,
k223kim,
lianakoleva and
t-vi
as code owners
June 16, 2026 17:30
AutoTokenizer.from_pretrained raises a bare OSError for gated repos, which _is_gated_hf_error doesn't recognise — causing the test to fail and retry 3×120s per model (6 min each) until the 35-min job times out. Switch to huggingface_hub.snapshot_download with allow_patterns limited to tokenizer/config files (no weights, no safetensors). It raises the typed GatedRepoError that the existing skip check already handles, so fork PRs without HF_TOKEN skip cleanly in seconds. Using the default HF cache (no local_dir) also means retries and CI caching reuse downloads.
pytest 9 removed the `path` argument from the pytest_collect_file hookspec. pytest-check-links 0.9.1 still declares the old signature, causing a PluginValidationError on startup. The uv migration (Lightning-AI#2257) un-pinned pytest, which let 9.x get resolved. Pin to <9 until pytest-check-links ships a compatible release.
bhimrazy
changed the title
bhimrazy
changed the title
Fork PRs have no HF_TOKEN, so 6 parallel CI jobs hit HF anonymously and get rate-limited (HTTP 429). The HfHubHTTPError wasn't caught, causing each rate-limited model to fail and retry 3x120s until the job timed out. Extend the skip check to include 429 and rename to _is_hf_skip_error to reflect that it now covers both gated repos and anonymous rate-limits.
bhimrazy
changed the title
snapshot_download returns a commit-hash path; litgpt's Tokenizer infers BOS from the directory name (SmolLM2-*-Instruct, Llama-3*, etc.) so symlink the cached files into tmp_path/<model-name> to preserve that. Also temporarily scope pytester to test_tokenizer.py for faster CI iteration while this is being validated.
CI sets a relative HF_HOME (.cache-HF), so snapshot_download returns a relative path; symlinking from the test's tmp_path produced dangling links and every tokenizer test failed. Copy the small tokenizer/config files into the model-named dir instead — robust regardless of HF_HOME.
Drop the check-links and cpu-tests.yml edits (check-links fix lives in a separate PR; restore the full CPU test suite) and trim the tokenizer test comments to the essentials.
bhimrazy
changed the title
Collaborator
Author
|
requires #2270 |
bhimrazy
changed the title
lianakoleva
approved these changes
Jun 23, 2026
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
2 participants
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.
What does this PR do?
This PR introduces a mechanism to skip the test_tokenizer_against_hf test for gated models in fork-based PRs. Instead of blocking the entire PR due to failures in this test, it allows the test to be skipped when running in a fork context.
This helps unblock unrelated PRs that are not dependent on gated models.
Note: This is likely to be addressed more permanently with Lightning Registry models. see #2262