Add locator support for OpenShift Virtualization - #2327
Conversation
amolpati30
added
CherryPick
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The XPath for the
bootablecheckbox inComputeResourceOpenShiftProfileStorageItemrelies on a positionaldiv[3]and anonclicksubstring, which is brittle; if possible, prefer a more stable locator such as an explicitidor a less position-dependent path. - The new
openshift_networkfield onBridgeFormis added but not wired into any logic inafter_fillor related helpers; verify whether it should participate in the same branching/selection logic asnetwork_type/networkto avoid being silently ignored.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The XPath for the `bootable` checkbox in `ComputeResourceOpenShiftProfileStorageItem` relies on a positional `div[3]` and an `onclick` substring, which is brittle; if possible, prefer a more stable locator such as an explicit `id` or a less position-dependent path.
- The new `openshift_network` field on `BridgeForm` is added but not wired into any logic in `after_fill` or related helpers; verify whether it should participate in the same branching/selection logic as `network_type`/`network` to avoid being silently ignored.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
LadislavVasina1
left a comment
LadislavVasina1
left a comment
There was a problem hiding this comment.
ACK, looks good, @amolpati30 do you have some automation using this implementation?
I am using those change here - SatelliteQE/robottelo#20850 |
|
@amolpati30 thanks for linking the robottelo PR. |
Gauravtalreja1
left a comment
Gauravtalreja1
left a comment
There was a problem hiding this comment.
Great job @amolpati30 , added a few comments and could you also add locators for ComputeResource page so this PR covers everything related to OCP-V?
| root_password = TextInput(id='host_root_pass') | ||
|
|
||
| @provider_content.register('OpenShift Virtualization') | ||
| class OpenShiftResourceForm(View): |
There was a problem hiding this comment.
just to keep naming consistent with API/Hammer
| class OpenShiftResourceForm(View): | |
| class OCPVComputeResourceForm(View): |
There was a problem hiding this comment.
Taking the name OCPVResource , as per listed cloud resource name.
| # Compute resource attributes | ||
| network_type = FilteredDropdown(id='_compute_attributes_type') | ||
| network = FilteredDropdown(id='_compute_attributes_bridge') | ||
| openshift_network = FilteredDropdown(id='_compute_attributes_network') |
There was a problem hiding this comment.
| openshift_network = FilteredDropdown(id='_compute_attributes_network') | |
| ocpv_network = FilteredDropdown(id='_compute_attributes_network') |
| # Compute resource attributes | ||
| network_type = FilteredDropdown(id='_compute_attributes_type') | ||
| network = FilteredDropdown(id='_compute_attributes_bridge') | ||
| openshift_network = FilteredDropdown(id='_compute_attributes_network') |
There was a problem hiding this comment.
Also expose cni_provider here which would be default to multus
792bf06 to
8a91906
Compare
I have added the locator for the ComputeResource page. Could you please review it? |
8a91906 to
3c43096
Compare
(cherry picked from commit 21a03d0)
(cherry picked from commit 21a03d0)
5 participants
This change introduces locator support for OpenShift Virtualization. With this enhancement, during host creation, the automation framework will dynamically resolve the appropriate UI elements and navigation paths specific to OpenShift Virtualization, ensuring the correct workflow is executed.