Harmonize the main.pm files further to DRY #4432

okurz · 2018-02-18T18:51:19Z

DRY on openSUSE/SLE load_ssh_key_import_tests
DRY on openSUSE/SLE load_inst_tests
DRY on openSUSE/SLE load_console_tests
DRY on openSUSE/SLE load_x11_tests
DRY on openSUSE/SLE x11regression tests

The impact of the change should be only on the schedule so I mainly compared
the evaluated schedule rather than look into the results from the test
modules. Mainly the change is a refactoring with no visible impact on actual
test execution. However, in some cases the order of modules execution was
changed for either SLE or openSUSE to harmonize both.

Verification runs:

openSUSE staging cryptlvm: http://lord.arch/tests/596
openSUSE TW textmode: http://lord.arch/tests/601
openSUSE TW textmode: http://lord.arch/tests/591
openSUSE TW gnome: http://lord.arch/tests/595
openSUSE TW kde: http://lord.arch/tests/603
openSUSE TW lxde: http://lord.arch/tests/593
openSUSE TW lxde: http://lord.arch/tests/598
openSUSE Leap 42.3 maintenance gnome: http://lord.arch/tests/589
SLE staging gnome: http://lord.arch/tests/597

Related progress issue: https://progress.opensuse.org/issues/15132

okurz · 2018-02-19T09:40:51Z

products/caasp/main.pm

@@ -43,7 +43,7 @@ if (check_var('VIRSH_VMM_FAMILY', 'xen') && check_var('VIRSH_VMM_TYPE', 'linux')
    set_var('SERIALDEV', 'hvc0');
 }

-sub load_boot_tests {
+sub load_kubic_boot_tests {


@kravciak are you fine with this rename to ensure we can use "load_boot_tests" to use that for the "common" ones or would you recommend to also rename the openSUSE/SLE ones?

If rename helps you then I don't care, but I don't see the need for it - function is overloaded in caasp/main.pm and it does not conflict with SLE/main_common - or did I miss something?

Well, I don't know how the overloading should work but perl reports an error (or at least a warning?) about the method in products/caasp/main.pm this is why I changed it.

I see, then feel free to change it, but please don't use kubic naming scheme - it's a bit misleading in caasp/main.pm. What about:

load_boot_tests -> caasp_boot_tests load_inst_tests -> load_oci_tests || caasp_inst_tests

okurz · 2018-02-19T09:45:52Z

lib/main_common.pm

+
+# TODO check why we want this enabled on SLE staging but not openSUSE staging
+sub packagekit_available {
+    return !check_var('FLAVOR', 'Rescue-CD') && (!is_staging || is_sle);


@DimStar77 @lnussel this differs between SLE and openSUSE and causes the "glxgears" test to not run on openSUSE staging tests but on SLE. "glxgears" is important for SLE because it is the only packagekit based install test in staging tests. Any reason why we should not enable this on openSUSE as well?

Generally speaking, we should be able to cope with glxgears in Stagings (Staging2 only, or Mesa-demo would need to move to Ring1) - at least for TW I'll have to add Mesa-demo to the test DVD, but that cost is acceptable.

Done in #4443 , based on this PR here. http://lord.arch/tests/606#step/glxgears/12 shows Mesa-demo-x missing. Can you add it?

asmorodskyi · 2018-02-20T07:00:35Z

lib/main_common.pm

+    return (check_var("FLAVOR", 'Rescue-CD') || get_var("LIVETEST"));
+}
+
+sub is_kde_live {


shouldn't we keep everything which calculate version in version_utils ?

After discussing with you in person I moved some functions into version_utils as well as utils but not this one because it is only used in this file here and also not exported.

asmorodskyi · 2018-02-20T07:11:54Z

lib/main_common.pm

+    # with SLE 15 LeanOS only the default is textmode
+    return 'gnome' if get_var('BASE_VERSION', '') =~ /^12/;
+    return 'textmode' if (get_var('SYSTEM_ROLE') && !check_var('SYSTEM_ROLE', 'default'));
+    return 'gnome' if is_desktop_module_selected;


most probably it works as expected after all but just FYI that is_sles4sap defined as positive condition in both is_desktop_module_selected and is_server. So switching lines 302 and 300 will bring different results for SAP guys

do you think me just moving it to a different file should have an effect like this?

don't things , my comment regarding version_utils not related to this. actually after discussing with you in person I can say that you can ignore this comment because it is just code which you move from one place to another.

asmorodskyi · 2018-02-21T08:51:42Z

make travis green and I will merge

The "default" tests are the ones that are called in a hidden "else" branch when all other test variables do not evaluate to another explicit test schedule. This should be more apparent.

* DRY on openSUSE/SLE load_ssh_key_import_tests * DRY on openSUSE/SLE load_inst_tests * DRY on openSUSE/SLE load_console_tests * DRY on openSUSE/SLE load_x11_tests * DRY on openSUSE/SLE x11regression tests The impact of the change should be only on the schedule so I mainly compared the evaluated schedule rather than look into the results from the test modules. Mainly the change is a refactoring with no visible impact on actual test execution. However, in some cases the order of modules execution was changed for either SLE or openSUSE to harmonize both. Verification runs: * openSUSE staging cryptlvm: http://lord.arch/tests/596 * openSUSE TW textmode: http://lord.arch/tests/601 * openSUSE TW textmode: http://lord.arch/tests/591 * openSUSE TW gnome: http://lord.arch/tests/595 * openSUSE TW kde: http://lord.arch/tests/603 * openSUSE TW lxde: http://lord.arch/tests/593 * openSUSE TW lxde: http://lord.arch/tests/598 * openSUSE Leap 42.3 maintenance gnome: http://lord.arch/tests/589 * SLE staging gnome: http://lord.arch/tests/597 Related progress issue: https://progress.opensuse.org/issues/15132

lib/main_common.pm is meant as a common file for the test schedules. Therefore functions from this file should not be imported by test modules. This commit moves the corresponding functions to better places, e.g. `version_utils` or `utils`.

…st#4432)

Fix order of setup_online_repos and timezone (regression in #4432)

…utoinst#4432) See https://openqa.opensuse.org/tests/615862#step/hostname_inst/2 Seems like hostname_inst was never tried on openSUSE.

Fix wrong scheduling of hostname_inst on openSUSE (regression in #4432)

Fix wrong scheduling of chromium on SLE (regression in #4432)

…st#4432

alvarocarvajald · 2018-02-22T09:58:58Z

products/sle/main.pm

 use strict;
 use warnings;
 use testapi qw(check_var get_var get_required_var set_var check_var_array diag);
 use lockapi;
 use needle;
 use registration;
 use utils;
-use version_utils qw(is_hyperv_in_gui is_caasp is_installcheck is_rescuesystem sle_version_at_least is_desktop_installed is_jeos is_sle is_upgrade);
+use version_utils qw(is_hyperv_in_gui is_caasp is_installcheck is_rescuesystem sle_version_at_least is_desktop_installed is_jeos is_sle is_staging is_upgrade);


Shouldn't is_sles4sap() and is_sles4sap_standard() be also included in this line?

Failing test: https://openqa.suse.de/tests/1493885/file/autoinst-log.txt

OK. Saw that is_sles4sap is being exported in lib/main_common, so I guess the fix should be to add is_sles4sap_standard also there.

alvarocarvajald · 2018-02-22T10:16:50Z

lib/main_common.pm

+        {
+            loadtest "installation/system_role";
+        }
+        if (is_sles4sap() and sle_version_at_least('15')) {


Why was and check_var('SYSTEM_ROLE', 'default') dropped from the condition? In SLES4SAP, when any of the SLES System Roles are selected, the installer goes directly to partitioning instead of the SLES4SAP Product Installation Mode screen.

Faling test: https://openqa.suse.de/tests/1492176#step/sles4sap_product_installation_mode/2

…st#4432

Workaround bsc#1070233 for TW as well after we hit it after #4432

…st#4432)

…utoinst#4432) See https://openqa.opensuse.org/tests/615862#step/hostname_inst/2 Seems like hostname_inst was never tried on openSUSE.

…st#4432

okurz commented Feb 19, 2018

View reviewed changes

okurz force-pushed the feature/next branch from 4b1c3d7 to 3b6c679 Compare February 19, 2018 13:44

okurz mentioned this pull request Feb 20, 2018

Execute glxgears for openSUSE staging tests to be in line with SLE #4443

Merged

asmorodskyi reviewed Feb 20, 2018

View reviewed changes

okurz force-pushed the feature/next branch 2 times, most recently from 9be1504 to a16d808 Compare February 21, 2018 14:09

okurz added 5 commits February 21, 2018 15:21

DRY on openSUSE/SLE load_boot_tests

6cfc86d

DRY on openSUSE/SLE load_reboot_tests

87c3a38

Make the "default" tests more obvious

5d83981

The "default" tests are the ones that are called in a hidden "else" branch when all other test variables do not evaluate to another explicit test schedule. This should be more apparent.

okurz force-pushed the feature/next branch from a16d808 to 7a90832 Compare February 21, 2018 16:08

asmorodskyi merged commit ea79647 into os-autoinst:master Feb 21, 2018

okurz added a commit to okurz/os-autoinst-distri-opensuse that referenced this pull request Feb 21, 2018

Fix order of setup_online_repos and timezone (regression in os-autoin…

b28f7ac

…st#4432)

okurz deleted the feature/next branch February 21, 2018 17:51

okurz added a commit that referenced this pull request Feb 21, 2018

Merge pull request #4466 from okurz/fix/setup_online_repos

a2f25ef

Fix order of setup_online_repos and timezone (regression in #4432)

okurz added a commit that referenced this pull request Feb 21, 2018

Merge pull request #4467 from okurz/fix/hostname_inst

ab3f3c0

Fix wrong scheduling of hostname_inst on openSUSE (regression in #4432)

okurz added a commit to okurz/os-autoinst-distri-opensuse that referenced this pull request Feb 22, 2018

Fix wrong scheduling of chromium on SLE (regression in os-autoinst#4432)

17c21ca

okurz added a commit that referenced this pull request Feb 22, 2018

Merge pull request #4469 from okurz/fix/chromium

880c03a

Fix wrong scheduling of chromium on SLE (regression in #4432)

okurz added a commit to okurz/os-autoinst-distri-opensuse that referenced this pull request Feb 22, 2018

Workaround bsc#1070233 for TW as well after we hit it after os-autoin…

6fc8a01

…st#4432

okurz added a commit to okurz/os-autoinst-distri-opensuse that referenced this pull request Feb 22, 2018

Workaround bsc#1070233 for TW as well after we hit it after os-autoin…

95324a0

…st#4432

alvarocarvajald reviewed Feb 22, 2018

View reviewed changes

okurz added a commit to okurz/os-autoinst-distri-opensuse that referenced this pull request Feb 22, 2018

Workaround bsc#1070233 for TW as well after we hit it after os-autoin…

8e9b151

…st#4432

alvarocarvajald mentioned this pull request Feb 22, 2018

Fix calls to sles4sap methods in main and main_common #4474

Merged

SergioAtSUSE mentioned this pull request Feb 22, 2018

Do not load inexistent qa_net.pm #4476

Closed

okurz added a commit to okurz/os-autoinst-distri-opensuse that referenced this pull request Feb 22, 2018

Workaround bsc#1070233 for TW as well after we hit it after os-autoin…

28af263

…st#4432

okurz added a commit that referenced this pull request Feb 22, 2018

Merge pull request #4470 from okurz/fix/disable_grub_timeout_upgrade

6810476

Workaround bsc#1070233 for TW as well after we hit it after #4432

nicksinger mentioned this pull request Feb 28, 2018

Fix scheduling regression for EXIT_AFTER_START_INSTALL #4521

Merged

StefanBruens pushed a commit to StefanBruens/os-autoinst-distri-opensuse that referenced this pull request Mar 1, 2018

Fix order of setup_online_repos and timezone (regression in os-autoin…

d9ca624

…st#4432)

StefanBruens pushed a commit to StefanBruens/os-autoinst-distri-opensuse that referenced this pull request Mar 1, 2018

Fix wrong scheduling of chromium on SLE (regression in os-autoinst#4432)

88ae9f6

StefanBruens pushed a commit to StefanBruens/os-autoinst-distri-opensuse that referenced this pull request Mar 1, 2018

Workaround bsc#1070233 for TW as well after we hit it after os-autoin…

2e22f3f

…st#4432

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Harmonize the main.pm files further to DRY #4432

Harmonize the main.pm files further to DRY #4432

okurz commented Feb 18, 2018

okurz Feb 19, 2018

kravciak Feb 19, 2018

okurz Feb 19, 2018

kravciak Feb 21, 2018

okurz Feb 19, 2018

DimStar77 Feb 19, 2018

okurz Feb 20, 2018

asmorodskyi Feb 20, 2018

okurz Feb 21, 2018

asmorodskyi Feb 20, 2018

okurz Feb 21, 2018

asmorodskyi Feb 21, 2018

asmorodskyi commented Feb 21, 2018

alvarocarvajald Feb 22, 2018

alvarocarvajald Feb 22, 2018

alvarocarvajald Feb 22, 2018

alvarocarvajald Feb 22, 2018

alvarocarvajald Feb 22, 2018

Harmonize the main.pm files further to DRY #4432

Harmonize the main.pm files further to DRY #4432

Conversation

okurz commented Feb 18, 2018

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

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

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

asmorodskyi commented Feb 21, 2018

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

Choose a reason for hiding this comment