Skip to content

[CI] Resettle pooling entrypoints tests. #29370

Merged
DarkLight1337 merged 2 commits intovllm-project:mainfrom
noooop:resettle_pooling_tests
Nov 25, 2025
Merged

[CI] Resettle pooling entrypoints tests. #29370
DarkLight1337 merged 2 commits intovllm-project:mainfrom
noooop:resettle_pooling_tests

Conversation

@noooop
Copy link
Collaborator

@noooop noooop commented Nov 25, 2025

Purpose

Resettle pooling entrypoints tests by use cases.

cc @DarkLight1337

Test Plan

tests/entrypoints/pooling/

Test Result

pass


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.
  • (Optional) Release notes update. If your change is user facing, please update the release notes draft in the Google Doc.

Signed-off-by: wang.yuqi <yuqi.wang@daocloud.io>
@noooop noooop requested a review from DarkLight1337 November 25, 2025 04:23
@DarkLight1337
Copy link
Member

I think we should still separate online and offline tests, because the fixtures they use are different.

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 is a pure refactoring of the pooling entrypoint tests. The tests have been reorganized into subdirectories based on their use case, such as basic, classify, embed, and score. This change significantly improves the structure and maintainability of the test suite, making it easier to navigate and locate specific tests. The new organization is logical and well-structured. As the PR only contains file moves and renames with no code modifications, there are no code-level issues to address. The refactoring is a positive change for the project.

@DarkLight1337
Copy link
Member

DarkLight1337 commented Nov 25, 2025

Also, in order to better integrate with our CI refactoring, it is better for the tests to match the code organization in vLLM, so we can automatically detect which tests to run based on matching directories.

@noooop
Copy link
Collaborator Author

noooop commented Nov 25, 2025

Also, in order to better integrate with our CI refactoring, it is better for the tests to match the code organization in vLLM, so we can automatically detect which tests to run based on matching directories.

I plan to gradually refactor the pooling entrypoints to have the same layout as this test.

#28631

What do you think?


and examples

#29365

@DarkLight1337
Copy link
Member

We have a more extensive plan for this, see #27882 (review)

@noooop
Copy link
Collaborator Author

noooop commented Nov 25, 2025

We have a more extensive plan for this, see #27882 (review)

I think there’s more code that can be reused (e.g. render)between the online and offline versions for the same use case.

Code from the same provider is hardly reusable across different use cases.


APIs should be organized by use case, not by provider.

@DarkLight1337
Copy link
Member

Renderer should be placed outside both offline and online directories., similar to chat utils. There should be no need for offline code to reference online code, and vice versa.

@DarkLight1337 DarkLight1337 enabled auto-merge (squash) November 25, 2025 09:24
@github-actions github-actions bot added the ready ONLY add when PR is ready to merge/full CI is needed label Nov 25, 2025
@DarkLight1337 DarkLight1337 merged commit 7a80b01 into vllm-project:main Nov 25, 2025
18 checks passed
devpatelio pushed a commit to SumanthRH/vllm that referenced this pull request Nov 29, 2025
Signed-off-by: wang.yuqi <yuqi.wang@daocloud.io>
@noooop noooop deleted the resettle_pooling_tests branch December 1, 2025 08:24
kitaekatt pushed a commit to kitaekatt/vllm that referenced this pull request Dec 1, 2025
Signed-off-by: wang.yuqi <yuqi.wang@daocloud.io>
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