Skip to content

Update int tests #50

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

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

Update int tests #50

wants to merge 3 commits into from

Conversation

adeelehsan
Copy link
Contributor

No description provided.

response = self.client.chats.list()
for chat in response:
self.assertIn(chat.id, chat_ids)
# def test_list_chats(self):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

List chat method have issue. Debugging it. Thats why its commented out.

self.assertLessEqual(len(limited_llms.items), 2)

# def test_create_and_delete_llm(self):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Create method have error. Thats why its commented out

corpora=[
KeyedSearchCorpus(
corpus_key="test-chat",
corpus_key=cls.corpus_name,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Curious why lexical_interpolation is set to "1" - that's no vector search and full lexical search.

@@ -103,7 +103,7 @@ def __init__(
self,
*,
environment: VectaraEnvironment = VectaraEnvironment.PRODUCTION,
api_key: typing.Optional[str] = os.getenv("VECTARA_API_KEY"),
api_key: typing.Optional[str] = None,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does this need to change to None?

corpus_key=self.corpus.key,
file=file,
table_extraction_config=table_config,
filename="test_document_with_table_extraction.pdf",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this the right filename?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess so. file name is based on the config. just to differentiate the filenames.


chunking_strategy = MaxCharsChunkingStrategy(
type="max_chars_chunking_strategy",
max_chars_per_chunk=200
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps add another test with higher chars_per_chunk (e.g. 1024), and maybe also test the limit to see that it fails if above the limit in the API?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps add another test with higher chars_per_chunk (e.g. 1024), and maybe also test the limit to see that it fails if above the limit in the API?

We don't raise error if we specify the bigger limit then supported. we just create it with the default I think.

@ofermend
Copy link
Collaborator

ofermend commented May 2, 2025

Not that there's a new file called "test_documnet" which is a type, so now also the old "test_document" and the new one exist.

@ofermend
Copy link
Collaborator

ofermend commented May 2, 2025

There's a folder called "int_tests" vs "Vectara_int_test" - can u pls explain both names? are these all integration tests for Vectara in one way or another?

from dotenv import load_dotenv

# Load environment variables from .env file
env_path = Path(__file__).parent / '.env'
Copy link
Collaborator

Choose a reason for hiding this comment

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

What should .env include for the tests to path?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Vectara API key to run the tests

enable_factual_consistency_score=True,
)
self.chat_params = ChatParameters(store=True)
self.request_options = RequestOptions(timeout_in_seconds=100)

def test_chat(self):
session = self.client.create_chat_session(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here - perhaps check that the chat_id from the first turn (after create_chat_session) is the same as in the second turn? Also - I think a better example is helpful here - something where the question in the 2nd turn depends on the first turn and you can check the response (not just IsNotNone)

@adeelehsan
Copy link
Contributor Author

Not that there's a new file called "test_documnet" which is a type, so now also the old "test_document" and the new one exist.

Yeah earlier they were mis spelled. Now I have change it to documents(tests documents operations) and documents manager(tests the document manager that is written by david).

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