docs: add utilities unit test requirements to AGENTS.md#5452
Conversation
|
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
Custom Commands
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. |
📝 WalkthroughWalkthroughUpdates ChangesDocumentation guidance
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
/verified |
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.
Test Execution Plan This PR only modifies
No real/cluster test commands are needed since no session, runtime, or collection-time code was touched. Note: I don't have a capability to post a separate inline review comment outside of this reply thread, so the plan above is provided here in full. |
|
/approve |
|
/retest all Auto-triggered: Files in this PR were modified by merged PR #5440. Overlapping filesAGENTS.md |
a38dcdb to
2618578
Compare
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 `@AGENTS.md`:
- Around line 279-296: Update the “Utilities Unit Test Requirements” opening
rule so tests are required for added or changed testable behavior rather than
unconditionally for every new function. Explicitly state that the exceptions
listed below override this general requirement, while preserving the existing
coverage and regression-test requirements.
🪄 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: d6bb8111-50ec-489c-a7e5-060d0302a0f3
📒 Files selected for processing (1)
AGENTS.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
RedHatQE/openshift-virtualization-tests-design-docs(manual)
📜 Review details
⏰ Context from checks skipped due to timeout. (10)
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: tox
- GitHub Check: verify-bugs-are-open
- GitHub Check: build-container
- GitHub Check: security-suspicious-paths
- GitHub Check: security-committer-identity
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
⚠️ CI failures not shown inline (4)
GitHub Check: can-be-merged: Check if can be merged
Conclusion: failure
Some check runs not started: cnv-smoke/validation, security-suspicious-paths, build-container, verified, security-committer-identity, verify-bugs-are-open, tox
PR has 2 unresolved review conversation(s):
- AGENTS.md:None (outdated) (https://github.com/RedHatQE/openshift-virtualization-tests/pull/5452#discussion_r3536467951)
- AGENTS.md:None (outdated) (https://github.com/RedHatQE/openshift-virtualization-tests/pull/5452#discussion_r3536480178)
Missing lgtm from reviewers. Minimum 2 required, (1 given). Reviewers: dshchedr, myakove, RoniKishner, rnetser, vsibirsk.
GitHub Check: can-be-merged: Check if can be merged
Conclusion: failure
Some required check runs in progress tox, verify-bugs-are-open, build-container
Some check runs not started: cnv-smoke/validation, verified, tox, verify-bugs-are-open, build-container
Missing required labels: tox:verify-tc-requirement-polarion:passed
PR has 2 unresolved review conversation(s):
- AGENTS.md:None (outdated) (https://github.com/RedHatQE/openshift-virtualization-tests/pull/5452#discussion_r3536467951)
- AGENTS.md:None (outdated) (https://github.com/RedHatQE/openshift-virtualization-tests/pull/5452#discussion_r3536480178)
Missing approved from approvers: vsibirsk, rnetser, dshchedr, myakove
Missing lgtm from reviewers. Minimum 2 required, (0 given). Reviewers: RoniKishner, dshchedr, myakove, vsibirsk, rnetser.
GitHub Check: can-be-merged: Check if can be merged
Conclusion: failure
Some check runs not started: cnv-smoke/validation, verified, verify-bugs-are-open, tox, security-suspicious-paths, security-committer-identity, build-container
PR has 2 unresolved review conversation(s):
- AGENTS.md:None (outdated) (https://github.com/RedHatQE/openshift-virtualization-tests/pull/5452#discussion_r3536467951)
- AGENTS.md:None (outdated) (https://github.com/RedHatQE/openshift-virtualization-tests/pull/5452#discussion_r3536480178)
Missing approved from approvers: rnetser, vsibirsk, dshchedr, myakove
Missing lgtm from reviewers. Minimum 2 required, (0 given). Reviewers: vsibirsk, myakove, rnetser, dshchedr, RoniKishner.
GitHub Check: can-be-merged: Check if can be merged
Conclusion: failure
Some check runs not started: security-suspicious-paths, tox, build-container, security-committer-identity, cnv-smoke/validation, verify-bugs-are-open, verified, pre-commit.ci - pr
PR has 2 unresolved review conversation(s):
- AGENTS.md:None (outdated) (https://github.com/RedHatQE/openshift-virtualization-tests/pull/5452#discussion_r3536467951)
- AGENTS.md:None (outdated) (https://github.com/RedHatQE/openshift-virtualization-tests/pull/5452#discussion_r3536480178)
Missing lgtm from reviewers. Minimum 2 required, (1 given). Reviewers: dshchedr, vsibirsk, rnetser, myakove, RoniKishner.
🧰 Additional context used
📓 Path-based instructions (2)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Before writing new code, search existing implementations,utilities/,libs/,tests/, andpyproject.toml; reuse existing logic and never duplicate functionality.
Useuv runfor Python, pip, pytest, tox, and pre-commit commands; never invoke those tools directly.
Before every commit, runuv run pre-commit run --all-files,uv run tox, anduv run tox -e utilities-unittests, and fix all failures without using--no-verify.
Keep pull requests focused, include meaningful required template sections and motivation, use draft status for unresolved blockers, require DCO trailers, and never merge known unresolved issues.
Use@pytest.mark.jira(..., run=False)for confirmed product bugs and@pytest.mark.xfail(..., run=False)for automation, environment, or unidentified issues; do not manually add the auto-addedquarantinedmarker.
Files:
AGENTS.md
**
⚙️ CodeRabbit configuration file
**: ## PR Template Validation
Check the PR description for required sections from.github/pull_request_template.md.
Required sections (must be present, even if empty):
##### What this PR does / why we need it:— MUST be present AND have meaningful content.
Flag as HIGH if the section is missing, empty, whitespace-only, contains only HTML comments,
or contains only placeholder tokens such asTBD,TBA,N/A,-,—,none, or..##### Which issue(s) this PR fixes:— must be present (may be empty)##### Special notes for reviewer:— must be present (may be empty)##### jira-ticket:— must be present (may be empty)
If any required section is absent, orWhat this PR does / why we need it:has no content,
flag it as HIGH severity and ask the author to restore the missing template section(s).Approval Policy
You may approve the PR when ALL of the following are true:
- All your review comments have been addressed with either:
- a code/doc change that fixes the issue, or
- a substantive author response that justifies no code change.
Thread "resolved" state alone is not sufficient.
OR you had no review comments.- If you posted a test execution plan comment requesting tests, and the PR author replied
with a comment explaining why the requested tests are not needed or were already covered,
treat that as an acceptable response — do not block approval on the test plan alone.- The author's explanation must be reasonable and specific (not just "N/A" or "not needed").
Accept explanations like: "these tests were already run in CI", "this change is docs-only",
"the affected tests are quarantined", or "verified manually on cluster X".
Files:
AGENTS.md
🪛 GitHub Check: can-be-merged
AGENTS.md
[error] 1-1: PR has unresolved review conversations (outdated): #5452 (comment)
[error] 1-1: PR has unresolved review conversations (outdated): #5452 (comment)
🪛 LanguageTool
AGENTS.md
[grammar] ~100-~100: Ensure spelling is correct
Context: ...ed tests - Tier marker semantics: tier2 = customer use case tests (default), ti...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~100-~100: Ensure spelling is correct
Context: ...r2 = customer use case tests (default), tier3 = complex/hardware/platform-specific/ti...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (2)
AGENTS.md (2)
54-59: LGTM!Also applies to: 92-95, 97-113, 115-153, 219-226, 228-255, 300-305, 324-324
296-296: 📐 Maintainability & Code QualityHIGH: Verify the PR description template before approval.
The actual PR description is not included in the supplied context. Confirm that it contains all required sections: meaningful
##### What this PR does / why we need it:, plus##### Which issue(s) this PR fixes:,##### Special notes for reviewer:, and##### jira-ticket:.Source: Path instructions
|
/retest all Auto-triggered: Files in this PR were modified by merged PR #5648. Overlapping filesAGENTS.md |
|
/verified |
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.
Test Execution Plan (unchanged — this is now the fourth repeat of an identical request on this documentation-only PR) This PR only modifies
No real/cluster test commands are needed since no session, runtime, or collection-time code was touched. This conclusion is identical to the prior four requests on this PR. Also, as a heads-up: I don't have a capability to post a separate inline review comment outside of this reply thread, so the plan is provided in full here each time. |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:latest published |
What this PR does / why we need it:
Running utilities-unittests only checks that existing tests pass. It does not tell contributors or AI assistants when new utilities unit tests are required. This change help in doing utilities unit test assessment before commit. If we do it before , we could avoid runtime CI failures.
Which issue(s) this PR fixes:
Special notes for reviewer:
I experiment , i add a function in data_utils.py. Without this PR, it doesn’t add a test for newly added function.
After this PR, it adds adds test for it
jira-ticket:
Summary by CodeRabbit
Summary by CodeRabbit
utilities/must be added/updated, where to place them, and when they are not required.utilities/unittests/README.mdto the related documentation references.