Add post-test alerts#5232
Conversation
openshift-virtualization-qe-bot
added
the
tox:verify-tc-requirement-polarion:running
|
Warning Review limit reached
More reviews will be available in 59 minutes and 39 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughA new ChangesPost-test critical alert checker
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsLinked repositories: Your configuration references 1 linked repositories, but your current plan allows 0. Analyzed ``, skipped 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 |
openshift-virtualization-qe-bot
removed
the
tox:verify-tc-requirement-polarion:running
|
D/S test |
openshift-virtualization-qe-bot
added
the
tox:verify-tc-requirement-polarion:failed
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Branch Management
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
AI Features
Security Checks
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/post_test_alerts/test_post_test_alerts.py`:
- Around line 1-7: The module docstring in test_post_test_alerts.py contains
only the Jira key "CNV-80353" instead of a full Jira URL as required by coding
guidelines for traceability. Replace the line "Jira: CNV-80353" with a complete
URL in the format "Jira: https://issues.redhat.com/browse/CNV-80353" to provide
a proper link for coverage tracking and follow the requirement that RFE/Jira
links must be included as actual links in module, class, or test docstrings.
- Around line 48-49: The condition at line 48 only inspects the first element of
alerts_by_name using alerts_by_name[0]["state"], which means if a firing alert
exists in later entries of the list, it will be missed causing false negatives.
Fix this by using a loop or the any() function to check if ANY element in the
alerts_by_name list has state equal to "firing", rather than only checking the
first element at index 0. This ensures all alert samples are inspected to
determine if the alert is actively firing.
🪄 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
Run ID: 2e876e14-e533-4540-9a89-ce379cad38d7
📒 Files selected for processing (3)
conftest.pytests/post_test_alerts/__init__.pytests/post_test_alerts/test_post_test_alerts.py
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
|
/wip cancel |
|
/build-and-push-container |
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
|
/retest tox |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:pr-5232 published |
|
/build-and-push-container |
Verifies 7 critical CNV alerts were not triggered during test execution. Follows the deprecated_api auto-inclusion pattern. Refs: CNV-80353 Signed-off-by: albarker-rh <albarker@redhat.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…fired alerts Signed-off-by: albarker-rh <albarker@redhat.com>
Signed-off-by: albarker-rh <albarker@redhat.com>
Signed-off-by: albarker-rh <albarker@redhat.com>
Signed-off-by: albarker-rh <albarker@redhat.com>
albarker-rh
force-pushed
the
post-test-alerts
branch
from
b5fe3d8 to
b62637f
Compare
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
|
/build-and-push-container |
|
/verified 4.22 |
Test execution plan request detailsCRITICAL: You MUST post an inline review comment on the first changed line of the first file. As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan. Analysis Requirements:
Output rules: Your deliverable: Test Execution Plan
Use these formats:
Real test commands (MANDATORY when changes affect session/runtime code): When the affected code runs at session/collection time (conftest fixtures, pytest plugins,
Example output for a session-startup fix: Guidelines:
Hardware-Related Checks (SR-IOV, GPU, DPDK): When PR modifies fixtures for hardware-specific resources:
CRITICAL WORKFLOW COMPLETION RULES: When responding to this test execution plan request, you MUST follow these rules EXACTLY:
ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):
Remember: The pull request review is visible to users. Additional comments are redundant noise. CRITICAL — Verification rules for smoke and gating decisions: |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
REPO="RedHatQE/openshift-virtualization-tests"
PR_NUMBER="5232"
echo "== Fetch PR files in order =="
gh api "repos/$REPO/pulls/$PR_NUMBER/files?per_page=100" > /tmp/pr_files.json
jq -r '.[].filename' /tmp/pr_files.json
FIRST_FILE=$(jq -r '.[0].filename' /tmp/pr_files.json)
echo "FIRST_FILE=$FIRST_FILE"
echo
echo "== Determine first added line in first file patch =="
PATCH=$(jq -r '.[0].patch // ""' /tmp/pr_files.json)
python - <<'PY' > /tmp/first_line.txt
import json, re, sys
from pathlib import Path
data = json.loads(Path('/tmp/pr_files.json').read_text())
patch = data[0].get('patch') or ''
current_new = None
first_added = None
for raw in patch.splitlines():
if raw.startswith('@@'):
m = re.match(r'^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@', raw)
if m:
current_new = int(m.group(1))
continue
if current_new is None:
continue
if raw.startswith('+') and not raw.startswith('+++'):
first_added = current_new
break
if raw.startswith('-') and not raw.startswith('---'):
continue
current_new += 1
if first_added is None:
raise SystemExit('Could not determine first added line in first file patch')
print(first_added)
PY
FIRST_LINE=$(cat /tmp/first_line.txt)
echo "FIRST_LINE=$FIRST_LINE"
echo
echo "== Latest head commit =="
COMMIT_SHA=$(gh pr view "$PR_NUMBER" --repo "$REPO" --json headRefOid --jq '.headRefOid')
echo "$COMMIT_SHA"
echo
echo "== Outline and inspect changed files =="
ast-grep outline "$FIRST_FILE" --view expanded || true
echo
SECOND_FILE=$(jq -r '.[1].filename // empty' /tmp/pr_files.json)
if [ -n "$SECOND_FILE" ]; then
ast-grep outline "$SECOND_FILE" --view expanded || true
fi
echo
echo "== Targeted conftest evidence =="
rg -n -C3 'post_test_alerts|skip-post-test-alerts|deprecated_api|pytest_configure|pytest_collection_modifyitems|TEAM_MARKERS|file_or_dir|_test_execution_start_time' conftest.py
echo
echo "== New test evidence =="
sed -n '1,220p' tests/post_test_alerts/test_post_test_alerts.py
echo
echo "== Smoke-marked tests (verify actual file paths) =="
rg -n --glob 'tests/**/*.py' '`@pytest`\.mark\.smoke' tests | head -100 || true
echo
echo "== Gating-marked tests (verify actual file paths) =="
rg -n --glob 'tests/**/*.py' '`@pytest`\.mark\.gating' tests | head -100 || true
echo
echo "== Search for direct references to post_test_alerts marker and new CLI option =="
rg -n -C2 'post_test_alerts|skip-post-test-alerts' tests conftest.py || true
COMMENT_FILE=/tmp/test_execution_plan.md
cat > "$COMMENT_FILE" <<'EOF'
## Test Execution Plan
**Run smoke tests: False**
No verified dependency path from any `@pytest.mark.smoke` test to the changed code. The changes are limited to shared collection/session behavior in `conftest.py` plus the new post-test alert check, and no smoke-marked test was traced to this new lane.
**Run gating tests: False**
No verified dependency path from any `@pytest.mark.gating` test to the changed code, and no gating-marked test was traced to the new post-test alert lane.
**Affected tests to run**
- `tests/post_test_alerts/test_post_test_alerts.py::test_no_critical_alerts_after_tests`
- `tests/observability/metrics/test_vms_metrics.py` — representative path to verify the auto-inclusion behavior when running a specific test path
- `tests/post_test_alerts/test_post_test_alerts.py` with `--skip-post-test-alerts` disabled/enabled to verify collection filtering in shared `conftest.py`
**Real tests (cluster required)**
Error/fix path:
```bash
pytest tests/observability/metrics/test_vms_metrics.py --skip-post-test-alerts --collect-onlyExpected: Happy path (regression): pytest tests/observability/metrics/test_vms_metrics.py --collect-onlyExpected: the requested metrics test(s) are collected and End-to-end alert verification: pytest -s -o log_cli=true -m tier2 --jira tests/observability/metrics/test_vms_metrics.pyExpected: workload tests run normally and the post-test alert check runs last, failing only if one of the configured critical CNV alerts fired during the test window. echo echo "$RESP" if [ $STATUS -ne 0 ]; then if [ $STATUS2 -ne 0 ]; then Expected: Happy path (regression): pytest tests/observability/metrics/test_vms_metrics.py --collect-onlyExpected: the requested metrics test(s) are collected and End-to-end alert verification: pytest -s -o log_cli=true -m tier2 --jira tests/observability/metrics/test_vms_metrics.pyExpected: workload tests run normally and the post-test alert check runs last, failing only if one of the configured critical CNV alerts fired during the test window. |
|
/lgtm |
|
/retest all Auto-triggered: Files in this PR were modified by merged PR #5440. Overlapping filesconftest.py |
9 participants
What this PR does / why we need it:
Verifies 7 critical CNV alerts were not triggered during test execution. Follows the deprecated_api auto-inclusion pattern.
Which issue(s) this PR fixes:
We don't check for the above critical alerts firing during test execution
Special notes for reviewer:
jira-ticket:
https://redhat.atlassian.net/browse/CNV-80353
Summary by CodeRabbit
Tests