createpool: Add more coverage testing

tests/storage-xml/pool-disk-volclone.xml

@@ -1,12 +1,12 @@
 <volume type="file">
   <name>pool-disk-volclone</name>
-  <key>/some/target/path/pool-disk-vol</key>
+  <key>/dev/pool-disk-vol</key>
   <source>
   </source>
   <capacity unit="bytes">10737418240</capacity>
   <allocation unit="bytes">5368709120</allocation>
   <target>
-    <path>/some/target/path/pool-disk-vol</path>
+    <path>/dev/pool-disk-vol</path>
     <format type="none"/>
     <permissions>
       <mode>0700</mode>

tests/storage-xml/pool-disk.xml

@@ -4,6 +4,6 @@
     <device path="/some/source/path"/>
   </source>
   <target>
-    <path>/some/target/path</path>
+    <path>/dev</path>
   </target>
 </pool>

tests/storage.py

@@ -211,10 +211,6 @@ class TestStorage(unittest.TestCase):
         diskvol.pool = diskpool
         self.assertTrue(diskvol.supports_format() is False)
 
-        diskpoolxml = StoragePool(fullconn)
-        diskpoolxml.type = "disk"
-        self.assertEqual(diskpoolxml.default_target_path(), "/dev")
-
         glusterpool.destroy()
         StoragePool.ensure_pool_is_running(glusterpool)
 

tests/uitests/createpool.py

@@ -24,10 +24,20 @@ class CreatePool(uiutils.UITestCase):
     def testCreatePools(self):
         hostwin = self._open_host_window("Storage")
         win = self._open_create_win(hostwin)
-
-        # Create a simple default dir pool
         finish = win.find("Finish", "push button")
         name = win.find("Name:", "text")
+
+        def _browse_local_path(winlabel, usepath):
+            chooser = self.app.root.find(winlabel, "file chooser")
+            # Enter the filename and select it
+            chooser.find(usepath, "table cell").click()
+            obutton = chooser.find("Open", "push button")
+            uiutils.check_in_loop(lambda: obutton.sensitive)
+            obutton.click()
+            uiutils.check_in_loop(lambda: not chooser.showing)
+            uiutils.check_in_loop(lambda: win.active)
+
+        # Create a simple default dir pool
         self.assertEqual(name.text, "pool")
         newname = "a-test-new-pool"
         name.text = newname
@@ -58,6 +68,39 @@ class CreatePool(uiutils.UITestCase):
         # Ensure it's gone
         uiutils.check_in_loop(lambda: cell.dead)
 
+        # Test a disk pool
+        win = self._open_create_win(hostwin)
+        typ = win.find("Type:", "combo box")
+        newname = "a-disk-pool"
+        name.text = "a-disk-pool"
+        typ.click()
+        win.find_fuzzy("Physical Disk", "menu item").click()
+        win.find("source-browse").click()
+        _browse_local_path("Choose source path", "console")
+        finish.click()
+        hostwin.find(newname, "table cell")
+
+        # Test a iscsi pool
+        win = self._open_create_win(hostwin)
+        typ = win.find("Type:", "combo box")
+        newname = "a-iscsi-pool"
+        name.text = "a-iscsi-pool"
+        typ.click()
+        win.find_fuzzy("iSCSI", "menu item").click()
+        win.find("target-browse").click()
+        _browse_local_path("Choose target directory", "by-path")
+        finish.click()
+        # Catch example error
+        alert = self.app.root.find("vmm dialog", "alert")
+        alert.find_fuzzy("source host name", "label")
+        alert.find("Close", "push button").click()
+        win.find("Host Name:", "text").text = "example.com"
+        win.find("pool-source-path-text").text = "foo-iqn"
+        win.find_fuzzy("Initiator IQN:", "check").click()
+        win.find("iqn-text", "text").text = "initiator-foo"
+        finish.click()
+        hostwin.find(newname, "table cell")
+
         # Test a logical pool
         win = self._open_create_win(hostwin)
         typ = win.find("Type:", "combo box")

tests/xmlparse-xml/change-disk-out.xml

@@ -19,7 +19,7 @@
   <devices>
     <emulator>/usr/lib/xen/bin/qemu-dm</emulator>
     <disk type="file" device="disk">
-      <source file="/dev/null"/>
+      <source file="/dev/foo/null"/>
       <target dev="hda" bus="usb" removable="on"/>
       <serial>frob</serial>
       <driver name="test" type="raw"/>

tests/xmlparse.py

@@ -372,7 +372,7 @@ class XMLParseTest(unittest.TestCase):
 
         disk = _get_disk("hda")
         check = self._make_checker(disk)
-        check("path", "/tmp/test.img", "/dev/null")
+        check("path", "/tmp/test.img", "/dev/foo/null")
         disk.sync_path_props()
         check("driver_name", None, "test")
         check("driver_type", None, "raw")

ui/createpool.ui

@@ -133,6 +133,11 @@
                             <property name="can_focus">True</property>
                             <property name="invisible_char">●</property>
                             <property name="width_chars">25</property>
+                            <child internal-child="accessible">
+                              <object class="AtkObject" id="pool-iqn-atkobject">
+                                <property name="AtkObject::accessible-name">iqn-text</property>
+                              </object>
+                            </child>
                           </object>
                           <packing>
                             <property name="expand">True</property>
@@ -192,7 +197,7 @@
                         <property name="visible">True</property>
                         <property name="can_focus">False</property>
                         <property name="halign">end</property>
-                        <property name="label">_sourcep:</property>
+                        <property name="label">_Sourcep:</property>
                         <property name="use_underline">True</property>
                         <property name="mnemonic_widget">pool-source-path</property>
                       </object>
@@ -224,6 +229,11 @@
                         <property name="halign">start</property>
                         <property name="use_underline">True</property>
                         <signal name="clicked" handler="on_pool_target_button_clicked" swapped="no"/>
+                        <child internal-child="accessible">
+                          <object class="AtkObject" id="pool-target-button-atkobject">
+                            <property name="AtkObject::accessible-name">target-browse</property>
+                          </object>
+                        </child>
                       </object>
                       <packing>
                         <property name="left_attach">2</property>
@@ -239,6 +249,11 @@
                         <property name="halign">start</property>
                         <property name="use_underline">True</property>
                         <signal name="clicked" handler="on_pool_source_button_clicked" swapped="no"/>
+                        <child internal-child="accessible">
+                          <object class="AtkObject" id="pool-source-button-atkobject">
+                            <property name="AtkObject::accessible-name">source-browse</property>
+                          </object>
+                        </child>
                       </object>
                       <packing>
                         <property name="left_attach">2</property>
@@ -254,6 +269,11 @@
                           <object class="GtkEntry">
                             <property name="can_focus">True</property>
                             <property name="width_chars">25</property>
+                            <child internal-child="accessible">
+                              <object class="AtkObject">
+                                <property name="AtkObject::accessible-name">pool-source-path-text</property>
+                              </object>
+                            </child>
                           </object>
                         </child>
                         <child internal-child="accessible">
@@ -273,7 +293,6 @@
                         <property name="can_focus">True</property>
                         <property name="invisible_char">●</property>
                         <property name="width_chars">25</property>
-                        <signal name="activate" handler="on_pool_hostname_activate" swapped="no"/>
                       </object>
                       <packing>
                         <property name="left_attach">1</property>

virtManager/createpool.py

@@ -43,7 +43,6 @@ class vmmCreatePool(vmmGObjectUI):
             "on_pool_source_button_clicked": self._browse_source_cb,
             "on_pool_target_button_clicked": self._browse_target_cb,
 
-            "on_pool_hostname_activate": self._hostname_changed_cb,
             "on_pool_iqn_chk_toggled": self._iqn_toggled_cb,
         })
         self.bind_escape_key_close()
@@ -175,16 +174,13 @@ class vmmCreatePool(vmmGObjectUI):
         try:
             plist = StoragePool.pool_list_from_sources(
                     self.conn.get_backend(), pool_type)
-        except Exception:
+        except Exception:  # pragma: no cover
             log.exception("Pool enumeration failed")
 
         return plist
 
     def _get_build_default(self, pooltype):
         """ Return (default value, whether build option can be changed)"""
-        if not pooltype:
-            return (False, False)
-
         if pooltype in [StoragePool.TYPE_DIR,
                         StoragePool.TYPE_FS,
                         StoragePool.TYPE_NETFS]:
@@ -240,6 +236,7 @@ class vmmCreatePool(vmmGObjectUI):
         elif is_scsi:
             src_label = _("_Source Adapter:")
         self.widget("pool-source-label").set_text(src_label)
+        self.widget("pool-source-label").set_use_underline(True)
 
         if tgt:
             self.widget("pool-target-path").set_text(
@@ -267,10 +264,7 @@ class vmmCreatePool(vmmGObjectUI):
         if column is None:
             return widget.get_text().strip()
 
-        ret = uiutil.get_list_selection(widget, column=column)
+        return uiutil.get_list_selection(widget, column=column)
-        if ret is not None:
-            return ret
-        return widget_name.get_child().get_text().strip()
 
     def _get_config_pool_type(self):
         return uiutil.get_list_selection(self.widget("pool-type"))
@@ -393,9 +387,8 @@ class vmmCreatePool(vmmGObjectUI):
         try:
             if self._validate(pool) is False:
                 return
-        except Exception as e:
+        except Exception as e:  # pragma: no cover
-            return self.err.show_err(
+            return self.err.show_err(_("Error validating pool: %s") % e)
-                    _("Error validating pool: %s") % e)
 
         self.reset_finish_cursor()
 
@@ -444,9 +437,5 @@ class vmmCreatePool(vmmGObjectUI):
         if target:
             self.widget("pool-target-path").set_text(target)
 
-    def _hostname_changed_cb(self, src):
-        # If a hostname was entered, try to lookup valid pool sources.
-        self._populate_pool_sources()
-
     def _iqn_toggled_cb(self, src):
         self.widget("pool-iqn").set_sensitive(src.get_active())

virtinst/storage.py

@@ -232,8 +232,6 @@ class StoragePool(_StorageObject):
             self.type == self.TYPE_FS):
             return os.path.join(
                     _preferred_default_pool_path(self.conn), self.name)
-        if self.type == self.TYPE_DISK:
-            return _DEFAULT_DEV_TARGET
         if self.type == self.TYPE_ISCSI or self.type == self.TYPE_SCSI:
             return _DEFAULT_SCSI_TARGET
         if self.type == self.TYPE_MPATH:
@@ -309,7 +307,7 @@ class StoragePool(_StorageObject):
     def supports_target_path(self):
         return self.type in [
                 self.TYPE_DIR, self.TYPE_FS, self.TYPE_NETFS,
-                self.TYPE_DISK, self.TYPE_ISCSI,
+                self.TYPE_ISCSI,
                 self.TYPE_SCSI, self.TYPE_MPATH]
 
     def supports_source_name(self):
@@ -357,7 +355,12 @@ class StoragePool(_StorageObject):
         self.validate_name(self.conn, self.name)
 
         if not self.target_path:
-            self.target_path = self.default_target_path()
+            if self.type == self.TYPE_DISK:
+                # disk is a bit special, in that it demands a target path,
+                # but basically can't handle anything other than /dev
+                self.target_path = _DEFAULT_DEV_TARGET
+            else:
+                self.target_path = self.default_target_path()
         if not self.source_name:
             self.source_name = self.default_source_name()
         if not self.format and self.supports_format():