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

Fix integration test with multiple parts #681

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pedrito87
Copy link
Contributor

PR Description

Fix integration test connected to the multi part tool calling.

Relevant issues

This is connected to this fix from previous PR.

Type

πŸ› Bug Fix
βœ… Test

Changes(optional)

Integration test test_chat_function_calling_with_multiple_parts can now be run without failing.

@pedrito87 pedrito87 marked this pull request as ready for review January 8, 2025 11:24
@pedrito87
Copy link
Contributor Author

@lkuligin you can take a look at this integration test fix. It is possible for it not to fail now thanks to my previous PR. Cheers.

@lkuligin
Copy link
Collaborator

integration tests are green at the moment, are you sure we need this PR?

@pedrito87
Copy link
Contributor Author

pedrito87 commented Jan 20, 2025

integration tests are green at the moment, are you sure we need this PR?

@lkuligin They are green because this test is marked as pytest xfail, which runs the test and the marks the test as XFAIL (if it actually fails) or XPASS (if it passes despite being marked as xfail). Both cases are considered a success in terms of CI/CD.

My change actually fixes this integration test, which in the current form has no chance of succeeding, and is a good example of how the system works in terms of multiple tools uses.

And when you compare the integration test between the main branch and this one you will see that the test fixed here is actually an XPASS on this branch, compared to XFAIL on main branch.

So, answering your question, I think it is worth merging πŸ‘
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants