Fix mock_provider_lazy configuration in integration tests#232
Conversation
…tion tests Added missing `__name__` and `return_value` assignments to `mock_provider_lazy` in Qdrant provider integration tests. This ensures the mocks are consistent with how they are used by the provider factory and matches the pattern used for other providers in the same test file. Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts Qdrant provider integration tests to correctly configure the lazy mock provider so it behaves like a late-imported provider class, matching the existing Voyage provider test setup. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The setup for
mock_provider_lazyis duplicated between the two Qdrant tests; consider extracting a small helper (e.g.,make_qdrant_lazy_mock()) to configure__name__,_resolve, andreturn_valuein one place for consistency and easier future changes. - The inline comment referencing a specific line number (
line 959) intest_qdrant_provider_with_url_modeis brittle; instead, describe the behavioral reason (e.g., "mock_provider_lazy is invoked like the provider class") so the comment stays accurate if code is refactored.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The setup for `mock_provider_lazy` is duplicated between the two Qdrant tests; consider extracting a small helper (e.g., `make_qdrant_lazy_mock()`) to configure `__name__`, `_resolve`, and `return_value` in one place for consistency and easier future changes.
- The inline comment referencing a specific line number (`line 959`) in `test_qdrant_provider_with_url_mode` is brittle; instead, describe the behavioral reason (e.g., "mock_provider_lazy is invoked like the provider class") so the comment stays accurate if code is refactored.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
Fixes the lazy provider mock setup in the (deprecated/skipped) client-factory integration tests so the mocked “late-imported provider class” behaves like a real callable class/factory.
Changes:
- Set
mock_provider_lazy.__name__for Qdrant mocks (memory + URL modes) to match the pattern used by the Voyage mock. - Ensure the Qdrant URL-mode lazy mock has an explicit
return_valueso calls succeed when the factory treats it as callable.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@jules please implement the suggestions below: The setup for mock_provider_lazy is duplicated between the two Qdrant tests; consider extracting a small helper (e.g., make_qdrant_lazy_mock()) to configure name, _resolve, and return_value in one place for consistency and easier future changes. |
I have implemented the suggestions. I created a |
- Implemented `make_lazy_provider_mock` helper to centralize mock configuration. - Replaced brittle line-number comments with behavioral descriptions. - Updated all Voyage and Qdrant provider tests to use the new helper. Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
This submission fixes an issue in the integration tests for the client factory where the
mock_provider_lazyobject (representing a late-imported provider class) was not correctly configured with a__name__and areturn_value.Specifically:
test_qdrant_provider_with_memory_mode, addedmock_provider_lazy.__name__ = "MockQdrantProvider".test_qdrant_provider_with_url_mode, addedmock_provider_lazy.__name__ = "MockQdrantProvider"andmock_provider_lazy.return_value = Mock().These changes align the Qdrant mock configuration with the Voyage mock configuration in the same file and ensure that the factory can successfully call the mock as if it were a class or factory function.
PR created automatically by Jules for task 16248330206876263418 started by @bashandbone
Summary by Sourcery
Fix Qdrant provider integration tests by correctly configuring the lazy mock provider used by the client factory.
Bug Fixes:
Tests: