-
Notifications
You must be signed in to change notification settings - Fork 52
tests, net, service-mesh: Add client arg #2972
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughPublic test resource constructors and fixtures were updated to accept a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (6)📓 Common learnings📚 Learning: 2025-06-18T09:21:34.315ZApplied to files:
📚 Learning: 2025-08-06T13:57:51.928ZApplied to files:
📚 Learning: 2025-09-07T13:16:32.011ZApplied to files:
📚 Learning: 2025-08-06T13:57:34.740ZApplied to files:
📚 Learning: 2025-09-12T08:10:48.874ZApplied to files:
🧬 Code graph analysis (1)tests/network/service_mesh/conftest.py (2)
🪛 Ruff (0.14.7)tests/network/service_mesh/conftest.py223-223: Unused function argument: (ARG001) 🔇 Additional comments (3)
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 |
|
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
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
|
/lgtm |
EdDev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is changing the implicit admin client to an unpriv client. Please express and reason for it.
|
#2972 (review) |
|
/verified Several branches were united into a big branch to test them all at once - #2965 This is the module that needs to be executed in order to verify the current PR: Tested on bm02-ibm Test Result: |
Test execution plan request detailsCRITICAL: You MUST respond with a review comment on the Files Changed tab, NOT as a regular PR comment. As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan. Analysis Requirements:
Your deliverable: Test Execution Plan
Guidelines:
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 17985 @openshift-virtualization-qe-bot-3 Based on my analysis of the PR changes, here is the test execution plan: Test Execution PlanThe changes modify fixture signatures in Tests to Execute:
Rationale:
Note: Anatw has already verified these changes by running the entire Important: I cannot post review comments directly on the Files Changed tab. You'll need to manually create a review comment with this test execution plan on the first line of 🧠 Learnings used |
I do not see it updated. |
Updated class instance and fixtures - all calls to openshift-python-wrapper resource should be updated to pass client. As of its next release, openshift-python-wrapper will enforce passing client when working with cluster resources. openshift-virtualization-tests must align with this change. All calls in the code to openshift-python-wrapper resources should be updated to pass client arg. unprivileged_client is used since all other resources in the service mesh tests already use unprivileged_client (deployments, services, service accounts, VMs), and the Istio resources should follow the same pattern. Signed-off-by: Anat Wax <[email protected]>
17f903d to
70d81a8
Compare
|
/verified Tested on bm02-ibm |
Test execution plan request detailsCRITICAL: You MUST respond with a review comment on the Files Changed tab, NOT as a regular PR comment. As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan. Analysis Requirements:
Your deliverable: Test Execution Plan
Guidelines:
|
|
Tip For best results, initiate chat on the files or code changes. @openshift-virtualization-qe-bot-3 I acknowledge your repeated request for a test execution plan on the Files Changed tab. As noted in my previous response, I cannot post review comments directly on the Files Changed tab through the available API tools. However, I'm pleased to confirm that testing has been completed successfully following my previous recommendation: Test Execution Plan (Previously Provided)Tests to Execute:
Verification Status: ✅ COMPLETED Anatw has verified the changes by running the recommended test module:
The test execution plan I provided has been successfully executed, and the changes have been verified to work correctly. The single failure ( Note: If a review comment on the Files Changed tab is still required for documentation purposes, it will need to be added manually by a maintainer or through a different automation mechanism. 🧠 Learnings used |
EdDev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
/approve
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:latest published |
Short description:
Updated class instance and fixtures - all calls to openshift-python-wrapper resource should be updated to pass client.
What this PR does / why we need it:
As of its next releas, openshift-python-wrapper will enforce passing
clientwhen working with cluster resources.openshift-virtualization-tests must align with this change.
All calls in the code to openshift-python-wrapper resources should be updated to pass
clientarg.All other resources in the service mesh tests already use
unprivileged_client(deployments, services, service accounts, VMs), and the Istio resources should follow the same pattern.jira-ticket:
https://issues.redhat.com/browse/CNV-72392
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.