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

Fix Upgrade scenarios #16477

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

Fix Upgrade scenarios #16477

wants to merge 1 commit into from

Conversation

shweta83
Copy link
Contributor

@shweta83 shweta83 commented Sep 24, 2024 •
edited
Loading

Problem Statement

Provisioning Template and Foreman Discovery Upgrade Scenario tests are failing with JSON parse error.

Solution

  • Updated test_pre_scenario_provisioning_templates to save hostname instead of host id in pre_upgrade_data file.
  • Saving FDI version in string format in test_pre_upgrade_fdi_version as Version type data is not supported to be stored.

Related Issues

@shweta83 shweta83 requested a review from a team as a code owner September 24, 2024 06:28
@shweta83 shweta83 marked this pull request as draft September 24, 2024 07:43
@shweta83 shweta83 force-pushed the upgrade_scenarios_test_fix branch 2 times, most recently from 12edf99 to 60ac9ff Compare September 25, 2024 08:42
@shweta83 shweta83 marked this pull request as ready for review September 25, 2024 08:42
@shweta83 shweta83 added CherryPick PR needs CherryPick to previous branches 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 TestFailure Issues and PRs related to a test failing in automation labels Sep 25, 2024
@shweta83
Copy link
Contributor Author

trigger: test-robottelo
pytest: tests/upgrades/test_discovery.py -k test_pre_upgrade_fdi_version -m pre_upgrade

@Satellite-QE
Copy link
Collaborator

PRT Result

Build Number: 8788
Build Status: SUCCESS
PRT Comment: pytest tests/upgrades/test_discovery.py -k test_pre_upgrade_fdi_version -m pre_upgrade --external-logging
Test Result : =========== 1 passed, 1 deselected, 10 warnings in 898.68s (0:14:58) ===========

@Satellite-QE Satellite-QE added the PRT-Passed Indicates that latest PRT run is passed for the PR label Sep 25, 2024
Gauravtalreja1
Gauravtalreja1 reviewed Sep 25, 2024
View reviewed changes
tests/upgrades/test_discovery.py
@@ -60,5 +60,5 @@ def test_post_upgrade_fdi_version(self, target_sat, pre_upgrade_data):
fdi_package = target_sat.execute('rpm -qa *foreman-discovery-image*').stdout
# Note: The regular exp takes care of format digit.digit.digit or digit.digit.digit-digit in the output
post_upgrade_version = Version(re.search(r'\d+\.\d+\.\d+(-\d+)?', fdi_package).group())
assert post_upgrade_version >= pre_upgrade_version
assert str(post_upgrade_version) >= pre_upgrade_version
Copy link
Collaborator

Choose a reason for hiding this comment

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

comparing versions here make more sense here, so just would like to know why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we dictionary doesn't accept Version so changing it to str.

Gauravtalreja1
Gauravtalreja1 reviewed Sep 25, 2024
View reviewed changes
tests/upgrades/test_provisioningtemplate.py
pre_update_data_dict = {
'provision_host_id': host.id,
'provision_host_name': host.name,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does host.id change after upgrade?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Host.name would be more reliable here. Id doesn't change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Gauravtalreja1 Gauravtalreja1 Gauravtalreja1 left review comments

@pondrejk pondrejk pondrejk approved these changes

Assignees

@shweta83 shweta83

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 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 TestFailure Issues and PRs related to a test failing in automation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@shweta83 @Satellite-QE @pondrejk @Gauravtalreja1