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

Adding coverage for BZ2221621 #13694

Merged
merged 1 commit into from
Jan 24, 2024
Merged

Conversation

rmynar
Copy link
Contributor

@rmynar rmynar commented Jan 9, 2024

Added new testcase to cover BZ 2221621 (and it's clone BZ 2238363)

BZ reported in Sat 6.13. Attempting to run installer with invalid certificates on existing installation causes breakage of current setup - hammer ping reports failures. After fix the installer returns error and leaves current setup (hammer ping reports OK statuses)

@rmynar rmynar added CherryPick PR needs CherryPick to previous branches 6.14.z Introduced in or relating directly to Satellite 6.14 6.15.z Introduced in or relating directly to Satellite 6.15 labels Jan 9, 2024
@rmynar rmynar self-assigned this Jan 9, 2024
@rmynar rmynar requested a review from a team as a code owner January 9, 2024 17:48
Copy link
Member

@JacobCallahan JacobCallahan left a comment

Choose a reason for hiding this comment

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

You can also use the InstallerCommand class from robottelo.utils.installer instead of constructing the command by string. This isn't a blocker suggestion, though.
https://github.com/SatelliteQE/robottelo/blob/master/robottelo/utils/installer.py#L4

tests/foreman/destructive/test_installer.py Outdated Show resolved Hide resolved
@rmynar
Copy link
Contributor Author

rmynar commented Jan 10, 2024

trigger: test-robottelo
pytest: tests/foreman/installer/test_installer.py -k 'test_handle_invalid_certificate'

@rmynar rmynar requested a review from JacobCallahan January 10, 2024 15:00
@rmynar
Copy link
Contributor Author

rmynar commented Jan 10, 2024

trigger: test-robottelo
pytest: tests/foreman/destructive/test_installer.py -k 'test_handle_invalid_certificate'

Copy link
Member

@JacobCallahan JacobCallahan left a comment

Choose a reason for hiding this comment

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

very close! one minor change

tests/foreman/destructive/test_installer.py Outdated Show resolved Hide resolved
@rmynar rmynar requested a review from ogajduse January 23, 2024 13:21
@rmynar rmynar added the QETestCoverage Issues and PRs relating to a Satellite bug label Jan 23, 2024
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.

LGTM

Copy link
Member

@ColeHiggins2 ColeHiggins2 left a comment

Choose a reason for hiding this comment

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

ack pending comments

@ogajduse
Copy link
Member

trigger: test-robottelo
pytest: tests/foreman/destructive/test_installer.py -k 'test_handle_invalid_certificate'

Copy link
Contributor

@pondrejk pondrejk left a comment

Choose a reason for hiding this comment

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

small suggestion

tests/foreman/destructive/test_installer.py Outdated Show resolved Hide resolved
Copy link
Member

@ogajduse ogajduse left a comment

Choose a reason for hiding this comment

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

PRT passed, and comments were addressed. Merging

@ogajduse ogajduse merged commit ff8b7f6 into SatelliteQE:master Jan 24, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.14.z Introduced in or relating directly to Satellite 6.14 6.15.z Introduced in or relating directly to Satellite 6.15 CherryPick PR needs CherryPick to previous branches QETestCoverage Issues and PRs relating to a Satellite bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants