unattended: Finish off code coverage testing

tests/clitest.py

@@ -55,6 +55,7 @@ iso_links = [
     "/tmp/fake-no-osinfo.iso",
     "/tmp/fake-win7.iso",
     "/tmp/fake-f26-netinst.iso",
+    "/tmp/fake-f29-live.iso",
 ]
 
 exist_files = exist_images
@@ -85,6 +86,7 @@ test_files = {
     'ISO-NO-OS': iso_links[2],
     'ISO-WIN7': iso_links[3],
     'ISO-F26-NETINST': iso_links[4],
+    'ISO-F29-LIVE': iso_links[5],
     'TREEDIR': "%s/fakefedoratree" % XMLDIR,
     'COLLIDE': "/dev/default-pool/collidevol1.img",
 }
@@ -807,11 +809,16 @@ c.add_compare("--pxe --print-step all", "simple-pxe")  # Diskless PXE install
 c.add_compare("--location ftp://example.com", "fake-ftp")  # fake ftp:// install using urlfetcher.py mocking
 c.add_compare("--location https://foobar.com", "fake-http")  # fake https:// install using urlfetcher.py mocking
 c.add_compare("--connect %(URI-KVM)s --os-variant fedora26,install=location", "osinfo-url")  # getting URL from osinfo
-c.add_compare("--connect %(URI-KVM)s --os-variant fedora26 --unattended profile=desktop,admin-password=foobar", "osinfo-url-unattended", prerun_check=no_osinfo_unattend_cb)  # unattended install for fedora, using initrd injection
+c.add_compare("--connect %(URI-KVM)s --os-variant fedora26 --unattended profile=desktop,admin-password=foobar,user-password=blah,product-key=1234", "osinfo-url-unattended", prerun_check=no_osinfo_unattend_cb)  # unattended install for fedora, using initrd injection
 c.add_compare("--connect %(URI-KVM)s --os-variant win7 --cdrom %(ISO-WIN7)s --unattended profile=desktop,admin-password=foobar", "osinfo-win7-unattended", prerun_check=no_osinfo_unattend_cb)  # unattended install for win7
 c.add_compare("--connect %(URI-KVM)s --os-variant fedora26 --unattended profile=jeos,admin-password=123456 --cdrom %(ISO-F26-NETINST)s", "osinfo-netinst-unattended", prerun_check=no_osinfo_unattend_cb)  # triggering the special netinst checking code
 c.add_compare("--connect %(URI-KVM)s --os-variant silverblue29 --location http://example.com", "network-install-resources", prerun_check=no_osinfo_unattend_cb)  # triggering network-install resources override
-c.add_invalid("--pxe --virt-type bogus")  # Bogus virt-type
+c.add_invalid("--connect %(URI-KVM)s --os-variant fedora26 --unattended profile=jeos --location http://example.foo", grep="admin-password")  # will trigger admin-password required error
+c.add_invalid("--connect %(URI-KVM)s --os-variant debian9 --unattended profile=desktop,admin-password=foobar --location http://example.foo", grep="user-password")  # will trigger user-password required error
+c.add_invalid("--connect %(URI-KVM)s --os-variant debian9 --unattended profile=FRIBBER,admin-password=foobar --location http://example.foo", grep="Available profiles")  # will trigger unknown profile error
+c.add_invalid("--connect %(URI-KVM)s --os-variant fedora29 --unattended profile=desktop,admin-password=foobar --cdrom %(ISO-F29-LIVE)s", grep="media does not support")  # live media doesn't support installscript
+c.add_invalid("--connect %(URI-KVM)s --os-variant msdos --unattended profile=desktop --location http://example.com")  # msdos doesn't support unattended install
+c.add_invalid("--connect %(URI-KVM)s --os-variant winxp --unattended profile=desktop --cdrom %(ISO-WIN7)s")  # winxp doesn't support expected injection method 'cdrom'
 c.add_invalid("--pxe --virt-type bogus")  # Bogus virt-type
 c.add_invalid("--pxe --arch bogus")  # Bogus arch
 c.add_invalid("--livecd")  # LiveCD with no media

virtinst/cli.py

@@ -912,11 +912,6 @@ class _SuboptCheckerClass:
         self._seen = set()
 
     def add_all(self, name):
-        if name.startswith("--unattended"):
-            # Hack, we don't have any test suite coverage of the
-            # unattended option. This should change soon but until then
-            # disable the test suite failure
-            return
         self._all.add(name)
 
     def add_seen(self, name):

virtinst/unattended.py

@@ -24,9 +24,8 @@ def _make_installconfig(script, osobj, unattended_data, arch, hostname, url):
         TZ_FILE = "/etc/localtime"
         linkpath = os.path.realpath(TZ_FILE)
         tokens = linkpath.split("zoneinfo/")
-        if len(tokens) < 2:
+        if len(tokens) > 1:
-            return None
+            return tokens[1]
-        return tokens[1]
 
     def get_language():
         return locale.getlocale()[0]
@@ -77,15 +76,11 @@ def _make_installconfig(script, osobj, unattended_data, arch, hostname, url):
     timezone = get_timezone()
     if timezone:
         config.set_l10n_timezone(timezone)
-    else:
-        logging.warning(
-            _("'America/New_York' timezone will be used for this "
-              "unattended installation."))
 
     # Try to guess to language and keyboard layout from the system's
     # language.
     #
-    # This method has flows as it's quite common to have language and
+    # This method has flaws as it's quite common to have language and
     # keyboard layout not matching. Otherwise, there's no easy way to guess
     # the keyboard layout without relying on a set of APIs of an specific
     # Desktop Environment.
@@ -93,10 +88,6 @@ def _make_installconfig(script, osobj, unattended_data, arch, hostname, url):
     if language:
         config.set_l10n_language(language)
         config.set_l10n_keyboard(language)
-    else:
-        logging.warning(
-            _("'en_US' will be used as both language and keyboard layout "
-              "for unattended installation."))
 
     if url:
         config.set_installation_url(url)  # pylint: disable=no-member
@@ -135,68 +126,46 @@ class OSInstallScript:
             if (Libosinfo.InstallScriptInjectionMethod.CDROM &
                 script.get_injection_methods()):
                 return True
-        return False
+        return False  # pragma: no cover
 
     def __init__(self, script, osobj):
         self._script = script
         self._osobj = osobj
         self._config = None
 
-        if not OSInstallScript.have_new_libosinfo():
+        if not OSInstallScript.have_new_libosinfo():  # pragma: no cover
             raise RuntimeError(_("libosinfo or osinfo-db is too old to "
                 "support unattended installs."))
 
     def get_expected_filename(self):
         return self._script.get_expected_filename()
 
-    def set_preferred_injection_method(self, method):
+    def set_preferred_injection_method(self, namestr):
-        def nick_to_value(method):
+        # If we ever make this user configurable, this will need to be smarter
-            injection_methods = [
+        names = {
-                    Libosinfo.InstallScriptInjectionMethod.CDROM,
+            "cdrom": Libosinfo.InstallScriptInjectionMethod.CDROM,
-                    Libosinfo.InstallScriptInjectionMethod.DISK,
+            "initrd": Libosinfo.InstallScriptInjectionMethod.INITRD,
-                    Libosinfo.InstallScriptInjectionMethod.FLOPPY,
+        }
-                    Libosinfo.InstallScriptInjectionMethod.INITRD,
-                    Libosinfo.InstallScriptInjectionMethod.WEB]
 
-            for m in injection_methods:
+        logging.debug("Using '%s' injection method", namestr)
-                # pylint: disable=no-member
+        injection_method = names[namestr]
-                if method == m.value_nicks[0]:
-                    return m
-
-            raise RuntimeError(
-                _("%s is a non-valid injection method in libosinfo.") % method)
-
-        injection_method = nick_to_value(method)
         supported_injection_methods = self._script.get_injection_methods()
         if (injection_method & supported_injection_methods == 0):
             raise RuntimeError(
-                _("OS '%s' unattended install is not supported") %
+                _("OS '%s' does not support required injection method '%s'") %
-                self._osobj.name)
+                (self._osobj.name, namestr))
 
-        logging.debug("Using '%s' injection method", method)
         self._script.set_preferred_injection_method(injection_method)
 
-    def set_installation_source(self, source):
+    def set_installation_source(self, namestr):
-        def nick_to_value(source):
+        # If we ever make this user configurable, this will need to be smarter
-            # This requires quite new libosinfo as of Mar 2019, disable
+        names = {
-            # pylint errors here.
+            "media": Libosinfo.InstallScriptInstallationSource.MEDIA,
-            # pylint: disable=no-member
+            "network": Libosinfo.InstallScriptInstallationSource.NETWORK,
-            installation_sources = [
+        }
-                    Libosinfo.InstallScriptInstallationSource.MEDIA,
-                    Libosinfo.InstallScriptInstallationSource.NETWORK]
 
-            for s in installation_sources:
+        logging.debug("Using '%s' installation source", namestr)
-                if source == s.value_nick:
+        self._script.set_installation_source(names[namestr])
-                    return s
-
-            raise RuntimeError(
-                _("%s is a non-valid installation source in libosinfo.") %
-                source)
-
-        installation_source = nick_to_value(source)
-
-        logging.debug("Using '%s' installation source", source)
-        self._script.set_installation_source(installation_source)
 
     def _requires_param(self, config_param):
         param = self._script.get_config_param(config_param)
@@ -276,13 +245,14 @@ def _lookup_rawscript(guest, profile, os_media):
               "the '%s' profile. Available profiles: %s") %
             (guest.osinfo.name, profile, ", ".join(list(profile_names))))
 
-    logging.debug("Install script found for profile '%s'", profile)
-
     # Some OSes (as Windows) have more than one installer script,
     # depending on the OS version and profile chosen, to be used to
     # perform the unattended installation. Let's just deal with
     # multiple installer scripts when its actually needed, though.
-    return rawscripts[0]
+    usescript = rawscripts[0]
+    logging.debug("Install script found for profile '%s': %s",
+            profile, usescript.get_id())
+    return usescript
 
 
 def prepare_install_script(guest, unattended_data, url, os_media):