Skip to content

Conversation

@tisnik
Copy link
Contributor

@tisnik tisnik commented Nov 16, 2025

Description

LCORE-987: proper HTTP method in conversation endpoint e2e tests

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement

Related Tickets & Documents

  • Related Issue #LCORE-987

Summary by CodeRabbit

  • Tests
    • Updated test scenarios for the conversations endpoint to use appropriate HTTP methods and verify expected error handling.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 16, 2025

Walkthrough

The PR updates two test scenarios in the conversations feature file to use the DELETE HTTP method instead of GET when testing endpoint behavior against a disrupted llama-stack service. The expected 503 error responses and error messages remain unchanged.

Changes

Cohort / File(s) Change Summary
Conversation endpoint test method updates
tests/e2e/features/conversations.feature
Modified two scenarios to use DELETE instead of GET HTTP method for conversation endpoint requests when testing service unavailability (503 response)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

  • Verify that both GET→DELETE conversions align with the intended endpoint behavior
  • Confirm that the change reflects the actual API contract being tested

Possibly related PRs

Suggested reviewers

  • maorfr

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: correcting HTTP methods in conversation endpoint e2e tests, matching the actual file modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8d163cf and 08704f9.

📒 Files selected for processing (1)
  • tests/e2e/features/conversations.feature (1 hunks)
⏰ 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: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: e2e_tests (ci)
  • GitHub Check: e2e_tests (azure)
🔇 Additional comments (1)
tests/e2e/features/conversations.feature (1)

172-172: LGTM! Bug fix correctly aligns test method with scenario intention.

The change from GET to DELETE method is correct. The scenario at line 162 explicitly tests the "DELETE endpoint" behavior when llama-stack is unavailable, so using the DELETE method properly validates this endpoint's error handling.

This fix ensures consistency with other DELETE endpoint tests in the file (lines 140, 153, 159) and correctly distinguishes this test from the similar GET endpoint test at line 127.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@tisnik tisnik merged commit 437f6bc into lightspeed-core:main Nov 16, 2025
21 of 23 checks passed
@coderabbitai coderabbitai bot mentioned this pull request Nov 16, 2025
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant