LCORE-2882: make e2e fail on error or test failure in konflux#2190
LCORE-2882: make e2e fail on error or test failure in konflux#2190radofuchs wants to merge 6 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe integration test pipeline now propagates ChangesIntegration test execution
Estimated code review effort: 1 (Trivial) | ~2 minutes Possibly related PRs
Suggested reviewers: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/e2e/utils/utils.py`:
- Line 268: Update the assertion message in the schema-item matching logic to
avoid interpolating the full HTTP response value from actual. Use a bounded,
sanitized summary or report only the failing index/item, while retaining enough
context to identify the mismatch without exposing sensitive data or producing
oversized CI logs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a3bb65f5-c4e9-4df3-a0a1-c350e863c000
📒 Files selected for processing (1)
tests/e2e/utils/utils.py
📜 Review details
⏰ Context from checks skipped due to timeout. (17)
- GitHub Check: integration_tests (3.13)
- GitHub Check: integration_tests (3.12)
- GitHub Check: unit_tests (3.13)
- GitHub Check: mypy
- GitHub Check: Pylinter
- GitHub Check: Pyright
- GitHub Check: radon
- GitHub Check: build-pr
- GitHub Check: spectral
- GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-7-on-pull-request
- GitHub Check: E2E: library mode / ci / group 1
- GitHub Check: E2E: library mode / ci / group 2
- GitHub Check: E2E: library mode / ci / group 3
- GitHub Check: E2E: server mode / ci / group 1
- GitHub Check: E2E: server mode / ci / group 2
- GitHub Check: E2E: server mode / ci / group 3
- GitHub Check: E2E Tests for Lightspeed Evaluation job
🧰 Additional context used
📓 Path-based instructions (3)
**/*
📄 CodeRabbit inference engine (Custom checks)
**/*: Flag meaningful O(n^2)+ algorithms on non-trivial inputs, including handlers and Kubernetes list operations.
Flag N+1 patterns that list items and then query once per item, including Kubernetes API and database access.
Flag expensive work inside loops, including API calls, JSON parsing, and regex compilation.
Flag unbounded growth in caches, watchers, or buffers when eviction or limits are missing.
Flag missing pagination or limits on list operations and API endpoints.
Flag secrets or tokens logged in plaintext or hardcoded in source.
Flag API endpoints missing authentication or authorization.
Flag injection vulnerabilities, including SQL injection, command injection, and path traversal.
Flag sensitive data leaked in API responses, WebSocket messages, or logs.
Flag Kubernetes Secrets and Red Hat secrets missing OwnerReferences.Never commit secrets or keys; use environment variables for sensitive data.
Files:
tests/e2e/utils/utils.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use absolute imports for internal Python modules.
All modules must begin with descriptive docstrings; package__init__.pyfiles must contain brief package descriptions.
Uselogger = get_logger(__name__)fromlog.pyfor module logging.
UseFinal[type]type hints for constants and keep shared constants in the centralconstants.pymodule with descriptive comments.
Functions must have complete parameter and return type annotations, descriptive snake_case action-oriented names, and docstrings.
Use modern union syntax such asstr | int,Optional[Type]for optional values, andtyping_extensions.Selffor model validators.
Avoid modifying input parameters in place; return a new data structure instead.
Useasync deffor I/O operations and external API calls.
HandleAPIConnectionErrorfrom Llama Stack integrations.
All classes must have descriptive docstrings, PascalCase descriptive names, complete attribute annotations, and specific types instead ofAny.
UseConfigurationBasefor configuration Pydantic models andBaseModelfor other data models.
Use ABC interfaces with@abstractmethoddecorators for abstract classes.
Use@model_validatorand@field_validatorfor custom Pydantic model validation.
Follow Google Python docstring conventions, includingParameters,Returns,Raises, andAttributessections when applicable.
Useuv runfor project commands, including formatting, verification, and tests.
Files:
tests/e2e/utils/utils.py
tests/e2e/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Use behave with BDD step definitions for end-to-end tests.
Files:
tests/e2e/utils/utils.py
🧠 Learnings (1)
📚 Learning: 2026-06-24T13:45:37.249Z
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 1971
File: src/utils/markdown_repair.py:31-36
Timestamp: 2026-06-24T13:45:37.249Z
Learning: In the lightspeed-stack repository, docstrings must use the section header name "Parameters:" (not "Args:") for function arguments, even if the project references Google Python docstring conventions. Ensure docstrings follow the project’s established "Parameters:" header format for any documented function parameters.
Applied to files:
tests/e2e/utils/utils.py
| assert ( | ||
| matched | ||
| ), f"No matching element found in list for schema item {schema_item}" | ||
| ), f"No matching element found in list for schema item {schema_item}, got {actual}" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Avoid logging the full response value in assertion failures.
actual comes from HTTP response data, so embedding the entire list in the error message can leak PII, tokens, or other sensitive fields into CI logs and produce excessively large failure output. Include a bounded, sanitized representation or only the failing index/item instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/e2e/utils/utils.py` at line 268, Update the assertion message in the
schema-item matching logic to avoid interpolating the full HTTP response value
from actual. Use a bounded, sanitized summary or report only the failing
index/item, while retaining enough context to identify the mismatch without
exposing sensitive data or producing oversized CI logs.
Source: Coding guidelines
1 participant
Description
Make integration test fail on konflux if error or test failure happen
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
features/skills.featureas the final entry.