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.

Sign up for GitHub

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

Jump to bottom

variable value should del after update #16969

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

variable value should del after update #16969

wants to merge 1 commit into from

Conversation

amolpati30
Copy link
Contributor

@amolpati30 amolpati30 commented Nov 20, 2024 •
edited
Loading

I extended the existing test case to include the following steps:

  1. Create an Ansible variable with a default value and an overridden key.
  2. Update the value of the Ansible variable at the host level.
  3. After updating, delete the value, ensuring that it reverts to the default value.

Dependent PR: SatelliteQE/airgun#1643

@amolpati30 amolpati30 requested review from a team as code owners November 20, 2024 15:52
@amolpati30 amolpati30 changed the title success > Succeeded status change in host table (#16952) variable value should del after update Nov 20, 2024
@amolpati30 amolpati30 force-pushed the update_ansiblevar branch 2 times, most recently from 86675f4 to 43bc163 Compare November 20, 2024 16:12
@amolpati30 amolpati30 added AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing 6.13.z Introduced in or relating directly to Satellite 6.13 6.14.z Introduced in or relating directly to Satellite 6.14 6.15.z Introduced in or relating directly to Satellite 6.15 6.16.z Introduced in or relating directly to Satellite 6.16 CherryPick PR needs CherryPick to previous branches labels Nov 20, 2024
@amolpati30 amolpati30 mentioned this pull request Nov 20, 2024
Open
@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 9422
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 817.03s (0:13:37) ===========

@Satellite-QE Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Nov 20, 2024
shweta83
shweta83 reviewed Nov 22, 2024
View reviewed changes
tests/foreman/ui/test_ansible.py Outdated
:customerscenario: true

:steps:
1. Create an Ansible variable with array type and set the default value.
2. Enable both 'Merge Overrides' and 'Merge Default'.
3. Add the variable to a role and attach the role to the host.
4. Verify that ansible role and variable is added to the host.
5. In variable override the value.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
5. In variable override the value.
5. Override the variable value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

shweta83
shweta83 reviewed Nov 22, 2024
View reviewed changes
tests/foreman/ui/test_ansible.py Outdated
:customerscenario: true

:steps:
1. Create an Ansible variable with array type and set the default value.
2. Enable both 'Merge Overrides' and 'Merge Default'.
3. Add the variable to a role and attach the role to the host.
4. Verify that ansible role and variable is added to the host.
5. In variable override the value.
6. Delete the overridden value.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
6. Delete the overridden value.
6. Reset the overridden value to the original value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

shweta83
shweta83 reviewed Nov 22, 2024
View reviewed changes
tests/foreman/ui/test_ansible.py
'default_value': default_value,
}
)
new_value = '["test update"]'
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't this give you error?
You can try new_value = '["test update"]'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It did not produce any errors, but I made a small change.

shweta83
shweta83 reviewed Nov 22, 2024
View reviewed changes
tests/foreman/ui/test_ansible.py Outdated
)
assert new_value in update_value
# Revert the updated value to its default state.
delete_value = session.host_new.read_value_after_del(rhel_contenthost.hostname, new_key)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please rename this variable? The name is confusing.
may be reset_variable_value ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated the variable name.

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 9443
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 793.28s (0:13:13) ===========

@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
@amolpati30 amolpati30 force-pushed the update_ansiblevar branch 3 times, most recently from 49ca48b to 37f40d7 Compare November 22, 2024 07:32
@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 9446
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 933.30s (0:15:33) ===========

@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
lhellebr
lhellebr approved these changes Nov 28, 2024
View reviewed changes
Copy link
Contributor

@lhellebr lhellebr left a comment

Choose a reason for hiding this comment

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

I have a question in the dependence PR, this PR itself LGTM

@Satellite-QE Satellite-QE removed the PRT-Passed Indicates that latest PRT run is passed for the PR label Nov 29, 2024
@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 9480
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 843.76s (0:14:03) ===========

@Satellite-QE Satellite-QE added the PRT-Failed Indicates that latest PRT run is failed for the PR label 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
airgun: 1643

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 9482
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 1629.27s (0:27:09) ==========

@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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LadislavVasina1 LadislavVasina1 LadislavVasina1 approved these changes

@synkd synkd synkd approved these changes

@ColeHiggins2 ColeHiggins2 ColeHiggins2 approved these changes

@lhellebr lhellebr lhellebr approved these changes

@shweta83 shweta83 Awaiting requested review from shweta83

Assignees

@amolpati30 amolpati30

Labels
6.13.z Introduced in or relating directly to Satellite 6.13 6.14.z Introduced in or relating directly to Satellite 6.14 6.15.z Introduced in or relating directly to Satellite 6.15 6.16.z Introduced in or relating directly to Satellite 6.16 AutoMerge_Cherry_Picked The cherrypicked PRs of master PR would be automerged if all checks passing CherryPick PR needs CherryPick to previous branches PRT-Passed Indicates that latest PRT run is passed for the PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@amolpati30 @Satellite-QE @lhellebr @ColeHiggins2 @shweta83 @synkd @LadislavVasina1