fix: Allow overriding model in integration tests #3372
Conversation
Pass TEXT_MODEL parameter to pytest to enable model override in integration test runs. Signed-off-by: Derek Higgins <[email protected]>
derekhiggins
requested review from
ashwinb,
yanxi0830,
hardikjshah,
raghotham,
ehhuang,
terrytangyuan,
leseb,
bbrowning,
reluctantfuturist,
mattf and
slekkala1
as code owners
meta-cla
bot
added
the
CLA Signed
|
required for rebase of #3128 |
| @@ -232,7 +232,7 @@ if [[ -n "$TEST_SUBDIRS" ]]; then | |||
| EXTRA_PARAMS="$EXTRA_PARAMS --text-model=$TEXT_MODEL --embedding-model=sentence-transformers/all-MiniLM-L6-v2" | |||
| else | |||
| PYTEST_TARGET="tests/integration/" | |||
| EXTRA_PARAMS="$EXTRA_PARAMS --suite=$TEST_SUITE" | |||
There was a problem hiding this comment.
this was intentionally not done. the model comes as part of the "test suite" definition. I think it is unsatisfactory and am working to make it slightly better. however, can you tell me what broke -- what your use case was?
There was a problem hiding this comment.
I've been keeping the vLLM ci PR up to date #3128,
One of the reasons that the latest rebase fails was because the model name was incorrect
There was a problem hiding this comment.
the suites need to somehow key off provider. maybe detect the provider that's active (i'm already not a fan of --provider)? https://github.com/llamastack/llama-stack/blob/main/tests/integration/suites.py#L28 is only setup for ollama.
we have multiple ways to set the model for testing. what if suite only specified the set of tests to run, not the models?
There was a problem hiding this comment.
thanks, I've closed this and updated the CI PR
|
#3128 has been update to use the new vllm setup |
Pass TEXT_MODEL parameter to pytest to enable model override in integration test runs.