IUO: featuregates tests should consider multiarch and v1beta1 api#4744
IUO: featuregates tests should consider multiarch and v1beta1 api#4744hmeir wants to merge 1 commit intoRedHatQE:mainfrom
Conversation
📝 WalkthroughWalkthroughThis PR adds multi-architecture cluster detection and refactors feature-gates tests. It introduces an ChangesMulti-architecture and Jira-conditional feature-gates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
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
AI Features
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
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/install_upgrade_operators/conftest.py`:
- Around line 158-160: The fixture cdi_feature_gates assumes
cdi_resource_scope_function.instance.spec.config exists; add a guard that checks
instance and instance.spec and instance.spec.config before accessing it and fail
fast with a clear, contextual error (e.g., using pytest.fail or raising
RuntimeError) if any are None; reference the fixture name cdi_feature_gates and
the accessed symbol cdi_resource_scope_function.instance.spec.config in the
message and include what was expected vs actual (e.g., "expected spec.config
dict, got None for instance <instance identifier or instance>") so callers get
specific context.
🪄 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: 708ff4db-2e1f-4e94-aa3c-667b4f494690
📒 Files selected for processing (5)
tests/conftest.pytests/install_upgrade_operators/conftest.pytests/install_upgrade_operators/constants.pytests/install_upgrade_operators/feature_gates/conftest.pytests/install_upgrade_operators/feature_gates/test_default_featuregates.py
|
/retest all Auto-triggered: Files in this PR were modified by merged PR #4658. Overlapping filestests/install_upgrade_operators/conftest.py |
|
Clean rebase detected — no code changes compared to previous head ( |
1. Use v1beta1. 2. Align with multiarch FG enabled by multiarch deployments. 3. Add jira reference for misconfiguration in deployments. Signed-off-by: Harel Meir <hmeir@redhat.com>
|
Clean rebase detected — no code changes compared to previous head ( |
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/install_upgrade_operators/conftest.py`:
- Around line 292-295: The PR template currently lacks a meaningful "##### What
this PR does / why we need it:" section; update the PR description (or template
used for this repo) to restore that heading and add concrete reviewer-facing
context: state the problem being solved, the behavior change introduced, and why
the change is required (traceability/impact), and ensure this appears for
changes touching tests like the jira_86639_open fixture in
tests/install_upgrade_operators/conftest.py so reviewers can quickly understand
intent and rationale.
🪄 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: 2d923ac0-ff60-4adf-9553-0357d2ad4b1b
📒 Files selected for processing (4)
tests/conftest.pytests/install_upgrade_operators/conftest.pytests/install_upgrade_operators/constants.pytests/install_upgrade_operators/feature_gates/test_default_featuregates.py
|
/verified Ran the whole featuregate suite on multiarch cluster. |
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:
Your deliverable: Test Execution Plan
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: Before setting "Run smoke tests: True", you MUST use shell scripts to: |
| from libs.net.ip import filter_link_local_addresses, random_ipv4_address, random_ipv6_address | ||
| from libs.net.vmspec import lookup_iface_status | ||
| from tests.utils import download_and_extract_tar | ||
| from utilities.architecture import get_cluster_architecture |
There was a problem hiding this comment.
Test Execution Plan
Run smoke tests: False
No concrete dependency path found from any
@pytest.mark.smoketest to the changed fixtures (is_multi_arch_cluster,expected_value,cdi_feature_gates,jira_86639_open).
Tests to run:
-
tests/install_upgrade_operators/feature_gates/test_default_featuregates.py
Full file — test refactored (new fixture-based parametrization viafeaturegates_fixture, newhco_featuregatesfixture, removedresource_object_value_by_key) -
tests/install_upgrade_operators/feature_gates/test_featuregate_reconcile.py::TestHardcodedFeatureGates::test_managed_cr_featuregate_reconcile
Usesexpected_valuefixture (indirect), which now conditionally addsENABLE_MULTI_ARCH_BOOT_IMAGE_IMPORTandDISABLE_MDEV_CONFIGURATIONbased onis_multi_arch_cluster/jira_86639_open
Note: Run on both a multi-arch cluster and a single-arch cluster to validate the conditional logic in
expected_valueforENABLE_MULTI_ARCH_BOOT_IMAGE_IMPORT.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 58030 🏁 Script executed: Length of output: 223 🏁 Script executed: Length of output: 1735 |
Short description:
Fix featuregates failuers
More details:
What this PR does / why we need it:
Green runs on iuo lanes
Which issue(s) this PR fixes:
Special notes for reviewer:
jira-ticket:
https://redhat.atlassian.net/browse/CNV-86648
Summary by CodeRabbit