Skip to content

LCORE-1361: various improvements in integration tests#1197

Merged
tisnik merged 3 commits intolightspeed-core:mainfrom
tisnik:lcore-1361-various-improvements-in-integration-tests
Feb 23, 2026
Merged

LCORE-1361: various improvements in integration tests#1197
tisnik merged 3 commits intolightspeed-core:mainfrom
tisnik:lcore-1361-various-improvements-in-integration-tests

Conversation

@tisnik
Copy link
Contributor

@tisnik tisnik commented Feb 23, 2026

Description

LCORE-1361: various improvements in integration 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
  • Benchmarks improvement

Tools used to create PR

  • Assisted-by: N/A
  • Generated by: N/A

Related Tickets & Documents

  • Related Issue #LCORE-1361

Summary by CodeRabbit

  • Tests
    • Strengthened API endpoint test assertions to validate structured error response details more granularly
    • Enhanced test fixture reliability with more explicit condition checks
    • Improved test coverage tracking with additional pragma directives
    • Refined input payload validation expectations

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 23, 2026

Walkthrough

Multiple test files updated with assertion refinements, condition checks, and coverage directives to align with modified validation logic and test expectations.

Changes

Cohort / File(s) Summary
Exception Assertion Refinements
tests/integration/endpoints/test_model_list.py, tests/integration/endpoints/test_rlsapi_v1_integration.py
Enhanced HTTPException validation by switching from string matching to granular field inspection; adjusted input source expectation from "terminal" to "terminal output".
Conditional Logic & Configuration
tests/integration/test_rh_identity_integration.py, tests/integration/test_configuration.py
Explicit None-check for environment variable restoration logic; stylistic refactor using discard variable assignment.
Coverage & Formatting
tests/integration/test_openapi_json.py, tests/integration/test_version.py
Added coverage pragma for fallback path; reformatted noqa comment placement.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

🚥 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 accurately captures the main focus of the PR: improvements to integration tests across multiple test files with various refinements.
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 docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 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
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
tests/integration/endpoints/test_model_list.py (1)

245-255: Granular assertions are a clear improvement; consider using the status constant for the status code.

The four-assertion structure correctly mirrors the ServiceUnavailableResponse(backend_name="Llama Stack", cause=str(e)) model dump used in models.py. One optional consistency nit: test_rlsapi_v1_integration.py uses status.HTTP_503_SERVICE_UNAVAILABLE for the same check; importing and using it here would keep the test files uniform.

♻️ Optional: use the `status` constant
-from fastapi import Request
+from fastapi import Request, status
-    assert exc_info.value.status_code == 503
+    assert exc_info.value.status_code == status.HTTP_503_SERVICE_UNAVAILABLE
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/integration/endpoints/test_model_list.py` around lines 245 - 255,
Replace the hard-coded status code assertion with the shared status constant:
import status (e.g., from fastapi import status) at the top of
tests/integration/endpoints/test_model_list.py and change the assertion that
checks exc_info.value.status_code == 503 to use
status.HTTP_503_SERVICE_UNAVAILABLE; keep the other assertions intact (they
align with ServiceUnavailableResponse used by models_endpoint_handler).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tests/integration/endpoints/test_model_list.py`:
- Around line 245-255: Replace the hard-coded status code assertion with the
shared status constant: import status (e.g., from fastapi import status) at the
top of tests/integration/endpoints/test_model_list.py and change the assertion
that checks exc_info.value.status_code == 503 to use
status.HTTP_503_SERVICE_UNAVAILABLE; keep the other assertions intact (they
align with ServiceUnavailableResponse used by models_endpoint_handler).
ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fb0389b and 3dfe539.

📒 Files selected for processing (6)
  • tests/integration/endpoints/test_model_list.py
  • tests/integration/endpoints/test_rlsapi_v1_integration.py
  • tests/integration/test_configuration.py
  • tests/integration/test_openapi_json.py
  • tests/integration/test_rh_identity_integration.py
  • tests/integration/test_version.py

@tisnik tisnik merged commit 494220d into lightspeed-core:main Feb 23, 2026
20 of 22 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.

1 participant