-
Notifications
You must be signed in to change notification settings - Fork 49
Improve test isolation and pytest integration #68
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
Improve test isolation and pytest integration #68
Conversation
Signed-off-by: elronbandel <elronbandel@gmail.com>
Signed-off-by: elronbandel <elronbandel@gmail.com>
avinash2692
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.
Hi @elronbandel:
- The API for
validatemight have changed since you last pushed. I think merging with main will solve this. - I also got these errors when I ran the tests:
====================================================================== short test summary info ======================================================================
FAILED test/backends/test_huggingface.py::test_constraint_lora_with_requirement - TypeError: MelleaSession.validate() got an unexpected keyword argument 'return_full_validation_results'
FAILED test/backends/test_huggingface.py::test_constraint_lora_override - TypeError: MelleaSession.validate() got an unexpected keyword argument 'return_full_validation_results'
FAILED test/backends/test_huggingface.py::test_constraint_lora_override_does_not_override_alora - TypeError: MelleaSession.validate() got an unexpected keyword argument 'return_full_validation_results'
FAILED test/backends/test_huggingface.py::test_llmaj_req_does_not_use_alora - TypeError: MelleaSession.validate() got an unexpected keyword argument 'return_full_validation_results'
FAILED test/stdlib_basics/test_genslot.py::test_func - NameError: name 'i' is not defined
ERROR test/stdlib_basics/test_genslot.py::test_gen_slot_output - NameError: name 'i' is not defined
ERROR test/stdlib_basics/test_genslot.py::test_sentiment_output - NameError: name 'i' is not defined
The first half is cause of validate but can't seem to figure out where the second half is coming from.
test/backends/test_huggingface.py
Outdated
| f"formatting directive failed for {random_result.value}: {e.json()}" | ||
| ) | ||
| ), | ||
| return_full_validation_results=True, |
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.
return_full_validation_results might be invalid because of #54. Can you remove this in all the tests?
|
@elronbandel, not sure if this makes things easier / makes sense, but most of the backend tests are very similar. It might make sense to move all of the common tests to one file and just parameterize the session / backend. I took a shot at it in the old repo, but kept getting sidetracked by other high priority work. No need to incorporate any of it or this suggestion. |
Previously, pytest would instantiate all test classes during discovery, which caused every backend to be loaded including heavy ones like the huggingface backend loading 8B model.
Now, tests can be scanned without eagerly realizing all backend. This means that missing dependencies or a faulty backend no longer block discovery: the remaining tests are still visible and runnable.
As a result, you can run and debug tests with VSCode tests extension (or other pytest integrations) without crashes or unnecessary loading of large models on every code change.