Skip to content
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

removing updated value and takes default value #1643

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

amolpati30
Copy link
Contributor

@amolpati30 amolpati30 commented Nov 20, 2024

Added to the entities with their respective locators:

  • update_variable_value: Updating the ansible variable value in the host.
  • del_variable_value: Deletes the updated value, reverts to the default value.
  • read_variable_value: Read the value of ansible variable in the host.

Dependent PR: SatelliteQE/robottelo#16969

@amolpati30 amolpati30 added CherryPick PR needs CherryPick to previous branches 6.13.z AutoMerge_Cherry_Picked Automatically merge the PR is PRT and all checks are passing 6.14.z 6.15.z 6.16.z labels Nov 20, 2024
@Satellite-QE
Copy link
Contributor

PRT Result

Build Number: 460
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/ui/test_ansible.py::TestAnsibleCfgMgmt -k test_positive_assign_ansible_role_variable_on_host --external-logging
Test Result : ========== 1 passed, 10 deselected, 24 warnings in 837.50s (0:13:57) ===========

@Satellite-QE Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Nov 20, 2024
@amolpati30 amolpati30 changed the title removing updated value with default removing updated value and takes default value Nov 20, 2024
Copy link
Contributor

@LadislavVasina1 LadislavVasina1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small change requested, otherwise looks good to me. GJ @amolpati30!

@Satellite-QE
Copy link
Contributor

PRT Result

Build Number: 461
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/ui/test_ansible.py::TestAnsibleCfgMgmt -k test_positive_assign_ansible_role_variable_on_host --external-logging
Test Result : ========== 1 failed, 10 deselected, 22 warnings in 800.28s (0:13:20) ===========

@Satellite-QE Satellite-QE added PRT-Failed Indicates that latest PRT run is failed for the PR and removed PRT-Passed Indicates that latest PRT run is passed for the PR labels Nov 22, 2024
@Satellite-QE
Copy link
Contributor

PRT Result

Build Number: 462
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/ui/test_ansible.py::TestAnsibleCfgMgmt -k test_positive_assign_ansible_role_variable_on_host --external-logging
Test Result : ========== 1 passed, 10 deselected, 24 warnings in 892.06s (0:14:52) ===========

@Satellite-QE Satellite-QE added PRT-Passed Indicates that latest PRT run is passed for the PR and removed PRT-Failed Indicates that latest PRT run is failed for the PR labels Nov 22, 2024
Copy link
Contributor

@LadislavVasina1 LadislavVasina1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK

@lhellebr
Copy link
Contributor

Is there a reason why each of these functions isn't an atomic function? Is it important to read RIGHT AFTER updating/deleting?

@amolpati30
Copy link
Contributor Author

@lhellebr
While working on this entity, I noticed that some features do not have a specific column for each actions item in the table. For example, actions like Submit, Cancel, and Edit point to a single column (indexed as 5). However, when we update a value and attempt to delete it, a new "3-dot" option appears, which does not correspond to any specific column, even though it is part of the table.
Due to these differences, two distinct methods are required. And I am reading the values here for assertion purposes, to verify the data after update and delete.

@lhellebr
Copy link
Contributor

Let me rephrase my question. Let's take one of the functions: read_value_after_del. Why is it a single function? What is so specific about operation "delete and then read"? I would think this can be two functions: delete and read and the test can just call one after the other.

@amolpati30
Copy link
Contributor Author

amolpati30 commented Nov 28, 2024

Thank you for the clarification. I would suggest that if we are deleting/updating value, we should read the value for verification/assertion purposes. I have already added this functionality in the method. However, creating a separate method specifically for reading the value is also a good idea.

This way, we could have two methods for update/delete operations and a third method to handle reading the value. Do you think I should update the implementation accordingly?

@lhellebr
Copy link
Contributor

I think yes, if you don't have a specific reason to make those inherently atomic operations a single operation in this case. I think that's also the way we have it everywhere else.

@Satellite-QE
Copy link
Contributor

PRT Result

Build Number: 463
Build Status: UNSTABLE
PRT Comment: pytest tests/foreman/ui/test_ansible.py::TestAnsibleCfgMgmt -k test_positive_assign_ansible_role_variable_on_host --external-logging
Test Result : ========== 1 failed, 10 deselected, 26 warnings in 769.00s (0:12:49) ===========

@Satellite-QE Satellite-QE added PRT-Failed Indicates that latest PRT run is failed for the PR and removed PRT-Passed Indicates that latest PRT run is passed for the PR labels Nov 29, 2024
@amolpati30
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/foreman/ui/test_ansible.py::TestAnsibleCfgMgmt -k test_positive_assign_ansible_role_variable_on_host
robottelo: 16969

@Satellite-QE
Copy link
Contributor

PRT Result

Build Number: 464
Build Status: SUCCESS
PRT Comment: pytest tests/foreman/ui/test_ansible.py::TestAnsibleCfgMgmt -k test_positive_assign_ansible_role_variable_on_host --external-logging
Test Result : ========== 1 passed, 10 deselected, 26 warnings in 1420.64s (0:23:40) ==========

@Satellite-QE Satellite-QE added PRT-Passed Indicates that latest PRT run is passed for the PR and removed PRT-Failed Indicates that latest PRT run is failed for the PR labels Nov 29, 2024
Comment on lines +530 to +533
6: Button(locator='.//button[@aria-label="Cancel editing override button"]'),
7: Button(locator='.//button[@aria-label="Submit override button"]'),
# Clicking this button hides it, and displays the previous 2
7: Button(locator='.//button[@aria-label="Edit override button"]'),
5: Button(locator='.//button[@aria-label="Edit override button"]'),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why changing the buttons order here?
and could you add some descriptions about change to this locators in the description?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Locators 6 and 7 are not supported, and there is no column for the edit, submit, or cancel buttons. Based on the index, these buttons correspond to key 5. To enable editing, I added the edit locator under key 5. For submitting the value, I created another variable with the same key (5) but for the submit action. However, combining them does not work—I tried that as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.13.z 6.14.z 6.15.z 6.16.z AutoMerge_Cherry_Picked Automatically merge the PR is PRT and all checks are passing CherryPick PR needs CherryPick to previous branches PRT-Passed Indicates that latest PRT run is passed for the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants