-
Notifications
You must be signed in to change notification settings - Fork 114
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
Convert perf tuning upgrade scenario to new format #16929
base: master
Are you sure you want to change the base?
Convert perf tuning upgrade scenario to new format #16929
Conversation
trigger: test-robottelo |
PRT Result
|
Copilot
AI
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 suggestion.
Comments skipped due to low confidence (2)
tests/new_upgrades/conftest.py:106
- [nitpick] The variable name
sat_instance
is ambiguous. It should be renamed tosatellite_instance
for clarity.
sat_instance = shared_checkout("perf_tuning_upgrade")
tests/new_upgrades/conftest.py:108
- [nitpick] The variable name
sat_instance
is ambiguous. It should be renamed tosatellite_instance
for clarity.
"perf_tuning_upgrade_tests", shared_checkin, sat_instance=sat_instance
@@ -0,0 +1,97 @@ | |||
"""Test for Performance Tuning related Upgrade Scenario's |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The word 'Scenario's' should be 'Scenarios'.
"""Test for Performance Tuning related Upgrade Scenario's | |
"""Test for Performance Tuning related Upgrade Scenarios |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor comments for ids. Otherwise, this is looking nice
def perf_tuning_upgrade_setup(perf_tuning_upgrade_shared_satellite, upgrade_action): | ||
"""In preupgrade scenario we apply non-default tuning size. | ||
|
||
:id: preupgrade-83404326-20b7-11ea-a370-48f17f1fc2e1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should be able to drop ids for this since it's a fixture now
"""In postupgrade scenario, we verify the set tuning parameters and custom-hiera.yaml | ||
file's content. | ||
|
||
:id: postupgrade-31e26b08-2157-11ea-9223-001a4a1601d8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be best to remove post-upgrade from these as well
:id: postupgrade-31e26b08-2157-11ea-9223-001a4a1601d8 | |
:id: 31e26b08-2157-11ea-9223-001a4a1601d8 |
As part of the current upgrade scenarios refactor, this PR converts the performance tuning scenario to use the
SharedResource
framework, converts the pre-upgrade test to a Pytest fixture, and adds a Satellite upgrade fixture for this scenario.