add image indexing tests #4477
add image indexing tests #4477
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR adds an image indexing integration test while updating settings API endpoints, file upload utilities, and test models to support image extraction and analysis.
- Modified
/backend/tests/integration/common_utils/managers/settings.pyto remove outdated endpoint prefixes. - Added
/backend/tests/integration/common_utils/connectors.pyto handle file uploads with robust error handling. - Updated
/backend/tests/integration/common_utils/test_models.pyby adding new boolean fields in DATestSettings. - Changed REST endpoint in
/backend/onyx/server/settings/api.pyfrom PUT to PATCH for partial updates. - Introduced
/backend/tests/integration/tests/image_indexing/indexing_images.pyfor end-to-end image indexing verification.
5 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile
| test_file_dir = "tests/integration/common_utils/test_data" | ||
| os.makedirs(test_file_dir, exist_ok=True) | ||
| test_file_path = os.path.join(test_file_dir, "sample.pdf") |
There was a problem hiding this comment.
logic: Test assumes sample.pdf exists. Verify file presence or handle missing file scenarios.
| test_file_dir = "tests/integration/common_utils/test_data" | |
| os.makedirs(test_file_dir, exist_ok=True) | |
| test_file_path = os.path.join(test_file_dir, "sample.pdf") | |
| test_file_dir = "tests/integration/common_utils/test_data" | |
| os.makedirs(test_file_dir, exist_ok=True) | |
| test_file_path = os.path.join(test_file_dir, "sample.pdf") | |
| assert os.path.exists(test_file_path), f"Test file {test_file_path} not found" |
| and default_provider.default_vision_model | ||
| and model_supports_image_input( | ||
| if default_provider and default_provider.default_vision_model: | ||
| if model_supports_image_input( |
There was a problem hiding this comment.
prefer keeping these if's together with and
| # Safely get groups - handle detached instance case | ||
| try: | ||
| groups = [group.id for group in llm_provider_model.groups] | ||
| except Exception: |
There was a problem hiding this comment.
would prefer catching the exact exception described in the comment, but all good if you really don't want errors here
| connector_id=connector.id, | ||
| access_type=AccessType.PUBLIC, | ||
| user_performing_action=admin_user, | ||
| ) |
There was a problem hiding this comment.
I feel like we should really have a utility that simplifies creating and linking credentials and connectors, more of a TODO item though
pablonyx
force-pushed
the
image_indexing_tests
branch
from
7937c6d to
62d3f92
Compare
Description
Fixes https://linear.app/danswer/issue/DAN-1768/image-indexing-flow-tests
How Has This Been Tested?
[Describe the tests you ran to verify your changes]
Backporting (check the box to trigger backport action)
Note: You have to check that the action passes, otherwise resolve the conflicts manually and tag the patches.