virtinst: Add a lot of test code coverage

* Delete dead code * Add tests for various device default code paths * Rework certain conditions a bit so code coverage always hits them
2024-10-26 08:55:13 +03:00 · 2019-06-09 16:39:15 -04:00 · 2019-06-09 16:39:15 -04:00 · 361657ad15
commit 361657ad15
parent 261a0a0482
22 changed files with 61 additions and 73 deletions
--- a/setup.py
+++ b/setup.py
@ -476,7 +476,7 @@ class TestBaseCommand(distutils.core.Command):
        err = int(bool(len(result.failures) > 0 or
                       len(result.errors) > 0))
        if cov and not err:
-            cov.report(show_missing=False)
+            cov.report(show_missing=False, skip_covered=True)
        sys.exit(err)


--- a/tests/cli-test-xml/compare/virt-install-many-devices.xml
+++ b/tests/cli-test-xml/compare/virt-install-many-devices.xml
@ -353,6 +353,7 @@
    </channel>
    <input type="keyboard" bus="usb"/>
    <input type="tablet" bus="usb"/>
+    <input type="mouse" bus="ps2"/>
    <tpm model="tpm-crb">
      <backend type="emulator" version="2.0"/>
    </tpm>
--- a/tests/cli-test-xml/compare/virt-install-singleton-config-1.xml
+++ b/tests/cli-test-xml/compare/virt-install-singleton-config-1.xml
@ -88,6 +88,9 @@
    <rng model="virtio">
      <backend model="random">/dev/random</backend>
    </rng>
+    <vsock model="virtio">
+      <cid auto="yes"/>
+    </vsock>
  </devices>
  <seclabel type="dynamic" model="selinux"/>
  <seclabel type="none" model="dac"/>
--- a/tests/cli-test-xml/compare/virt-install-singleton-config-2.xml
+++ b/tests/cli-test-xml/compare/virt-install-singleton-config-2.xml
@ -210,6 +210,9 @@
    <label>system_u:object_r:svirt_image_t:s0:c100,c200</label>
    <baselabel>baselabel</baselabel>
  </seclabel>
+  <seclabel type="dynamic" model="dac">
+    <label>012:345</label>
+  </seclabel>
  <on_lockfailure>ignore</on_lockfailure>
  <cputune>
    <vcpupin vcpu="0" cpuset="0-3"/>
@ -429,6 +432,9 @@
    <label>system_u:object_r:svirt_image_t:s0:c100,c200</label>
    <baselabel>baselabel</baselabel>
  </seclabel>
+  <seclabel type="dynamic" model="dac">
+    <label>012:345</label>
+  </seclabel>
  <on_lockfailure>ignore</on_lockfailure>
  <cputune>
    <vcpupin vcpu="0" cpuset="0-3"/>
--- a/tests/cli-test-xml/compare/virt-install-singleton-config-3.xml
+++ b/tests/cli-test-xml/compare/virt-install-singleton-config-3.xml
@ -118,6 +118,7 @@
    </video>
    <redirdev bus="usb" type="spicevmc"/>
    <redirdev bus="usb" type="spicevmc"/>
+    <memballoon model="virtio"/>
    <rng model="virtio">
      <backend model="egd" type="nmdm">
        <source master="/dev/foo1" slave="/dev/foo2"/>
--- a/tests/cli-test-xml/compare/virt-install-xen-pv.xml
+++ b/tests/cli-test-xml/compare/virt-install-xen-pv.xml
@ -27,6 +27,7 @@
      <mac address="00:11:22:33:44:55"/>
    </interface>
    <console type="pty"/>
+    <input type="mouse" bus="xen"/>
    <graphics type="vnc" port="-1"/>
  </devices>
 </domain>
@ -57,6 +58,7 @@
      <mac address="00:11:22:33:44:55"/>
    </interface>
    <console type="pty"/>
+    <input type="mouse" bus="xen"/>
    <graphics type="vnc" port="-1"/>
  </devices>
 </domain>
--- a/tests/clitest.py
+++ b/tests/clitest.py
@ -470,6 +470,7 @@ c.add_compare("""
 --watchdog default
 --tpm /dev/tpm0
 --rng /dev/random
+--vsock default
 """, "singleton-config-1")

 # Singleton element test #2, for complex strings
@ -491,6 +492,7 @@ cache.mode=emulate,cache.level=3
 --boot cdrom,fd,hd,network,menu=off,loader=/foo/bar,emulator=/new/emu,bootloader=/new/bootld,rebootTimeout=3,initargs="foo=bar baz=woo"
 --idmap uid_start=0,uid_target=1000,uid_count=10,gid_start=0,gid_target=1000,gid_count=10
 --seclabel type=static,label='system_u:object_r:svirt_image_t:s0:c100,c200',relabel=yes,baselabel=baselabel
+--seclabel type=dynamic,label=012:345
 --numatune 1-3,4,mode=strict
 --memtune hard_limit=10,soft_limit=20,swap_hard_limit=30,min_guarantee=40
 --blkiotune weight=100,device_path=/home/test/1.img,device_weight=200
@ -534,6 +536,7 @@ c.add_compare("""
 --memdev dimm,access=private,target.size=512,target.node=0,source.pagesize=4,source.nodemask=1-2
 --memdev nvdimm,source.path=/path/to/nvdimm,target.size=512,target.node=0,target.label_size=128,alias.name=mymemdev3,address.type=dimm,address.base=0x100000000,address.slot=1
 --vsock auto_cid=on
+--memballoon default

 --sysinfo bios.vendor="Acme LLC",bios.version=1.2.3,bios.date=01/01/1970,bios.release=10.22,system.manufacturer="Acme Inc.",system.product=Computer,system.version=3.2.1,system.serial=123456789,system.uuid=00000000-1111-2222-3333-444444444444,system.sku=abc-123,system.family=Server,baseBoard.manufacturer="Acme Corp.",baseBoard.product=Motherboard,baseBoard.version=A01,baseBoard.serial=1234-5678,baseBoard.asset=Tag,baseBoard.location=Chassis
 """, "singleton-config-3", predefine_check="5.3.0")
@ -600,6 +603,7 @@ source.reservations.managed=no,source.reservations.source.type=unix,source.reser

 --input type=keyboard,bus=usb
 --input tablet
+--input mouse

 --serial tcp,host=:2222,mode=bind,protocol=telnet,log.file=/tmp/foo.log,log.append=yes,,target.model.name=pci-serial
 --serial nmdm,source.master=/dev/foo1,source.slave=/dev/foo2,alias.name=testalias7
@ -898,7 +902,7 @@ c.add_compare("--connect %(URI-KVM-ARMV7L)s --disk %(EXISTIMG1)s --import --os-v
 c.add_compare("--arch aarch64 --machine virt --boot kernel=/f19-arm.kernel,initrd=/f19-arm.initrd,kernel_args=\"console=ttyAMA0,1234 rw root=/dev/vda3\" --disk %(EXISTIMG1)s", "aarch64-machvirt")
 c.add_compare("--arch aarch64 --boot kernel=/f19-arm.kernel,initrd=/f19-arm.initrd,kernel_args=\"console=ttyAMA0,1234 rw root=/dev/vda3\" --disk %(EXISTIMG1)s", "aarch64-machdefault")
 c.add_compare("--arch aarch64 --cdrom %(EXISTIMG2)s --boot loader=CODE.fd,nvram.template=VARS.fd --disk %(EXISTIMG1)s --cpu none --events on_crash=preserve,on_reboot=destroy,on_poweroff=restart", "aarch64-cdrom")
-c.add_compare("--connect %(URI-KVM-AARCH64)s --disk %(EXISTIMG1)s --import --os-variant fedora21", "aarch64-kvm-import")
+c.add_compare("--connect %(URI-KVM-AARCH64)s --disk %(EXISTIMG1)s --import --os-variant fedora21 --panic default", "aarch64-kvm-import")  # the --panic is a no-op
 c.add_compare("--connect %(URI-KVM-AARCH64)s --disk size=1 --os-variant fedora22 --features gic_version=host --network network=default,address.type=pci --controller type=scsi,model=virtio-scsi,address.type=pci", "aarch64-kvm-gic")


@ -926,6 +930,7 @@ c.add_compare("--connect %(URI-KVM)s --arch x86_64", "x86_64-graphics")
 ######################

 c = vinst.add_category("lxc", "--name foolxc --memory 64 --noautoconsole --connect " + utils.URIs.lxc)
+c.add_invalid("--filesystem /,not/abs")  # filesystem target is not absolute
 c.add_compare("", "default")
 c.add_compare("--os-variant fedora27", "default-f27")
 c.add_compare("--filesystem /source,/", "fs-default")
@ -940,7 +945,7 @@ c.add_compare("--init /usr/bin/httpd", "manual-init")
 c = vinst.add_category("xen", "--noautoconsole --connect " + utils.URIs.xen)
 c.add_valid("--disk %(EXISTIMG1)s --location %(TREEDIR)s --paravirt --graphics none")  # Xen PV install headless
 c.add_compare("--disk %(EXISTIMG1)s --import", "xen-default")  # Xen default
-c.add_compare("--disk %(EXISTIMG1)s --location %(TREEDIR)s --paravirt --controller xenbus,maxGrantFrames=64", "xen-pv", precompare_check="5.3.0")  # Xen PV
+c.add_compare("--disk %(EXISTIMG1)s --location %(TREEDIR)s --paravirt --controller xenbus,maxGrantFrames=64 --input default", "xen-pv", precompare_check="5.3.0")  # Xen PV
 c.add_compare("--disk  /iscsi-pool/diskvol1 --cdrom %(EXISTIMG1)s --livecd --hvm", "xen-hvm")  # Xen HVM


--- a/tests/nodedev.py
+++ b/tests/nodedev.py
@ -109,6 +109,7 @@ class TestNodeDev(unittest.TestCase):
                "/dev/dri/by-path/pci-0000:00:02.0-render")
        self.assertEqual(dev.devnodes[1].node_type, "link")
        self.assertEqual(dev.is_drm_render(), True)
+        self.assertTrue(dev.get_devnode("frob"))


    # NodeDevice 2 Device XML tests
--- a/tests/uriparse.py
+++ b/tests/uriparse.py
@ -7,6 +7,8 @@ import unittest

 from virtinst import URI

+import tests
+

 class TestURI(unittest.TestCase):
    """
@ -53,3 +55,9 @@ class TestURI(unittest.TestCase):
            "qemu+ssh://user%40domain.org@hostname/system",
            scheme="qemu", path="/system", transport="ssh",
            hostname="hostname", username="user@domain.org")
+
+    def test_magicuri_connver(self):
+        uri = tests.utils.URIs.test_default + ",connver=1,libver=2"
+        conn = tests.utils.URIs.openconn(uri)
+        self.assertEqual(conn.conn_version(), 1)
+        self.assertEqual(conn.local_libvirt_version(), 2)
--- a/tests/utils.py
+++ b/tests/utils.py
@ -170,17 +170,8 @@ class _URIs(object):
            self._testdriver_default = self.openconn(self.test_default)
        return self._testdriver_default

-    def _make_uri(self, base, connver=None, libver=None):
-        if connver:
-            base += ",connver=%s" % connver
-        if libver:
-            base += ",libver=%s" % libver
-        return base
-
-    def open_kvm(self, connver=None, libver=None):
-        return self.openconn(self._make_uri(self.kvm, connver, libver))
-    def open_kvm_rhel(self, connver=None):
-        return self.openconn(self._make_uri(self.kvm_rhel, connver))
+    def open_kvm(self):
+        return self.openconn(self.kvm)
    def open_test_remote(self):
        return self.openconn(self.test_remote)

--- a/tests/xmlparse-xml/change-boot-multi-out.xml
+++ b/tests/xmlparse-xml/change-boot-multi-out.xml
@ -12,6 +12,8 @@
    <cmdline>ks=foo.ks</cmdline>
    <dtb>/baz.dtb</dtb>
    <boot dev="cdrom"/>
+    <initarg>baz</initarg>
+    <initarg>wibble</initarg>
  </os>
  <features>
    <acpi/><apic/>
--- a/tests/xmlparse-xml/change-seclabel-out.xml
+++ b/tests/xmlparse-xml/change-seclabel-out.xml
@ -12,7 +12,7 @@
  </devices>
  <seclabel type="none" model="apparmor" relabel="no">
    <label>barlabel</label>
-    <imagelabel>fooimage</imagelabel>
+    <imagelabel>imagelabel</imagelabel>
    <baselabel>baselabel</baselabel>
  </seclabel>
  <seclabel type="dynamic" model="dac" relabel="yes"/>
--- a/tests/xmlparse.py
+++ b/tests/xmlparse.py
@ -305,6 +305,9 @@ class XMLParseTest(unittest.TestCase):
        check("dtb", None, "/baz.dtb")
        check("kernel_args", None, "ks=foo.ks")

+        guest.os.set_initargs_string("foo bar")
+        guest.os.set_initargs_string("baz wibble")
+
        self._alter_compare(guest.get_xml(), outfile)

    def testAlterBootKernel(self):
@ -1293,10 +1296,6 @@ class XMLParseTest(unittest.TestCase):
        check("macaddr", None, "52:54:00:69:eb:FF")
        check("virtualport_type", None, "openvswitch")

-        check = self._make_checker(net.forward)
-        check("mode", "nat", "route")
-        check("dev", None, "eth22")
-
        check = self._make_checker(net.bandwidth)
        check("inbound_average", "1000", "3000")
        check("inbound_peak", "5000", "4000")
@ -1315,9 +1314,16 @@ class XMLParseTest(unittest.TestCase):
        check = self._make_checker(net.ips[0])
        check("address", "192.168.7.1", "192.168.8.1")
        check("netmask", "255.255.255.0", "255.255.254.0")
+        self.assertEqual(net.can_pxe(), False)
        check("tftp", None, "/var/lib/tftproot")
        check("bootp_file", None, "pxeboot.img")
        check("bootp_server", None, "1.2.3.4")
+        self.assertEqual(net.can_pxe(), True)
+
+        check = self._make_checker(net.forward)
+        check("mode", "nat", "route")
+        check("dev", None, "eth22")
+        self.assertEqual(net.can_pxe(), True)

        check = self._make_checker(net.ips[0].ranges[0])
        check("start", "192.168.7.128", "192.168.8.128")
--- a/virtinst/capabilities.py
+++ b/virtinst/capabilities.py
@ -220,22 +220,6 @@ class Capabilities(XMLBuilder):
    guests = XMLChildProperty(_CapsGuest)


-    ###################
-    # Private helpers #
-    ###################
-
-    def _is_xen(self):
-        for g in self.guests:
-            if g.os_type != "xen":
-                continue
-
-            for d in g.domains:
-                if d.hypervisor_type == "xen":
-                    return True
-
-        return False
-
-
    ############################
    # Public XML building APIs #
    ############################
--- a/virtinst/cli.py
+++ b/virtinst/cli.py
@ -3525,7 +3525,7 @@ class ParserMemballoon(VirtCLIParser):
        VirtCLIParser._init_class(**kwargs)
        _add_common_device_args(cls, virtio_options=True)

-        cls.add_arg("model", "model")
+        cls.add_arg("model", "model", ignore_default=True)
        cls.add_arg("autodeflate", "autodeflate", is_onoff=True)
        cls.add_arg("stats.period", "stats_period")

@ -3578,7 +3578,7 @@ class ParserVsock(VirtCLIParser):
        VirtCLIParser._init_class(**kwargs)
        _add_common_device_args(cls)

-        cls.add_arg("model", "model")
+        cls.add_arg("model", "model", ignore_default=True)
        cls.add_arg("cid.auto", "auto_cid", is_onoff=True)
        cls.add_arg("cid.address", "cid")

--- a/virtinst/devices/hostdev.py
+++ b/virtinst/devices/hostdev.py
@ -13,12 +13,6 @@ class DeviceHostdev(Device):
    XML_NAME = "hostdev"

    def set_from_nodedev(self, nodedev):
-        """
-        @use_full_usb: If set, and nodedev is USB, specify both
-            vendor and product. Used if user requests bus/add on virt-install
-            command line, or if virt-manager detects a dup USB device
-            and we need to differentiate
-        """
        if nodedev.device_type == NodeDevice.CAPABILITY_TYPE_PCI:
            self.type = "pci"
            self.domain = nodedev.domain
@ -32,18 +26,12 @@ class DeviceHostdev(Device):
            self.product = nodedev.product_id

            count = 0
-
            for dev in self.conn.fetch_all_nodedevs():
                if (dev.device_type == NodeDevice.CAPABILITY_TYPE_USBDEV and
                    dev.vendor_id == self.vendor and
                    dev.product_id == self.product):
                    count += 1

-            if not count:
-                raise RuntimeError(_("Could not find USB device "
-                                     "(vendorId: %s, productId: %s)")
-                                   % (self.vendor, self.product))
-
            if count > 1:
                self.bus = nodedev.bus
                self.device = nodedev.device
@ -114,5 +102,3 @@ class DeviceHostdev(Device):
            self.managed = self.conn.is_xen() and "no" or "yes"
        if not self.mode:
            self.mode = "subsystem"
-        if self.type == "pci" and not self.domain:
-            self.domain = "0x0"
--- a/virtinst/devices/panic.py
+++ b/virtinst/devices/panic.py
@ -46,4 +46,4 @@ class DevicePanic(Device):
        if not self.address.type and self.address.iobase:
            self.address.type = "isa"
        if not self.model:
-            self.model = self.get_default_model(guest)
+            self.model = DevicePanic.get_default_model(guest)
--- a/virtinst/domain/os.py
+++ b/virtinst/domain/os.py
@ -48,8 +48,6 @@ class DomainOs(XMLBuilder):
        return self.arch == "aarch64"
    def is_arm(self):
        return self.is_arm32() or self.is_arm64()
-    def is_arm_vexpress(self):
-        return self.is_arm() and str(self.machine).startswith("vexpress-")
    def is_arm_machvirt(self):
        return self.is_arm() and str(self.machine).startswith("virt")

--- a/virtinst/generatename.py
+++ b/virtinst/generatename.py
@ -65,6 +65,7 @@ def generate_name(base, collision_cb, suffix="", lib_collision=True,
    if not force_num:
        numrange = [None] + numrange

+    ret = None
    for i in numrange:
        tryname = base
        if i is not None:
@ -72,6 +73,8 @@ def generate_name(base, collision_cb, suffix="", lib_collision=True,
        tryname += suffix

        if not collide(tryname):
-            return tryname
+            ret = tryname
+            break

-    raise ValueError(_("Name generation range exceeded."))
+    assert ret
+    return ret
--- a/virtinst/nodedev.py
+++ b/virtinst/nodedev.py
@ -58,10 +58,6 @@ class NodeDevice(XMLBuilder):

        :returns: NodeDevice instance
        """
-        if not conn.support.conn_nodedev():
-            raise ValueError(_("Connection does not support host device "
-                               "enumeration."))
-
        # First try and see if this is a libvirt nodedev name
        for nodedev in conn.fetch_all_nodedevs():
            if nodedev.name == idstring:
@ -171,7 +167,6 @@ class NodeDevice(XMLBuilder):
                return d
        if len(self.devnodes) > 0:
            return self.devnodes[0]
-        return None


 def _AddressStringToHostdev(conn, addrstr):
--- a/virtinst/uri.py
+++ b/virtinst/uri.py
@ -124,8 +124,7 @@ class MagicURI(object):
        return uri.startswith(MagicURI.VIRTINST_URI_MAGIC_PREFIX)

    def __init__(self, uri):
-        if not self.uri_is_magic(uri):
-            raise RuntimeError("uri=%s is not virtinst magic URI" % uri)
+        assert self.uri_is_magic(uri)

        from .cli import parse_optstr_tuples

@ -165,8 +164,7 @@ class MagicURI(object):
        if self.libvirt_version:
            self.libvirt_version = int(self.libvirt_version)

-        if opts:
-            raise RuntimeError("Unhandled virtinst test uri options %s" % opts)
+        assert not opts


    ##############
--- a/virtinst/xmlutil.py
+++ b/virtinst/xmlutil.py
@ -19,14 +19,12 @@ def xml_escape(xml):
    """
    Replaces chars ' " < > & with xml safe counterparts
    """
-    if xml is None:
-        return None
-
-    xml = xml.replace("&", "&amp;")
-    xml = xml.replace("'", "&apos;")
-    xml = xml.replace("\"", "&quot;")
-    xml = xml.replace("<", "&lt;")
-    xml = xml.replace(">", "&gt;")
+    if xml:
+        xml = xml.replace("&", "&amp;")
+        xml = xml.replace("'", "&apos;")
+        xml = xml.replace("\"", "&quot;")
+        xml = xml.replace("<", "&lt;")
+        xml = xml.replace(">", "&gt;")
    return xml