Fix test_negative_pre_upgrade_tuning_profile_check #12578

jameerpathan111 · 2023-09-12T07:44:03Z

Cherry-pick for #12535 😅

Can't provide test results. Tests are marked to be skipped for the stream, as we still don't have upgrade support.

ogajduse · 2023-09-12T08:36:48Z

We can not run PRT here since there is no support for 6.14 -> 6.15 upgrades.

tests/foreman/maintain/test_upgrade.py

Gauravtalreja1 · 2023-09-12T08:56:53Z

tests/foreman/maintain/test_upgrade.py

+    # Change to correct tuning profile (default or medium)
+    custom_host.execute(
+        f'sed -i "s/tuning: development/tuning: {profile}/g" {INSTALLER_CONFIG_FILE}'
+    )


Shouldn't we rerun installer after tuning profile in INSTALLER_CONFIG_FILE? I don't think it'll make any sense for pre-upgrade checks to run if we don't run installer?

I think the point of this test is to check whether the pre-upgrade checks catch the discrepancy. The question is, do the checks really use the config file? I'd suppose they do even though it's not specified explicitly. Otherwise, I think this test would fail on the last assert.

Also, this is just a cherrypick.

yeah, but in reality this should be an main PR and cherrypick got merged #12535, and main PR is called cherrypick here, lol :D

yeah😄, cause I wanted to test the fix, which I can't do with master branch, so I raised it first against 6.14 🤷

and tbh I don't like the current approach of us doing TFA for 6.14 and raising a fix against master branch and then running into a bunch of the issues that we never faced in automation cause we never ran the test with stream(and then trying to fix it for stream so prt can pass :| ).

I don't think it's required to run installer, and like Lukas mentioned the assertion would fail.

tests/foreman/maintain/test_upgrade.py

lhellebr · 2023-09-12T09:10:42Z

tests/foreman/maintain/test_upgrade.py

+    # Change to correct tuning profile (default or medium)
+    custom_host.execute(
+        f'sed -i "s/tuning: development/tuning: {profile}/g" {INSTALLER_CONFIG_FILE}'
+    )


I think the point of this test is to check whether the pre-upgrade checks catch the discrepancy. The question is, do the checks really use the config file? I'd suppose they do even though it's not specified explicitly. Otherwise, I think this test would fail on the last assert.

Also, this is just a cherrypick.

jameerpathan111 · 2023-09-15T08:01:12Z

trigger: test-robottelo
pytest: tests/foreman/maintain/test_upgrade.py -k test_negative_pre_upgrade_tuning_profile_check
env:
    ROBOTTELO_server__deploy_arguments__deploy_sat_version: '6.14.0'
    ROBOTTELO_server__deploy_arguments__deploy_snap_version: '16.0'
    ROBOTTELO_server__version__release: '6.14.0'

lpramuk · 2023-09-18T17:09:22Z

trigger: test-robottelo
pytest: tests/foreman/maintain/test_upgrade.py -k test_negative_pre_upgrade_tuning_profile_check
env:
    ROBOTTELO_server__deploy_arguments__deploy_sat_version: '6.14.0'
    ROBOTTELO_server__deploy_arguments__deploy_snap_version: '16.0'
    ROBOTTELO_server__version__release: '6.14.0'

Why are you testing against 6.14.0 while you choose not to cherrypick to 6.14.z branch ?
I would be for 6.14.z cherrypick

Green negative test in 6.14.z doesn't mean automatically it does its job right 😉

lpramuk · 2023-09-18T17:15:47Z

trigger: test-robottelo
pytest: tests/foreman/maintain/test_upgrade.py -k test_negative_pre_upgrade_tuning_profile_check

lpramuk

Make your changes complete
You switch over to development profile but you are not removing test parametrization [default,medium and also you have to accommodate all assert for {profile} which is now not parametrized

lpramuk · 2023-09-19T11:26:06Z

tests/foreman/maintain/test_upgrade.py

@@ -146,11 +147,16 @@ def test_negative_pre_upgrade_tuning_profile_check(request, custom_host):
    )
    custom_host.download_repofile(product='satellite', release=last_y_stream)
    custom_host.execute('dnf -y module enable satellite:el8 && dnf -y install satellite')
-    # Install without system checks to get around installer checks
+    # Install with development tuning profile to get around installer checks


if you are changing this test to use development profile the you should also handle tests parametrization ids=['default', 'medium'],

and all asserts for {profile}

Funny part is green PRT for this NEGATIVE test - failing on something else :D

We are only using the development profile to get around installer checks for system requirements like RAM and CPU cores. We have to do this in >= 6.14 because --disable-system-checks was removed. This test is still designed to make sure the default and medium profiles enforce their requirements upon an upgrade check.

jameerpathan111 · 2023-09-20T12:08:12Z

trigger: test-robottelo
pytest: tests/foreman/maintain/test_upgrade.py -k test_negative_pre_upgrade_tuning_profile_check
env:
    ROBOTTELO_server__deploy_arguments__deploy_sat_version: '6.14.0'
    ROBOTTELO_server__deploy_arguments__deploy_snap_version: '16.0'
    ROBOTTELO_server__version__release: '6.14.0'
Why are you testing against 6.14.0 while you choose not to cherrypick to 6.14.z branch ? I would be for 6.14.z cherrypick

Green negative test in 6.14.z doesn't mean automatically it does its job right 😉

Because I can't test it with stream as it doesn't have upgrade support and because Gaurav wanted to see how the test would do with the changes he suggested. :D

Griffin-Sullivan · 2023-09-22T15:50:25Z

We have the upgrade path in stream now so we can remove the skip and try on stream if you'd like

Griffin-Sullivan · 2023-10-27T16:50:01Z

trigger: test-robottelo
pytest: tests/foreman/maintain/test_upgrade.py -k test_negative_pre_upgrade_tuning_profile_check

Griffin-Sullivan · 2023-10-27T18:58:31Z

The failure here is due to a bug that @ehelms is working on. Putting this in draft till it's fixed.

Griffin-Sullivan · 2023-11-14T13:42:26Z

trigger: test-robottelo
pytest: tests/foreman/maintain/test_upgrade.py -k test_negative_pre_upgrade_tuning_profile_check

Griffin-Sullivan · 2023-11-14T20:44:10Z

trigger: test-robottelo
pytest: tests/foreman/maintain/test_upgrade.py -k test_negative_pre_upgrade_tuning_profile_check

jameerpathan111 added No-CherryPick PR doesnt need CherryPick to previous branches Stream Introduced in or relating directly to Satellite Stream/Master labels Sep 12, 2023

jameerpathan111 self-assigned this Sep 12, 2023

jameerpathan111 requested review from a team as code owners September 12, 2023 07:44

ogajduse approved these changes Sep 12, 2023

View reviewed changes

Gauravtalreja1 reviewed Sep 12, 2023

View reviewed changes

lhellebr approved these changes Sep 12, 2023

View reviewed changes

jameerpathan111 force-pushed the fix_fm_upgrade_615 branch from 0d20a38 to 5b72a11 Compare September 15, 2023 07:59

lpramuk requested changes Sep 19, 2023

View reviewed changes

lpramuk requested a review from ogajduse September 19, 2023 11:28

Griffin-Sullivan force-pushed the fix_fm_upgrade_615 branch from 5b72a11 to c3b208e Compare October 27, 2023 16:49

Griffin-Sullivan force-pushed the fix_fm_upgrade_615 branch 2 times, most recently from 7d05407 to 953221e Compare October 27, 2023 18:58

Griffin-Sullivan marked this pull request as draft October 27, 2023 18:58

Fix test_negative_pre_upgrade_tuning_profile_check

39c7dec

Griffin-Sullivan force-pushed the fix_fm_upgrade_615 branch from 953221e to 39c7dec Compare November 14, 2023 20:43

Griffin-Sullivan marked this pull request as ready for review November 15, 2023 13:07

JacobCallahan approved these changes Dec 4, 2023

View reviewed changes

JacobCallahan merged commit ca52622 into SatelliteQE:master Dec 4, 2023
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix test_negative_pre_upgrade_tuning_profile_check #12578

Fix test_negative_pre_upgrade_tuning_profile_check #12578

jameerpathan111 commented Sep 12, 2023

ogajduse commented Sep 12, 2023

Gauravtalreja1 Sep 12, 2023

lhellebr Sep 12, 2023

Gauravtalreja1 Sep 13, 2023

jameerpathan111 Sep 14, 2023 •

edited

Loading

jameerpathan111 Sep 14, 2023 •

edited

Loading

jameerpathan111 Sep 14, 2023

lhellebr Sep 12, 2023

jameerpathan111 commented Sep 15, 2023 •

edited

Loading

lpramuk commented Sep 18, 2023 •

edited

Loading

lpramuk commented Sep 18, 2023

lpramuk left a comment

lpramuk Sep 19, 2023

lpramuk Sep 19, 2023

Griffin-Sullivan Sep 22, 2023

jameerpathan111 commented Sep 20, 2023

Griffin-Sullivan commented Sep 22, 2023

Griffin-Sullivan commented Oct 27, 2023

Griffin-Sullivan commented Oct 27, 2023

Griffin-Sullivan commented Nov 14, 2023

Griffin-Sullivan commented Nov 14, 2023

Fix test_negative_pre_upgrade_tuning_profile_check #12578

Fix test_negative_pre_upgrade_tuning_profile_check #12578

Conversation

jameerpathan111 commented Sep 12, 2023

ogajduse commented Sep 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jameerpathan111 Sep 14, 2023 • edited Loading

Choose a reason for hiding this comment

jameerpathan111 Sep 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jameerpathan111 commented Sep 15, 2023 • edited Loading

lpramuk commented Sep 18, 2023 • edited Loading

lpramuk commented Sep 18, 2023

lpramuk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jameerpathan111 commented Sep 20, 2023

Griffin-Sullivan commented Sep 22, 2023

Griffin-Sullivan commented Oct 27, 2023

Griffin-Sullivan commented Oct 27, 2023

Griffin-Sullivan commented Nov 14, 2023

Griffin-Sullivan commented Nov 14, 2023

jameerpathan111 Sep 14, 2023 •

edited

Loading

jameerpathan111 Sep 14, 2023 •

edited

Loading

jameerpathan111 commented Sep 15, 2023 •

edited

Loading

lpramuk commented Sep 18, 2023 •

edited

Loading