Skip to content

fix logic in e2e tests to handle all providers#1309

Merged
tisnik merged 1 commit intolightspeed-core:mainfrom
radofuchs:e2e_test_fix
Mar 11, 2026
Merged

fix logic in e2e tests to handle all providers#1309
tisnik merged 1 commit intolightspeed-core:mainfrom
radofuchs:e2e_test_fix

Conversation

@radofuchs
Copy link
Copy Markdown
Contributor

@radofuchs radofuchs commented Mar 11, 2026

Description

fix logic in e2e tests to handle all providers

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
  • Benchmarks improvement

Tools used to create PR

Identify any AI code assistants used in this PR (for transparency and review context)

  • Assisted-by: (e.g., Claude, CodeRabbit, Ollama, etc., N/A if not used)
  • Generated by: (e.g., tool name and version; N/A if not used)

Related Tickets & Documents

  • Related Issue #
  • Closes #

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  • Please provide detailed steps to perform tests related to this code change.
  • How were the fix/results from this change verified? Please provide relevant screenshots or results.

Summary by CodeRabbit

  • Tests
    • Updated error message validation to use dynamic model ID placeholders instead of hard-coded values across query and streaming query test scenarios.
    • Enhanced test assertion utility to support variable placeholder replacement, improving test flexibility and maintainability across different model configurations.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3ba16aba-f0f6-43a7-bd78-ade6a0c37a48

📥 Commits

Reviewing files that changed from the base of the PR and between eb97491 and c9848f5.

📒 Files selected for processing (3)
  • tests/e2e/features/query.feature
  • tests/e2e/features/steps/common_http.py
  • tests/e2e/features/streaming_query.feature

Walkthrough

The changes add support for dynamic placeholder substitution in BDD test assertions. Feature files now use {MODEL} placeholders in expected error messages instead of hard-coded model identifiers, while the test step helper function is enhanced to resolve these placeholders before comparison.

Changes

Cohort / File(s) Summary
Feature Test Updates
tests/e2e/features/query.feature, tests/e2e/features/streaming_query.feature
Updated error message expectations to use dynamic {MODEL} placeholder instead of hard-coded "gpt-4o-mini" for missing provider and unknown provider scenarios.
Test Step Helper Enhancement
tests/e2e/features/steps/common_http.py
Enhanced check_response_body_contains function to preprocess substrings with replace_placeholders(context, substring) before assertion and error reporting, enabling placeholder resolution in test assertions.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 3
✅ 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 'fix logic in e2e tests to handle all providers' is directly related to the main changes, which update e2e test error message expectations to use dynamic placeholders instead of hardcoded provider names, enabling support for multiple providers.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

Copy link
Copy Markdown
Contributor

@tisnik tisnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tisnik tisnik merged commit 566862b into lightspeed-core:main Mar 11, 2026
23 checks passed
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.

2 participants