-
Notifications
You must be signed in to change notification settings - Fork 160
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
fix streaming test #140
fix streaming test #140
Conversation
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.
It seems the generated response changed @radames
we can ignore spaces imo in the token, eg text: expect.stringContaining(tokenText)
to be more robust
It might be due to update of text-generation-inference to v0.5.0, noticed it has changes related to token decoding in huggingface/text-generation-inference#144. |
@@ -68,7 +68,7 @@ | |||
"method": "POST" | |||
}, | |||
"response": { | |||
"body": "[{\"generated_text\":\"The answer to the universe is this: it is all of history.\\n\\nIt's time to reclaim our right to see it as we see it. It's time to take back the right to think and to make our opinions heard in the process\"}]", | |||
"body": "[{\"generated_text\":\"The answer to the universe is the fundamental property of space, but we're dealing only with the simplest possible set of answers, which is why we need to talk about how we come to know and understand what sets matter.\\n\\nLet's take a\"}]", |
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.
It's funny that tapes.json
would update with random outputs each time :)
9f1fe9a
to
19c1324
Compare
Not sure if this is due to the non-deterministic of text generation, however the space before the
" one"
keep making the test fail. So I've added the index to add the space WDYT @vvmnnnkv