Skip e2e/integration tests by default and gracefully skip gated tokenizer test#4757
Skip e2e/integration tests by default and gracefully skip gated tokenizer test#4757
Conversation
…izer test - Add `not integration` to pytest default `addopts` so the integration test runner that requires an Iris cluster is deselected when running plain `uv run pytest`. - Mark `test_integration_test_run` with `@pytest.mark.integration` so it is picked up by the default deselection (in addition to the existing CI skip). - Always skip the marin tokenizer test when the gated llama3 tokenizer cannot be downloaded (instead of only on CI), so contributors without HF access do not see a hard failure. Fixes #4741. Co-authored-by: Russell Power <rjpower@users.noreply.github.com>
|
Claude finished @rjpower's task in 1m 31s —— View job Code reviewNo issues found. Checked for bugs and AGENTS.md compliance. Notes from review:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3971d5cc47
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| except Exception as e: | ||
| if os.getenv("CI", False) in ["true", "1"]: | ||
| pytest.skip("Llama 3 tokenizer repository is gated") | ||
| raise e | ||
| pytest.skip(f"Llama 3 tokenizer is unavailable (gated repo or no network): {e}") |
There was a problem hiding this comment.
Catching Exception here causes the test to be skipped for any failure in load_llama3_tokenizer(), including real regressions (for example, a bad model ID, local API misuse, or transformer-side breakage) rather than only missing credentials/network. That turns genuine failures into skips and can let tokenizer changes merge without coverage; this should only skip for the specific access-related exceptions documented by load_llama3_tokenizer.
Useful? React with 👍 / 👎.
2 participants
Fixes #4741.
Summary
not integrationto pytest defaultaddoptsso a plainuv run pytestdeselects the integration runner.test_integration_test_runwith@pytest.mark.integration.test_marin_tokenizer.pywhen the gated llama3 tokenizer cannot be downloaded, not just on CI.