Skip to content
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

Use unique test names in TestvLLM #1124

Merged
merged 1 commit into from
Feb 4, 2025
Merged

Use unique test names in TestvLLM #1124

merged 1 commit into from
Feb 4, 2025

Conversation

dbarbuzzi
Copy link
Collaborator

SUMMARY:
This PR updates the TestvLLM class to pass the config file being tested as a parameter to the test. Functionally, this is no different than the existing behavior with regards to the test, but practically, it means that the config file will be included in the generated test name making it much easier to identify which config file is being tested across the board (in console output, in Testmo/reporting tools, etc.).

As a result, there is no longer a need for the previously added fixture that embedded the config file into the test suite metadata for downstream processing.

TEST PLAN:
A custom test run executing the e2e tests against this branch is here: https://github.com/neuralmagic/llm-compressor-testing/actions/runs/13138573134/job/36659921426
Reviewing the output of the 'run e2e tests' steps, the test names are updated:

  • Old: TestvLLM.test_vllm
  • New: TestvLLM_0_tests_e2e_vLLM_configs_fp8_dynamic_per_token_yaml.test_vllm

Note: The failures (RuntimeError: Failed to infer device type) are expected due to a recent change (seemingly on the vLLM side).

Signed-off-by: Domenic Barbuzzi <[email protected]>
Copy link

github-actions bot commented Feb 4, 2025

👋 Hi! Thank you for contributing to llm-compressor. Please add the ready label when the PR is ready for review.

Note: This is required to complete the testing suite, please only add the label once the PR is code complete and local testing has been performed.

@dbarbuzzi dbarbuzzi added the ready When a PR is ready for review label Feb 4, 2025
Copy link
Collaborator

@brian-dellabetta brian-dellabetta left a comment

Choose a reason for hiding this comment

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

I didn't know you could do this, awesome!

@dsikka dsikka merged commit 206d894 into main Feb 4, 2025
8 checks passed
@dsikka dsikka deleted the unique-test-names branch February 4, 2025 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready When a PR is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants