-
Notifications
You must be signed in to change notification settings - Fork 3
Tests #99
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
Tests #99
Conversation
Updated the docstring to clarify test purpose.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## add_authentication #99 +/- ##
=======================================================
+ Coverage 99.85% 100.00% +0.14%
=======================================================
Files 21 21
Lines 686 690 +4
=======================================================
+ Hits 685 690 +5
+ Misses 1 0 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds authentication support for Elasticsearch connections, enabling the application to work with authentication-enabled Elasticsearch instances. The changes implement Basic and Bearer authentication mechanisms and update the test infrastructure to support testing with authentication.
- Adds authentication header support to Elasticsearch requests via
get_elasticsearch_auth_header()
- Updates docker-compose configuration to enable Elasticsearch security with default credentials
- Implements comprehensive test coverage for authentication scenarios
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
tests/unit/test_auth_integration.py | New integration tests verifying authentication requirements and credential validation |
tests/unit/es_client/test_es_client.py | Unit tests for authentication header handling in search requests |
tests/unit/conftest.py | Test configuration updated to set authentication environment variables |
tests/helpers/unit_setup.py | Service startup configuration with authentication credentials |
tests/helpers/init_elasticsearch.py | Updated to use configurable Elasticsearch URL from environment |
src/search2_rpc/service.py | Modified to add authentication headers to index listing requests |
docker-compose.yaml | Enabled Elasticsearch security with authentication credentials |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
import os | ||
# Set environment variables BEFORE any other imports | ||
# This ensures the config module picks up the auth credentials | ||
os.environ['ELASTICSEARCH_URL'] = 'http://localhost:9200' | ||
os.environ['ELASTICSEARCH_AUTH_USERNAME'] = 'elastic' | ||
os.environ['ELASTICSEARCH_AUTH_PASSWORD'] = 'changeme' | ||
|
||
import pytest # noqa: E402 | ||
from tests.helpers.unit_setup import ( # noqa: E402 |
Copilot
AI
Oct 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setting environment variables directly in conftest.py creates hardcoded test configuration. Consider using pytest fixtures or a test configuration file to make these values configurable and avoid duplicating the same environment variable setup that also appears in unit_setup.py (lines 29-31).
Copilot uses AI. Check for mistakes.
# Set environment variables BEFORE importing config module | ||
# This ensures config picks up the auth credentials | ||
os.environ['ELASTICSEARCH_URL'] = 'http://localhost:9200' | ||
os.environ['ELASTICSEARCH_AUTH_USERNAME'] = 'elastic' | ||
os.environ['ELASTICSEARCH_AUTH_PASSWORD'] = 'changeme' |
Copilot
AI
Oct 21, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This duplicates the exact same environment variable configuration found in tests/unit/conftest.py (lines 5-7). Centralizing this configuration in one location would improve maintainability and prevent potential inconsistencies.
Copilot uses AI. Check for mistakes.
No description provided.