LCORE-730: add new e2e tests for conversation v2 #861
Conversation
WalkthroughThis PR adds conversation cache configuration and comprehensive end-to-end tests for the Conversation Cache V2 API. It introduces SQLite-based cache configuration across multiple test environments, defines extensive Gherkin test scenarios for cache operations, and implements test infrastructure and step definitions for conversation endpoint validation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/e2e/features/conversation_cache_v2.feature (1)
13-28: Known bug documented with @Skip tag.The scenario correctly documents a known issue (AttributeError with empty vector DB causing 500 error) and is marked for skipping. Consider tracking this in the issue tracker if not already done.
Would you like me to open a new issue to track the empty vector DB bug, or is this already tracked in LCORE-730?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
lightspeed-stack.yaml(1 hunks)tests/e2e/configuration/lightspeed-stack-auth-noop-token.yaml(1 hunks)tests/e2e/configuration/lightspeed-stack-no-cache.yaml(1 hunks)tests/e2e/features/conversation_cache_v2.feature(1 hunks)tests/e2e/features/environment.py(1 hunks)tests/e2e/features/steps/common_http.py(1 hunks)tests/e2e/features/steps/conversation.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
tests/e2e/features/**/*.feature
📄 CodeRabbit inference engine (CLAUDE.md)
Use behave (BDD) framework with Gherkin feature files for end-to-end tests
Files:
tests/e2e/features/conversation_cache_v2.feature
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use
pytest-mockwith AsyncMock objects for mocking in tests
Files:
tests/e2e/features/steps/common_http.pytests/e2e/features/environment.pytests/e2e/features/steps/conversation.py
🧠 Learnings (6)
📚 Learning: 2025-09-02T11:09:40.404Z
Learnt from: radofuchs
Repo: lightspeed-core/lightspeed-stack PR: 485
File: tests/e2e/features/environment.py:87-95
Timestamp: 2025-09-02T11:09:40.404Z
Learning: In the lightspeed-stack e2e tests, noop authentication tests use the default lightspeed-stack.yaml configuration, while noop-with-token tests use the Authorized tag to trigger a config swap to the specialized noop-with-token configuration file.
Applied to files:
tests/e2e/configuration/lightspeed-stack-auth-noop-token.yamltests/e2e/configuration/lightspeed-stack-no-cache.yamltests/e2e/features/environment.py
📚 Learning: 2025-09-02T11:15:02.411Z
Learnt from: radofuchs
Repo: lightspeed-core/lightspeed-stack PR: 485
File: tests/e2e/test_list.txt:2-3
Timestamp: 2025-09-02T11:15:02.411Z
Learning: In the lightspeed-stack e2e tests, the Authorized tag is intentionally omitted from noop authentication tests because they are designed to test against the default lightspeed-stack.yaml configuration rather than the specialized noop-with-token configuration.
Applied to files:
tests/e2e/configuration/lightspeed-stack-auth-noop-token.yamltests/e2e/configuration/lightspeed-stack-no-cache.yaml
📚 Learning: 2025-11-24T16:58:04.410Z
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:58:04.410Z
Learning: Applies to tests/e2e/features/**/*.feature : Use behave (BDD) framework with Gherkin feature files for end-to-end tests
Applied to files:
tests/e2e/features/conversation_cache_v2.feature
📚 Learning: 2025-11-24T16:58:04.410Z
Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:58:04.410Z
Learning: Applies to tests/e2e/test_list.txt : Maintain test list in `tests/e2e/test_list.txt` for end-to-end tests
Applied to files:
tests/e2e/features/conversation_cache_v2.feature
📚 Learning: 2025-08-25T09:05:18.350Z
Learnt from: radofuchs
Repo: lightspeed-core/lightspeed-stack PR: 445
File: tests/e2e/features/health.feature:50-53
Timestamp: 2025-08-25T09:05:18.350Z
Learning: The step definition for ignoring specific fields in response body comparisons is implemented in tests/e2e/features/steps/common_http.py at line 175 as `then('The body of the response, ignoring the "{field}" field, is the following')`.
Applied to files:
tests/e2e/features/steps/common_http.py
📚 Learning: 2025-08-25T09:11:38.701Z
Learnt from: radofuchs
Repo: lightspeed-core/lightspeed-stack PR: 445
File: tests/e2e/features/environment.py:0-0
Timestamp: 2025-08-25T09:11:38.701Z
Learning: In the Behave Python testing framework, the Context object is created once for the entire test run and reused across all features and scenarios. Custom attributes added to the context persist until explicitly cleared, so per-scenario state should be reset in hooks to maintain test isolation.
Applied to files:
tests/e2e/features/environment.py
🧬 Code graph analysis (2)
tests/e2e/features/environment.py (1)
tests/e2e/utils/utils.py (2)
switch_config(98-106)restart_container(130-144)
tests/e2e/features/steps/conversation.py (2)
src/models/responses.py (2)
endpoint(1325-1329)model(1522-1526)tests/e2e/utils/utils.py (1)
replace_placeholders(147-160)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-pr
- GitHub Check: e2e_tests (ci)
- GitHub Check: e2e_tests (azure)
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
🔇 Additional comments (21)
tests/e2e/features/steps/common_http.py (1)
123-132: LGTM! Enhanced error reporting for test failures.The improved error message now includes the response body (JSON or text) when status code mismatches occur, which significantly aids debugging. The fallback pattern (try JSON, fall back to text) is appropriate.
tests/e2e/features/environment.py (2)
82-90: LGTM! NoCacheConfig scenario handling properly implemented.The implementation follows the established pattern for scenario-specific configuration switching (consistent with
InvalidFeedbackStorageConfig). The immediate config switch and container restart ensures the no-cache configuration is active before the scenario runs.
98-100: LGTM! Cleanup properly restores configuration.The after_scenario hook correctly restores the feature configuration and restarts the container, ensuring test isolation.
tests/e2e/configuration/lightspeed-stack-no-cache.yaml (1)
1-28: LGTM! No-cache configuration for testing error scenarios.This configuration file correctly omits the
conversation_cacheblock to enable testing of error handling when the cache is not configured. The inline comment (line 24) clearly documents the intent.tests/e2e/features/conversation_cache_v2.feature (7)
88-141: LGTM! Comprehensive GET by ID testing with schema validation.The test scenario covers the happy path with multiple messages (2 turns), validates full metadata structure, and includes detailed JSON schema validation. The schema correctly enforces message structure with user/assistant types.
192-209: LGTM! Resilience testing when llama-stack is down.This scenario validates that conversation retrieval continues to work when the llama-stack connection is disrupted, which is critical for system resilience. The test properly verifies that cached data remains accessible.
212-218: LGTM! Error handling for missing cache configuration.The
@NoCacheConfigscenario correctly validates that the system returns a 500 error with an appropriate message when the conversation cache is not configured. This aligns with the newlightspeed-stack-no-cache.yamlconfiguration file.
225-244: LGTM! DELETE endpoint testing with verification.The scenario properly tests deletion and verifies the conversation is gone by attempting to retrieve it (expecting 404). The use of
ignoring the "conversation_id" fieldcomparison is appropriate for the success response.
287-307: LGTM! Resilience testing for DELETE operations.This scenario validates that conversation deletion continues to work even when llama-stack is down, ensuring cached operations remain functional during upstream service disruptions.
314-334: LGTM! PUT endpoint testing with persistence verification.The scenario tests topic summary updates and verifies the change persists by retrieving the conversation list and checking the updated value. This ensures the PUT operation correctly modifies the cache.
385-397: LGTM! Input validation testing for empty topic summary.The scenario correctly tests that empty topic summaries are rejected with a 422 validation error. The expected error message "String should have at least 1 character" indicates proper Pydantic validation.
tests/e2e/features/steps/conversation.py (8)
95-119: LGTM! PUT endpoint implementation follows established patterns.The implementation properly:
- Asserts required context attributes exist
- Constructs the URL following the same pattern as GET/DELETE
- Handles the
<EMPTY>token for testing empty string validation (line 112-113)- Uses requests.put with JSON payload
122-140: LGTM! Specific conversation ID variant properly implemented.This step definition allows testing with explicit conversation IDs (for malformed ID testing, non-existent IDs, etc.), complementing the "from above" variant. The implementation is consistent and correct.
143-164: LGTM! Empty topic summary testing properly supported.This dedicated step definition enables testing of validation errors when an empty topic_summary is provided. The explicit
{"topic_summary": ""}payload is appropriate for testing the 422 validation error case.
182-200: LGTM! Metadata validation is thorough.The validation step properly checks:
- Presence of required fields (
last_message_timestamp,topic_summary)- Type validation (timestamp should be number)
- Value constraints (timestamp should be positive)
- Non-null constraints for topic_summary
203-214: LGTM! Topic summary validation is precise.The step definition enables verification of updated topic summaries after PUT operations, which is essential for testing persistence of updates.
267-278: LGTM! Message count validation is straightforward.The step correctly validates the number of entries in the
chat_historyarray, which corresponds to the number of conversation turns (not individual messages).
281-316: LGTM! Comprehensive chat history metadata validation.This validation step thoroughly checks:
- Each turn has required fields (provider, model, messages, started_at, completed_at)
- Each turn has exactly 2 messages (user + assistant)
- Messages have correct types and content fields
- All fields are non-empty
The detailed assertions with turn indices provide clear debugging information when failures occur.
319-343: LGTM! Model and provider validation with placeholder support.The step correctly:
- Uses
replace_placeholdersto handle {MODEL} and {PROVIDER} tokens- Validates all turns use the expected model/provider
- Provides clear error messages with turn indices
tests/e2e/configuration/lightspeed-stack-auth-noop-token.yaml (1)
24-28: LGTM! Conversation cache configuration added.The sqlite-based conversation cache configuration is consistent with the main configuration file. The
/tmp/data/conversation-cache.dbpath is appropriate for ephemeral test data. Verify that the/tmp/datadirectory is created before tests run—check test setup and initialization scripts to ensure this directory exists or is properly created as part of test infrastructure setup.lightspeed-stack.yaml (1)
24-28: LGTM! Conversation cache configuration added.The sqlite-based conversation cache is properly configured. The inline comment clarifies persistence behavior across requests and cleanup between test runs. However, manual verification is needed to confirm this file's deployment context (development/test vs. production) and whether the
/tmppath is appropriate for the intended environment.
|
LGTM |
2 participants
Description
Type of change
Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit
Release Notes
New Features
Tests
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.