Skip to content

Stop testing for slow tokenizers as they will not exist soon#34235

Merged
DarkLight1337 merged 1 commit intovllm-project:mainfrom
hmellor:remove-invalid-test
Feb 10, 2026
Merged

Stop testing for slow tokenizers as they will not exist soon#34235
DarkLight1337 merged 1 commit intovllm-project:mainfrom
hmellor:remove-invalid-test

Conversation

@hmellor
Copy link
Member

@hmellor hmellor commented Feb 10, 2026

Slow tokenizders do not exist in Transformers v5. This PR pre-emptively removes the test that checks that slow tokenizers load slowly.

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request removes tests for slow tokenizers in anticipation of their deprecation. The change is straightforward, but it removes test coverage for code that still exists in the repository (e.g., handling for tokenizer_mode='slow'). This is risky. I've added a comment suggesting that the feature code and its tests should be removed together to avoid potential issues.

I am having trouble creating individual review comments. Click here to see my feedback.

tests/tokenizers_/test_basic.py (28-30)

high

While the goal of removing support for slow tokenizers is clear, this PR only removes the tests. The implementation logic for slow tokenizers (e.g., handling tokenizer_mode="slow" in vllm/tokenizers/registry.py) still exists in the codebase. Removing tests before the feature itself leaves this code untested, which can lead to regressions or dead code being left behind. It's recommended to remove the feature code and its tests in the same pull request.

@DarkLight1337 DarkLight1337 enabled auto-merge (squash) February 10, 2026 11:38
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Feb 10, 2026
@DarkLight1337 DarkLight1337 merged commit 6141397 into vllm-project:main Feb 10, 2026
15 checks passed
@hmellor hmellor deleted the remove-invalid-test branch February 10, 2026 12:30
llsj14 pushed a commit to llsj14/vllm that referenced this pull request Mar 1, 2026
…oject#34235)

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
tunglinwood pushed a commit to tunglinwood/vllm that referenced this pull request Mar 4, 2026
…oject#34235)

Signed-off-by: Harry Mellor <19981378+hmellor@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants