-
-
Notifications
You must be signed in to change notification settings - Fork 12k
[Refactor] [3/N] Move tool parser tests and run on CPU #30693
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Refactor] [3/N] Move tool parser tests and run on CPU #30693
Conversation
Signed-off-by: DarkLight1337 <[email protected]>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Signed-off-by: DarkLight1337 <[email protected]>
Signed-off-by: DarkLight1337 <[email protected]>
There was a problem hiding this 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 aims to move tool parser tests to a CPU-only CI job to reduce costs. The changes involve renaming test files and updating CI configuration. However, there are several issues in the CI configuration files. The paths for the moved test files are incorrect, which will prevent these tests from running. Additionally, some pytest commands seem to be malformed, which could cause CI jobs to fail. I've provided specific comments and suggestions to fix these issues.
Signed-off-by: DarkLight1337 <[email protected]>
Signed-off-by: DarkLight1337 <[email protected]>
|
/gemini review |
There was a problem hiding this 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 refactors the testing setup for tool parsers to reduce CI costs. The changes involve moving tool parser tests from tests/tool_use to a new tests/tool_parsers directory and configuring them to run on CPU within an existing test job. This correctly consolidates CPU-based tests and frees up GPU resources. The CI configuration files (.buildkite/*.yaml) are updated accordingly, removing the separate CPU test job for tool use and simplifying the GPU test command. The pytest.mark.cpu_test markers have been correctly removed from the moved test files. The changes are logical, consistent, and effectively achieve the goal of optimizing CI resource usage.
chaunceyjiang
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks~
…30693) Signed-off-by: DarkLight1337 <[email protected]>
…30693) Signed-off-by: DarkLight1337 <[email protected]> Signed-off-by: Joachim Studnia <[email protected]>
…30693) Signed-off-by: DarkLight1337 <[email protected]>
Purpose
Follow-up to #30675 to reduce CI cost
Test Plan
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.