revise storage: better test loading, shared disk selection

Summary:
This contains several tweaks to storage handling. It adds a
method for disk selection which all the storage tests can
share. It sets up a more extensible approach for main.pm to
run the storage tests, instead of an ever-growing forest of
'else' clauses. Finally it sets up a couple of methods for
changing partitioning schemes on the custom part screen and
uses one of them in the software RAID test; the other will
be used for other custom storage tests.

This kills the two_disks needle. I could keep it and work
it into select_disks, but it doesn't fit naturally and I
really just don't see the point of the needle. The only thing
we lose is we don't check that anaconda actually sees two
disks in the 'attach two disks, only install to one' test
(that's server_sata_multi), but the other multi-disk tests
will serve to catch that case failing for some reason.

What I actually intended to do was add some more tests for
different custom part storage types, but it seemed a good
idea to do some of this cleanup so that can be implemented
efficiently. I'll have followups for that.

Test Plan:
Run all tests and ensure they work exactly as
before (not just that they still pass, but that the correct
test steps are actually scheduled in each case.)

Reviewers: garretraziel, jskladan

Reviewed By: garretraziel, jskladan

Subscribers: tflink

Differential Revision: https://phab.qadevel.cloud.fedoraproject.org/D475

lib/anacondalog.pm

@@ -57,6 +57,53 @@ sub root_console {
     $self->console_login(user=>"root",check=>$args{check});
 }
 
+sub select_disks {
+    my ($self, $disks) = @_;
+    $disks ||= 1;
+    # Anaconda hub
+    assert_screen "anaconda_main_hub", 300; #
+    # Damn animation delay can cause bad clicks here too - wait for it
+    sleep 1;
+    assert_and_click "anaconda_main_hub_install_destination";
+
+    if (get_var('NUMDISKS') > 1) {
+        # Multi-disk case. Select however many disks the test needs. If
+        # $disks is 0, this will do nothing, and 0 disks will be selected.
+        for my $n (1 .. $disks) {
+            assert_and_click "anaconda_install_destination_select_disk_$n";
+        }
+    }
+    else {
+        # Single disk case.
+        if ($disks == 0) {
+            # Clicking will *de*-select.
+            assert_and_click "anaconda_install_destination_select_disk_1";
+        }
+        elsif ($disks > 1) {
+            die "Only one disk is connected! Cannot select $disks disks.";
+        }
+        # For exactly 1 disk, we don't need to do anything.
+    }
+
+    if (get_var('DISK_CUSTOM')) {
+        assert_and_click "anaconda_manual_partitioning";
+    }
+}
+
+sub custom_scheme_select {
+    my ($self, $scheme) = @_;
+    assert_and_click "anaconda_part_scheme";
+    assert_and_click "anaconda_part_scheme_$scheme";
+}
+
+sub custom_change_type {
+    my ($self, $type) = @_;
+    # We assume we start off with / selected.
+    assert_and_click "anaconda_part_device_type";
+    assert_and_click "anaconda_part_device_type_$type";
+    assert_and_click "anaconda_part_update_settings";
+}
+
 1;
 
 # vim: set sw=4 et:

main.pm

@@ -79,37 +79,37 @@ else
         ## Select package set. Minimal is the default, if 'default' is specified, skip selection.
         autotest::loadtest get_var('CASEDIR')."/tests/_software_selection.pm";
 
-        ## Disk partitioning
-        if (get_var('DISK_GUIDED_MULTI')) {
-            autotest::loadtest get_var('CASEDIR')."/tests/disk_guided_multi.pm";
-        }
-        elsif (get_var('DISK_GUIDED_DELETE_ALL')) {
-            autotest::loadtest get_var('CASEDIR')."/tests/disk_guided_delete_all.pm";
-        }
-        elsif (get_var('DISK_GUIDED_DELETE_PARTIAL')) {
-            autotest::loadtest get_var('CASEDIR')."/tests/disk_guided_delete_partial.pm";
-        }
-        elsif (get_var('DISK_GUIDED_MULTI_EMPTY_ALL')) {
-            autotest::loadtest get_var('CASEDIR')."/tests/disk_guided_multi_empty_all.pm";
-        }
-        elsif (get_var('DISK_SOFTWARE_RAID')) {
-            autotest::loadtest get_var('CASEDIR')."/tests/disk_part_software_raid.pm";
+        ## Disk partitioning.
+        # If DISK_CUSTOM or DISK_GUIDED is set, we pick the storage test
+        # to run based on the value (usually we run the test with the name
+        # that matches the value, except for a couple of commented cases).
+        my $storage = '';
+        if (get_var('DISK_CUSTOM')) {
+            $storage = get_var('CASEDIR')."/tests/disk_custom_".get_var('DISK_CUSTOM').".pm";
         }
         else {
-            # also DISK_GUIDED_FREE_SPACE
-            autotest::loadtest get_var('CASEDIR')."/tests/disk_guided_empty.pm";
+            my $disk_guided = get_var('DISK_GUIDED');
+            # if DISK_GUIDED is unset, or one of [...], use disk_guided_empty,
+            # which is the simplest / 'default' case.
+            if (! $disk_guided || $disk_guided ~~ ['empty', 'free_space']) {
+                $storage = get_var('CASEDIR')."/tests/disk_guided_empty.pm";
+            }
+            else {
+                $storage = get_var('CASEDIR')."/tests/disk_guided_".$disk_guided.".pm";
+            }
         }
+        autotest::loadtest $storage;
 
         if (get_var("ENCRYPT_PASSWORD")){
             autotest::loadtest get_var('CASEDIR')."/tests/disk_guided_encrypted.pm";
         }
 
-
         # Start installation, set user & root passwords, reboot
         autotest::loadtest get_var('CASEDIR')."/tests/_do_install_and_reboot.pm";
     }
 
-    # Unlock encrypted storage volumes, if necessary
+    # Unlock encrypted storage volumes, if necessary. The test name here
+    # follows the 'storage post-install' convention, but must be run earlier.
     if (get_var("ENCRYPT_PASSWORD")){
         autotest::loadtest get_var('CASEDIR')."/tests/disk_guided_encrypted_postinstall.pm";
     }
@@ -122,21 +122,19 @@ else
         autotest::loadtest get_var('CASEDIR')."/tests/_console_wait_login.pm";
     }
 
-    if (get_var('DISK_GUIDED_MULTI')) {
-        autotest::loadtest get_var('CASEDIR')."/tests/disk_guided_multi_postinstall.pm";
+    # If there is a post-install test to verify storage configuration worked
+    # correctly, run it. Again we determine the test name based on the value
+    # of DISK_CUSTOM or DISK_GUIDED.
+    my $storagepost = '';
+    if (get_var('DISK_GUIDED')) {
+        my $loc = get_var('CASEDIR')."/tests/disk_guided_".get_var('DISK_GUIDED')."_postinstall.pm";
+        $storagepost = $loc if (-e $loc);
     }
-    elsif (get_var('DISK_GUIDED_DELETE_PARTIAL')) {
-        autotest::loadtest get_var('CASEDIR')."/tests/disk_guided_delete_partial_postinstall.pm";
-    }
-    elsif (get_var('DISK_GUIDED_FREE_SPACE')) {
-        autotest::loadtest get_var('CASEDIR')."/tests/disk_guided_free_space_postinstall.pm";
-    }
-    elsif (get_var('DISK_GUIDED_MULTI_EMPTY_ALL')) {
-        autotest::loadtest get_var('CASEDIR')."/tests/disk_guided_multi_empty_all_postinstall.pm";
-    }
-    elsif (get_var('DISK_SOFTWARE_RAID')) {
-        autotest::loadtest get_var('CASEDIR')."/tests/disk_part_software_raid_postinstall.pm";
+    elsif (get_var('DISK_CUSTOM')) {
+        my $loc = get_var('CASEDIR')."/tests/disk_custom_".get_var('DISK_CUSTOM')."_postinstall.pm";
+        $storagepost = $loc if (-e $loc);
     }
+    autotest::loadtest $storagepost if ($storagepost);
 }
 
 

needles/anaconda_install_destination_two_disks.json

@@ -1,31 +0,0 @@
-{
-    "area": [
-        {
-            "height": 46,
-            "type": "match",
-            "width": 162,
-            "xpos": 122,
-            "ypos": 213
-        },
-        {
-            "height": 24,
-            "type": "match",
-            "width": 17,
-            "xpos": 41,
-            "ypos": 289
-        },
-        {
-            "height": 23,
-            "type": "match",
-            "width": 15,
-            "xpos": 206,
-            "ypos": 290
-        }
-    ],
-    "tags": [
-        "anaconda_install_destination_two_disks",
-        "ENV-DISTRI-fedora",
-        "ENV-INSTLANG-en_US",
-        "ENV-FLAVOR-server"
-    ]
-}

needles/anaconda_part_automatic.json

@@ -0,0 +1,17 @@
+{
+    "area": [
+        {
+            "height": 17,
+            "type": "match",
+            "width": 244,
+            "xpos": 71,
+            "ypos": 178
+        }
+    ],
+    "tags": [
+    "anaconda_part_automatic",
+    "ENV-DISTRI-fedora",
+    "ENV-INSTLANG-en_US",
+    "ENV-FLAVOR-server"
+    ]
+}

needles/anaconda_part_device_type_raid.json

@@ -10,7 +10,7 @@
     }
   ],
   "tags": [
-    "anaconda_part_raid_list",
+    "anaconda_part_device_type_raid",
     "ENV-DISTRI-fedora",
     "ENV-INSTLANG-en_US",
     "ENV-FLAVOR-server"

needles/anaconda_part_scheme.json

@@ -0,0 +1,17 @@
+{
+    "area": [
+        {
+            "height": 17,
+            "type": "match",
+            "width": 244,
+            "xpos": 71,
+            "ypos": 178
+        }
+    ],
+    "tags": [
+    "anaconda_part_automatic",
+    "ENV-DISTRI-fedora",
+    "ENV-INSTLANG-en_US",
+    "ENV-FLAVOR-server"
+    ]
+}

templates

@@ -275,7 +275,7 @@
                       name => "server_sata_multi",
                       prio => 2,
                       settings => [
-                        { key => "DISK_GUIDED_MULTI", value => "1" },
+                        { key => "DISK_GUIDED", value => "multi" },
                         { key => "HDDMODEL", value => "ide-hd,bus=ahci0.0" },
                         { key => "NUMDISKS", value => "2" },
                         { key => "HDD_2", value => "disk_full.img" },
@@ -309,7 +309,7 @@
                       name => "server_delete_pata",
                       prio => 5,
                       settings => [
-                        { key => "DISK_GUIDED_DELETE_ALL", value => "1" },
+                        { key => "DISK_GUIDED", value => "delete_all" },
                         { key => "HDDMODEL", value => "ide-hd" },
                         { key => "HDD_1", value => "disk_full.img" },
                       ],
@@ -343,7 +343,7 @@
                       name => "server_delete_partial",
                       prio => 9,
                       settings => [
-                        { key => "DISK_GUIDED_DELETE_PARTIAL", value => "1" },
+                        { key => "DISK_GUIDED", value => "delete_partial" },
                         { key => "HDD_1", value => "disk_full.img" },
                         { key => "ROOT_PASSWORD", value => "weakpassword" },
                       ],
@@ -361,7 +361,7 @@
                       name => "server_simple_free_space",
                       prio => 11,
                       settings => [
-                        { key => "DISK_GUIDED_FREE_SPACE", value => "1" },
+                        { key => "DISK_GUIDED", value => "free_space" },
                         { key => "HDD_1", value => "disk_freespace.img" },
                         { key => "ROOT_PASSWORD", value => "weakpassword" },
                       ],
@@ -371,7 +371,7 @@
                       name => "server_multi_empty",
                       prio => 12,
                       settings => [
-                        { key => "DISK_GUIDED_MULTI_EMPTY_ALL", value => "1" },
+                        { key => "DISK_GUIDED", value => "multi_empty_all" },
                         { key => "NUMDISKS", value => "2" },
                         { key => "ROOT_PASSWORD", value => "weakpassword" },
                       ],
@@ -381,7 +381,7 @@
                       name => "server_software_raid",
                       prio => 13,
                       settings => [
-                        { key => "DISK_SOFTWARE_RAID", value => "1" },
+                        { key => "DISK_CUSTOM", value => "software_raid" },
                         { key => "NUMDISKS", value => "2" },
                         { key => "ROOT_PASSWORD", value => "weakpassword" },
                       ],

tests/disk_custom_software_raid.pm

@@ -0,0 +1,33 @@
+use base "anacondalog";
+use strict;
+use testapi;
+
+sub run {
+    my $self = shift;
+    # Go to INSTALLATION DESTINATION and ensure two disks are selected.
+    # Because DISK_CUSTOM is set, select_disks will select custom for us.
+    $self->select_disks(2);
+    assert_and_click "anaconda_spoke_done";
+
+    # Manual partitioning spoke should be displayed
+    assert_and_click "anaconda_part_automatic";
+    $self->custom_change_type("raid");
+    assert_and_click "anaconda_spoke_done";
+    assert_and_click "anaconda_part_accept_changes";
+
+    # Anaconda hub
+    assert_screen "anaconda_main_hub", 300; #
+
+}
+
+sub test_flags {
+    # without anything - rollback to 'lastgood' snapshot if failed
+    # 'fatal' - whole test suite is in danger if this fails
+    # 'milestone' - after this test succeeds, update 'lastgood'
+    # 'important' - if this fails, set the overall state to 'fail'
+    return { fatal => 1 };
+}
+
+1;
+
+# vim: set sw=4 et:

tests/disk_guided_delete_all.pm

@@ -3,11 +3,10 @@ use strict;
 use testapi;
 
 sub run {
+    my $self = shift;
     # Anaconda hub
-    assert_screen "anaconda_main_hub", 300; #
-
-    assert_and_click "anaconda_main_hub_install_destination"; 
-
+    # Go to INSTALLATION DESTINATION and ensure one disk is selected.
+    $self->select_disks();
     assert_and_click "anaconda_spoke_done";
 
     # the only provided disk should be full

tests/disk_guided_delete_partial.pm

@@ -3,12 +3,10 @@ use strict;
 use testapi;
 
 sub run {
+    my $self = shift;
     # Anaconda hub
-    assert_screen "anaconda_main_hub", 300; #
-
-    # Select the first disk, it should be full
-    assert_and_click "anaconda_main_hub_install_destination";
-
+    # Go to INSTALLATION DESTINATION and ensure one disk is selected.
+    $self->select_disks();
     assert_and_click "anaconda_spoke_done";
 
     # Provided disk should be full

tests/disk_guided_empty.pm

@@ -3,12 +3,13 @@ use strict;
 use testapi;
 
 sub run {
+    my $self = shift;
     # Anaconda hub
-    assert_screen "anaconda_main_hub", 300; #
-
-    # Default install destination (hdd should be empty for new KVM machine)
-    assert_and_click "anaconda_main_hub_install_destination";
+    # Go to INSTALLATION DESTINATION and ensure one disk is selected.
+    $self->select_disks();
 
+    # updates.img tests work by changing the appearance of the INSTALLATION
+    # DESTINATION screen, so check that if needed.
     if (get_var('BOOT_UPDATES_IMG_URL')){
         assert_screen "anaconda_install_destination_pony", 30;
     }

tests/disk_guided_encrypted.pm

@@ -3,12 +3,10 @@ use strict;
 use testapi;
 
 sub run {
+    my $self = shift;
     # Anaconda hub
-    assert_screen "anaconda_main_hub", 300; #
-
-    # Default install destination (hdd should be empty for new KVM machine)
-    assert_and_click "anaconda_main_hub_install_destination";
-
+    # Go to INSTALLATION DESTINATION and ensure one disk is selected.
+    $self->select_disks();
     assert_and_click "anaconda_install_destination_encrypt_data";
     assert_and_click "anaconda_spoke_done";
 

tests/disk_guided_multi.pm

@@ -3,14 +3,10 @@ use strict;
 use testapi;
 
 sub run {
+    my $self = shift;
     # Anaconda hub
-    assert_screen "anaconda_main_hub", 300; #
-
-    # Select the first disk
-    assert_and_click "anaconda_main_hub_install_destination";
-
-    assert_screen "anaconda_install_destination_two_disks";
-    assert_and_click "anaconda_install_destination_select_disk_1";
+    # Go to INSTALLATION DESTINATION and select only one disk.
+    $self->select_disks(1);
     assert_and_click "anaconda_spoke_done";
 
     # Anaconda hub

tests/disk_guided_multi_empty_all.pm

@@ -3,15 +3,10 @@ use strict;
 use testapi;
 
 sub run {
+    my $self = shift;
     # Anaconda hub
-    assert_screen "anaconda_main_hub", 300; #
-
-    assert_and_click "anaconda_main_hub_install_destination";
-
-    # Select both disks for installation
-    assert_screen "anaconda_install_destination_two_disks";
-    assert_and_click "anaconda_install_destination_select_disk_1";
-    assert_and_click "anaconda_install_destination_select_disk_2";
+    # Go to INSTALLATION DESTINATION and select two disks.
+    $self->select_disks(2);
     assert_and_click "anaconda_spoke_done";
 
     # Anaconda hub

tests/disk_part_software_raid.pm

@@ -1,73 +0,0 @@
-use base "anacondalog";
-use strict;
-use testapi;
-
-sub run {
-    # Anaconda hub
-    assert_screen "anaconda_main_hub", 300; #
-
-    assert_and_click "anaconda_main_hub_install_destination";
-
-    # Select both disks for installation
-    assert_screen "anaconda_install_destination_two_disks";
-    assert_and_click "anaconda_install_destination_select_disk_1";
-    assert_and_click "anaconda_install_destination_select_disk_2";
-
-    # Select manual partitioning
-    assert_and_click "anaconda_manual_partitioning";
-
-    assert_and_click "anaconda_spoke_done";
-
-    # Manual partitioning spoke should be displayed
-
-    # Add /boot partition
-    assert_and_click "anaconda_part_plus_button";
-    assert_and_click "anaconda_part_list_box_button";
-    assert_and_click "anaconda_part_list_box_boot";
-    assert_and_click "anaconda_part_desired_capacity";
-
-    type_string "200M";
-
-    assert_and_click "anaconda_part_add_mountpoint";
-
-    # Add swap partition
-    assert_and_click "anaconda_part_plus_button";
-    assert_and_click "anaconda_part_list_box_button";
-    assert_and_click "anaconda_part_list_box_swap";
-    assert_and_click "anaconda_part_desired_capacity";
-
-    type_string "2G";
-
-    assert_and_click "anaconda_part_add_mountpoint";
-
-    # Add root partition
-    assert_and_click "anaconda_part_plus_button";
-    assert_and_click "anaconda_part_list_box_button";
-    assert_and_click "anaconda_part_list_box_root";
-
-    assert_and_click "anaconda_part_add_mountpoint";
-
-    # Change type to RAID
-    assert_and_click "anaconda_part_device_type";
-    assert_and_click "anaconda_part_raid_list";
-    assert_and_click "anaconda_part_update_settings";
-
-    assert_and_click "anaconda_spoke_done";
-    assert_and_click "anaconda_part_accept_changes";
-
-    # Anaconda hub
-    assert_screen "anaconda_main_hub", 300; #
-
-}
-
-sub test_flags {
-    # without anything - rollback to 'lastgood' snapshot if failed
-    # 'fatal' - whole test suite is in danger if this fails
-    # 'milestone' - after this test succeeds, update 'lastgood'
-    # 'important' - if this fails, set the overall state to 'fail'
-    return { fatal => 1 };
-}
-
-1;
-
-# vim: set sw=4 et: