LCORE-1223: Add e2e tests for rlsapi v1 /infer endpoint#1067
LCORE-1223: Add e2e tests for rlsapi v1 /infer endpoint#1067major wants to merge 1 commit intolightspeed-core:mainfrom
Conversation
WalkthroughAdds an Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
50ac720 to
19e699c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/e2e/features/steps/rlsapi_v1.py`:
- Around line 30-56: In both store_rlsapi_request_id and
check_rlsapi_request_id_different, tighten validation so request_id is a
non-empty string: after extracting response_json and before assigning
context.stored_request_id (in store_rlsapi_request_id) assert that
response_json["data"]["request_id"] is an instance of str and not empty, and in
check_rlsapi_request_id_different assert the same for current_request_id before
comparing; use clear assertion messages and keep referencing response_json,
current_request_id, and context.stored_request_id to locate the checks.
🧹 Nitpick comments (1)
tests/e2e/features/rlsapi_v1.feature (1)
65-73: Consider consolidating with the basic inference scenario.This scenario validates the same assertions as "Basic inference with minimal request" (lines 8-16): both check for a 200 status code and valid response structure. Unless there's a specific reason to test with different question content, consider removing this scenario to reduce test duplication.
19e699c to
1d14117
Compare
| @@ -0,0 +1,89 @@ | |||
| @Authorized | |||
| Feature: rlsapi v1 /infer endpoint API tests | |||
There was a problem hiding this comment.
I am missing here the cases for 403 and 429 status codes,
you can acheive 403 when using different bearer token.
For 429, the quota is configurable in configuration file.
Also, add a test for 503, with broken llama-stack connection
There was a problem hiding this comment.
@radofuchs At the moment, we don't have 429 responses because we don't currently limit via a quota (it's not in our API spec). Should I do the 403 and 503 errors?
- Add rlsapi_v1.feature with 7 test scenarios - Add rlsapi_v1.py step definitions for response validation - Update test_list.txt to include new feature file Implements LCORE-1223 Signed-off-by: Major Hayden <major@redhat.com>
1d14117 to
820ed10
Compare
Description
Add end-to-end test coverage for the rlsapi v1
/inferendpoint, which serves RHEL Lightspeed Command Line Assistant (CLA) clients.This PR adds 7 high-value test scenarios covering:
Type of change
Tools used to create PR
Related Tickets & Documents
Checklist before requesting a review
Summary by CodeRabbit
New Features
Tests