Mock OpenSearch#138
Conversation
Signed-off-by: Stephan Meijer <me@stephanmeijer.com>
add type hints to test functions Signed-off-by: Stephan Meijer <me@stephanmeijer.com>
Signed-off-by: Stephan Meijer <me@stephanmeijer.com>
Signed-off-by: Stephan Meijer <me@stephanmeijer.com>
…tests Signed-off-by: Stephan Meijer <me@stephanmeijer.com>
Signed-off-by: Stephan Meijer <me@stephanmeijer.com>
Signed-off-by: Stephan Meijer <me@stephanmeijer.com>
Signed-off-by: Stephan Meijer <me@stephanmeijer.com>
Signed-off-by: Stephan Meijer <me@stephanmeijer.com>
Signed-off-by: Stephan Meijer <me@stephanmeijer.com>
Signed-off-by: Stephan Meijer <me@stephanmeijer.com>
Signed-off-by: Stephan Meijer <me@stephanmeijer.com>
Signed-off-by: Stephan Meijer <me@stephanmeijer.com>
fixup! add xdist_group hook for integration tests
fixup! ruff format utils_opensearch.py
There was a problem hiding this comment.
Pull request overview
This PR removes the dependency on a live OpenSearch instance for backend unit tests by introducing an OpenSearch client mock fixture + helper utilities, migrating existing tests to rely on mocked responses, and updating CI to run the unit-test subset without provisioning an OpenSearch service.
Changes:
- Added
mock_opensearch_clientfixture (with@cachehandling) and OpenSearch response helper utilities for tests. - Migrated core unit tests to use mocked OpenSearch calls and reorganized tests under
core/tests/unit/.... - Updated CI and local tooling to run unit tests without starting OpenSearch (and adjusted OpenSearch auth handling in settings/client).
Reviewed changes
Copilot reviewed 10 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/backend/find/settings.py |
Adjusts OpenSearch password setting default/typing to support passwordless configs. |
src/backend/core/services/opensearch.py |
Allows http_auth=None when credentials aren’t configured. |
src/backend/core/tests/conftest.py |
Introduces mock_opensearch_client fixture and updates index cleanup fixture to skip real OpenSearch ops when mocking. |
src/backend/core/tests/utils_opensearch.py |
Adds helpers for building mock OpenSearch responses and configuring a mock client. |
src/backend/core/tests/unit/conftest.py |
Auto-enables OpenSearch mocking for all unit tests in core/tests/unit. |
src/backend/core/tests/unit/services/test_service_isolation.py |
Updates service-isolation tests to run with the mocked client. |
src/backend/core/tests/unit/api/test_documents_index_bulk.py |
Converts bulk indexing API tests to use mocked bulk/index calls. |
src/backend/core/tests/unit/api/test_documents_delete.py |
Converts delete API tests to use mocked search/delete-by-query calls. |
src/backend/core/tests/unit/test_selftests.py |
Adds unit tests for the selftest registry/result objects. |
src/backend/core/tests/unit/test_schemas.py |
Adds unit tests for schema helper(s) (e.g., cleanlist). |
src/backend/core/tests/unit/test_admin_selftest.py |
Adds tests for the admin selftest view behavior. |
src/backend/core/tests/unit/models/test_services.py |
Adds Service model unit tests (constraints/slugification/token length). |
src/backend/core/tests/unit/__init__.py |
Initializes the unit test package. |
src/backend/core/tests/unit/api/__init__.py |
Initializes the unit test API package. |
src/backend/core/tests/unit/models/__init__.py |
Initializes the unit test models package. |
src/backend/core/tests/unit/services/__init__.py |
Initializes the unit test services package. |
Makefile |
Uses pytest -n auto for parallel backend tests. |
.github/workflows/find.yml |
Removes OpenSearch service from CI and restricts test run to core/tests/unit/. |
Comments suppressed due to low confidence (3)
src/backend/core/tests/unit/services/test_service_isolation.py:44
- These assertions don’t validate the actual behavior of the indexing endpoint: the test sets mock_opensearch_client.get.return_value to contain the expected service and then asserts it, without checking what was sent to OpenSearch. To preserve the original intent (service derived from auth, not payload), assert on mock_opensearch_client.index/bulk call arguments (the indexed document body should have service=request.auth.name and should not keep the spoofed payload value).
src/backend/core/tests/unit/api/test_documents_index_bulk.py:320 - This bulk test currently stubs mock_opensearch_client.get.return_value but never reads it or asserts anything about default values being applied, so the test no longer verifies the behavior described in its docstring. Add an assertion against the payload passed to mock_opensearch_client.bulk (or explicitly call get and assert) to confirm the missing optional field is defaulted correctly during indexing.
src/backend/core/tests/unit/api/test_documents_delete.py:54 - These delete tests now only assert the HTTP response while fully controlling mock_opensearch_client.search/delete_by_query return values, so they won’t fail if the view builds the wrong OpenSearch query (e.g., missing the users filter or incorrect AND logic for document_ids/tags). To keep coverage equivalent to the prior integration-style tests, assert search() was called with a body containing the expected bool/must filters and that delete_by_query() was called (or not called) with the expected ids list.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
fixup! restore OpenSearch in CI, mark demo test as integration
fixup! revert to SecretValue for OPENSEARCH_PASSWORD
There was a problem hiding this comment.
Tests here seems a bit weak as they are testing the mock side-effect itself. Maybe we can think of an alternative testing strategy?
There was a problem hiding this comment.
What about rewriting all the tests from scratch? :)
Signed-off-by: Stephan Meijer <me@stephanmeijer.com>
|
Important Moved to #139 |
Important
Moved to #139
Summary
mock_opensearch_clientfixture withcache_clear()handling for the@cachedecoratorutils_opensearch.pywith typed mock response helperstests/unit/{api,services,models}/hierarchy@pytest.mark.integrationmarker for tests requiring real OpenSearchpytest_collection_modifyitemshook to run integration tests serially viaxdist_groupChanges
-n auto)@pytest.mark.integration, run serially on single workerdemo/tests/test_commands_create_demo.py)OPENSEARCH_PASSWORDremainsSecretValue(requires env var)Test Structure
Testing