Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Skip lm_eval tests if no config file is present #1149

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kylesayrs
Copy link
Collaborator

Purpose

  • Towards allowing users and contributors to run tests locally

Signed-off-by: Kyle Sayers <kylesayrs@gmail.com>
Copy link

👋 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.

Copy link
Collaborator

@dsikka dsikka left a comment

Choose a reason for hiding this comment

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

This change doesn't really make sense.

The entrypoint for these test cases is meant to be the bash script which if given an empty folder for configs, will not run anyway.

It would make more sense to have Brian make that clear in a readme as he's working on an update and setting a default config for TEST_DATA_FILE.

@kylesayrs
Copy link
Collaborator Author

@dsikka So these tests run when running the command python3 -m pytest tests, which is the standard testing pathway. Unless we want to move this file outside of the tests folder, this is likely the best way to skip the test

@dsikka
Copy link
Collaborator

dsikka commented Feb 14, 2025

I actually think these tests being moved out would be better since generally speaking, the overall compute and memory required is significantly larger than any of our other tests and we never want users running it by default.

@kylesayrs kylesayrs marked this pull request as draft February 19, 2025 02:24
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants