Skip to content

Commit

Permalink
test: Avoid alternative truths
Browse files Browse the repository at this point in the history 
These make tests unpredictable, and are prone to pollute the VM after a
test and influence the next test.

In many cases the commands aren't expected to fail. For the ones which
are, add explicit conditions.
  • Loading branch information
@martinpitt @jelly
martinpitt authored and jelly committed Feb 4, 2024
1 parent 3ac3d8f commit dd6d58f
Show file tree
Hide file tree
Showing 10 changed files with 38 additions and 36 deletions.
12 changes: 6 additions & 6 deletions test/check-machines-create
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1601,7 +1601,7 @@ vnc_password= "{vnc_passwd}"
new_fedora_28_xml = ET.tostring(root)
self.machine.execute(f"echo '{str(new_fedora_28_xml, 'utf-8')}' > {self.test_obj.vm_tmpdir}/fedora-28.xml")
self.machine.execute(f"mount -o bind {self.test_obj.vm_tmpdir}/fedora-28.xml /usr/share/osinfo/os/fedoraproject.org/fedora-28.xml")
self.test_obj.addCleanup(self.machine.execute, "umount /usr/share/osinfo/os/fedoraproject.org/fedora-28.xml || true")
self.test_obj.addCleanup(self.machine.execute, "umount /usr/share/osinfo/os/fedoraproject.org/fedora-28.xml")

# Fake distro with no minimum ram or storage defined
fedora_29_xml = self.machine.execute("cat /usr/share/osinfo/os/fedoraproject.org/fedora-29.xml")
Expand All @@ -1613,7 +1613,7 @@ vnc_password= "{vnc_passwd}"
new_fedora_29_xml = ET.tostring(root)
self.machine.execute(f"echo '{str(new_fedora_29_xml, 'utf-8')}' > {self.test_obj.vm_tmpdir}/fedora-29.xml")
self.machine.execute(f"mount -o bind {self.test_obj.vm_tmpdir}/fedora-29.xml /usr/share/osinfo/os/fedoraproject.org/fedora-29.xml")
self.test_obj.addCleanup(self.machine.execute, "umount /usr/share/osinfo/os/fedoraproject.org/fedora-29.xml || true")
self.test_obj.addCleanup(self.machine.execute, "umount /usr/share/osinfo/os/fedoraproject.org/fedora-29.xml")

def _fakeOsDBInfoRhel(self):
# Fake rhel 8 images
Expand All @@ -1625,7 +1625,7 @@ vnc_password= "{vnc_passwd}"
new_rhel_8_1_xml = ET.tostring(root)
self.machine.execute(f"echo '{str(new_rhel_8_1_xml, 'utf-8')}' > {self.test_obj.vm_tmpdir}/rhel-8.1.xml")
self.machine.execute(f"mount -o bind {self.test_obj.vm_tmpdir}/rhel-8.1.xml /usr/share/osinfo/os/redhat.com/rhel-8.1.xml")
self.test_obj.addCleanup(self.machine.execute, "umount /usr/share/osinfo/os/redhat.com/rhel-8.1.xml || true")
self.test_obj.addCleanup(self.machine.execute, "umount /usr/share/osinfo/os/redhat.com/rhel-8.1.xml")

rhel_8_2_xml = self.machine.execute("cat /usr/share/osinfo/os/redhat.com/rhel-8.2.xml")
root = ET.fromstring(rhel_8_2_xml)
Expand All @@ -1635,7 +1635,7 @@ vnc_password= "{vnc_passwd}"
new_rhel_8_2_xml = ET.tostring(root)
self.machine.execute(f"echo '{str(new_rhel_8_2_xml, 'utf-8')}' > {self.test_obj.vm_tmpdir}/rhel-8.2.xml")
self.machine.execute(f"mount -o bind {self.test_obj.vm_tmpdir}/rhel-8.2.xml /usr/share/osinfo/os/redhat.com/rhel-8.2.xml")
self.test_obj.addCleanup(self.machine.execute, "umount /usr/share/osinfo/os/redhat.com/rhel-8.2.xml || true")
self.test_obj.addCleanup(self.machine.execute, "umount /usr/share/osinfo/os/redhat.com/rhel-8.2.xml")

# Fake rhel 7 image
rhel_7_1_xml = self.machine.execute("cat /usr/share/osinfo/os/redhat.com/rhel-7.1.xml")
Expand All @@ -1645,7 +1645,7 @@ vnc_password= "{vnc_passwd}"
new_rhel_7_1_xml = ET.tostring(root)
self.machine.execute(f"echo '{str(new_rhel_7_1_xml, 'utf-8')}' > {self.test_obj.vm_tmpdir}/rhel-7.1.xml")
self.machine.execute(f"mount -o bind {self.test_obj.vm_tmpdir}/rhel-7.1.xml /usr/share/osinfo/os/redhat.com/rhel-7.1.xml")
self.test_obj.addCleanup(self.machine.execute, "umount /usr/share/osinfo/os/redhat.com/rhel-7.1.xml || true")
self.test_obj.addCleanup(self.machine.execute, "umount /usr/share/osinfo/os/redhat.com/rhel-7.1.xml")

def _fakeFedoraTree(self):
server = self.test_obj.setup_mock_server("mock-range-server.py", ["archive.fedoraproject.org"])
Expand Down Expand Up @@ -2146,7 +2146,7 @@ vnc_password= "{vnc_passwd}"
raise AssertionError("Unhandled distro for OVMF path")

m.execute("mount -t tmpfs tmpfs " + ovmf_path)
self.addCleanup(m.execute, f"umount {ovmf_path} || true")
self.addCleanup(m.execute, f"if mountpoint {ovmf_path}; then umount {ovmf_path}; fi")

# Reload for the new configuration to be read
b.reload()
Expand Down
3 changes: 2 additions & 1 deletion test/check-machines-disks
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1238,7 +1238,8 @@ class TestMachinesDisks(machineslib.VirtualMachinesCase):

# Ensure that adding disks with custom path is possible when no storage pools are present
# https://bugzilla.redhat.com/show_bug.cgi?id=1985228
m.execute("for pool in $(virsh pool-list --all --name); do virsh pool-destroy $pool || true; virsh pool-undefine $pool; done")
m.execute("for pool in $(virsh pool-list --name); do virsh pool-destroy $pool; done")
m.execute("for pool in $(virsh pool-list --all --name); do virsh pool-undefine $pool; done")
b.wait_visible("#vm-details[data-pools-count='0']")
b.click(prefix)
b.click(f"{prefix}-custompath")
Expand Down
2 changes: 1 addition & 1 deletion test/check-machines-filesystems
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ class TestMachinesFilesystems(machineslib.VirtualMachinesCase):
b.wait_in_text("tr[data-row-id='vm-subVmTest1-filesystem-/tmp/dir1/-dir1'] td[data-label='Mount tag']", "dir1")

domain_xml = "virsh -c qemu:///system dumpxml subVmTest1"
xmllint_element = f"{domain_xml} | xmllint --xpath 'string(//domain/{{prop}})' - 2>&1 || true"
xmllint_element = f"{domain_xml} | xmllint --xpath 'string(//domain/{{prop}})' -"

self.assertEqual('/tmp/dir1/', self.machine.execute(xmllint_element.format(prop='devices/filesystem/source/@dir')).strip())
self.assertEqual('dir1', self.machine.execute(xmllint_element.format(prop='devices/filesystem/target/@dir')).strip())
Expand Down
20 changes: 10 additions & 10 deletions test/check-machines-hostdevs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,26 +190,26 @@ class TestMachinesHostDevs(machineslib.VirtualMachinesCase):
m.execute("diff /tmp/vmdir/vmxml1 /tmp/vmdir/vmxml2 | sed -e 's/^>//;1d' > /tmp/vmdir/vmdiff")

if self.dev_type == "usb_device":
vendor_id = m.execute("cat /tmp/vmdir/vmdiff | xmllint --xpath 'string(//hostdev/source/vendor/@id)' - 2>&1 || true").strip()
product_id = m.execute("cat /tmp/vmdir/vmdiff | xmllint --xpath 'string(//hostdev/source/product/@id)' - 2>&1 || true").strip()
vendor_id = m.execute("cat /tmp/vmdir/vmdiff | xmllint --xpath 'string(//hostdev/source/vendor/@id)' -").strip()
product_id = m.execute("cat /tmp/vmdir/vmdiff | xmllint --xpath 'string(//hostdev/source/product/@id)' -").strip()

output = self.run_admin(f"virsh -c qemu:///{connectionName} nodedev-list --cap {self.dev_type}", connectionName)
devices = output.splitlines()
devices = list(filter(None, devices))
for dev in devices:
if self.dev_type == "usb_device":
self.run_admin(f"virsh -c qemu:///{connectionName} nodedev-dumpxml --device {dev} > /tmp/vmdir/nodedevxml", connectionName)
vendor = m.execute(f"xmllint --xpath 'string(//device/capability/vendor[starts-with(@id, \"{vendor_id}\")])' - 2>&1 < /tmp/vmdir/nodedevxml || true")
product = m.execute(f"xmllint --xpath 'string(//device/capability/product[starts-with(@id, \"{product_id}\")])' < /tmp/vmdir/nodedevxml - 2>&1 || true")
vendor = m.execute(f"xmllint --xpath 'string(//device/capability/vendor[starts-with(@id, \"{vendor_id}\")])' - 2>&1 < /tmp/vmdir/nodedevxml")
product = m.execute(f"xmllint --xpath 'string(//device/capability/product[starts-with(@id, \"{product_id}\")])' < /tmp/vmdir/nodedevxml - 2>&1")

if vendor.strip() == self._vendor and product.strip() == self._model:
return

elif self.dev_type == "pci":
domain = int(m.execute("cat /tmp/vmdir/vmdiff | xmllint --xpath 'string(//hostdev/source/address/@domain)' - 2>&1 || true"), base=16)
bus = int(m.execute("cat /tmp/vmdir/vmdiff | xmllint --xpath 'string(//hostdev/source/address/@bus)' - 2>&1 || true"), base=16)
slot = int(m.execute("cat /tmp/vmdir/vmdiff | xmllint --xpath 'string(//hostdev/source/address/@slot)' - 2>&1 || true"), base=16)
func = int(m.execute("cat /tmp/vmdir/vmdiff | xmllint --xpath 'string(//hostdev/source/address/@function)' - 2>&1 || true"), base=16)
domain = int(m.execute("cat /tmp/vmdir/vmdiff | xmllint --xpath 'string(//hostdev/source/address/@domain)' -"), base=16)
bus = int(m.execute("cat /tmp/vmdir/vmdiff | xmllint --xpath 'string(//hostdev/source/address/@bus)' -"), base=16)
slot = int(m.execute("cat /tmp/vmdir/vmdiff | xmllint --xpath 'string(//hostdev/source/address/@slot)' -"), base=16)
func = int(m.execute("cat /tmp/vmdir/vmdiff | xmllint --xpath 'string(//hostdev/source/address/@function)' -"), base=16)

slot_parts = re.split(r":|\.", self._slot)

Expand All @@ -233,11 +233,11 @@ class TestMachinesHostDevs(machineslib.VirtualMachinesCase):
dialog = HostDevAddDialog(self, fail_message="No host device selected")
dialog.open()
dialog.add()
testlib.wait(lambda: "true" == m.execute(f"virsh -c qemu:///{connectionName} dumpxml subVmTest1 | grep -A 5 hostdev || echo true").strip())
m.execute(f"while virsh -c qemu:///{connectionName} dumpxml subVmTest1 | grep -A 5 hostdev; do sleep 1; done")
# Check no host devices attached after shutting off the VM
self.performAction("subVmTest1", "forceOff", connectionName=connectionName)
b.wait_in_text(f"#vm-subVmTest1-{connectionName}-state", "Shut off")
testlib.wait(lambda: "true" == m.execute("virsh -c qemu:///{connectionName} dumpxml subVmTest1 | grep -A 5 hostdev || echo true").strip())
m.execute(f"while virsh -c qemu:///{connectionName} dumpxml subVmTest1 | grep -A 5 hostdev; do sleep 1; done")

output = self.run_admin(f"virsh -c qemu:///{connectionName} nodedev-list --cap usb_device", connectionName)
if output.strip() != "":
Expand Down
2 changes: 1 addition & 1 deletion test/check-machines-networks
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ class TestMachinesNetworks(machineslib.VirtualMachinesCase):

# Verify libvirt XML
net_xml = f"virsh -c qemu:///system net-dumpxml {self.name}"
xmllint_element = f"{net_xml} | xmllint --xpath 'string(//network/{{prop}})' - 2>&1 || true"
xmllint_element = f"{net_xml} | xmllint --xpath 'string(//network/{{prop}})' -"

self.test_obj.assertEqual(self.name, m.execute(xmllint_element.format(prop='name')).strip())
if (self.forward_mode == "none"):
Expand Down
4 changes: 2 additions & 2 deletions test/check-machines-nics
View file
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ class TestMachinesNICs(machineslib.VirtualMachinesCase):
def verify(self):
dom_xml = "virsh -c qemu:///system dumpxml --domain subVmTest1"
mac_string = f'"{self.mac}"'
xmllint_element = f"{dom_xml} | xmllint --xpath 'string(//domain/devices/interface[starts-with(mac/@address,{mac_string})]/{{prop}})' - 2>&1 || true"
xmllint_element = f"{dom_xml} | xmllint --xpath 'string(//domain/devices/interface[starts-with(mac/@address,{mac_string})]/{{prop}})' -"

if self.source_type == "network":
self.assertEqual(
Expand Down Expand Up @@ -730,7 +730,7 @@ class TestMachinesNICs(machineslib.VirtualMachinesCase):
# Verify libvirt XML
dom_xml = "virsh -c qemu:///system dumpxml --domain subVmTest1"
mac_string = f'"{self.mac}"'
xmllint_element = f"{dom_xml} | xmllint --xpath 'string(//domain/devices/interface[starts-with(mac/@address,{mac_string})]/{{prop}})' - 2>&1 || true"
xmllint_element = f"{dom_xml} | xmllint --xpath 'string(//domain/devices/interface[starts-with(mac/@address,{mac_string})]/{{prop}})' -"

if (self.source_type == "network"):
self.assertEqual("network", self.machine.execute(xmllint_element.format(prop='@type')).strip())
Expand Down
6 changes: 3 additions & 3 deletions test/check-machines-settings
View file
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ class TestMachinesSettings(machineslib.VirtualMachinesCase):

# Verify libvirt XML
dom_xml = "virsh dumpxml subVmTest1"
xmllint_element = f"{dom_xml} | xmllint --xpath 'string(//domain/{{prop}})' - 2>&1 || true"
xmllint_element = f"{dom_xml} | xmllint --xpath 'string(//domain/{{prop}})' -"
self.assertEqual("coreduo", m.execute(xmllint_element.format(prop='cpu/model')).strip())

# Host-model gets expanded to custom mode when the VM is running
Expand Down Expand Up @@ -481,10 +481,10 @@ class TestMachinesSettings(machineslib.VirtualMachinesCase):

dom_xml = "virsh -c qemu:///system dumpxml subVmTest1"

xmllint_elem = f"{dom_xml} | xmllint --xpath 'string(//domain/cpu/@mode)' - 2>&1 || true"
xmllint_elem = f"{dom_xml} | xmllint --xpath 'string(//domain/cpu/@mode)' -"
testlib.wait(lambda: cpu_model in m.execute(xmllint_elem).strip())

xmllint_elem = f"{dom_xml} | xmllint --xpath 'string(//domain/vcpu)' - 2>&1 || true"
xmllint_elem = f"{dom_xml} | xmllint --xpath 'string(//domain/vcpu)' -"
testlib.wait(lambda: "3" in m.execute(xmllint_elem).strip())

virsh_output = m.execute("virsh dumpxml subVmTest1 | xmllint --xpath '/domain/devices/watchdog/@action' -").strip()
Expand Down
2 changes: 1 addition & 1 deletion test/check-machines-snapshots
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ class TestMachinesSnapshots(machineslib.VirtualMachinesCase):
def verify_backend(self):
# Verify libvirt XML
snap_xml = f"virsh -c qemu:///system snapshot-dumpxml --domain subVmTest1 --snapshotname {self.name}"
xmllint_element = f"{snap_xml} | xmllint --xpath 'string(//domainsnapshot/{{prop}})' - 2>&1 || true"
xmllint_element = f"{snap_xml} | xmllint --xpath 'string(//domainsnapshot/{{prop}})' -"

if (self.name):
self.test_obj.assertEqual(self.name, m.execute(xmllint_element.format(prop='name')).strip())
Expand Down
5 changes: 2 additions & 3 deletions test/check-machines-storage-pools
View file
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ class TestMachinesStoragePools(machineslib.VirtualMachinesCase):

# Verify libvirt XML
pool_xml = f"virsh -c qemu:///system pool-dumpxml {self.name}"
xmllint_element = f"{pool_xml} | xmllint --xpath 'string(//pool/{{prop}})' - 2>&1 || true"
xmllint_element = f"{pool_xml} | xmllint --xpath 'string(//pool/{{prop}})' -"

self.test_obj.assertEqual(self.name, m.execute(xmllint_element.format(prop='name')).strip())
if (self.target):
Expand Down Expand Up @@ -519,7 +519,6 @@ class TestMachinesStoragePools(machineslib.VirtualMachinesCase):
m.execute("systemctl restart nfs-server")
m.execute(f"virsh pool-define-as nfs-pool --type netfs --target {nfs_pool} --source-host 127.0.0.1 --source-path {mnt_exports}")
m.execute("virsh pool-start nfs-pool")
self.addCleanup(m.execute, "virsh pool-destroy nfs-pool || true") # Destroy pool as it block removal of `nfs_pool`

# Prepare disk/block pool
dev = self.add_ram_disk(10)
Expand Down Expand Up @@ -607,7 +606,7 @@ class TestMachinesStoragePools(machineslib.VirtualMachinesCase):

# Verify libvirt XML
vol_xml = f"virsh -c qemu:///system vol-dumpxml --pool {self.pool_name} --vol {self.vol_name}"
xmllint_element = f"{vol_xml} | xmllint --xpath 'string(//volume/{{prop}})' - 2>&1 || true"
xmllint_element = f"{vol_xml} | xmllint --xpath 'string(//volume/{{prop}})' -"

self.test_obj.assertEqual(self.vol_name, m.execute(xmllint_element.format(prop='name')).strip())

Expand Down
18 changes: 10 additions & 8 deletions test/machineslib.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ def waitNetworkRow(self, networkName, connectionName="system", present=True):

def getDomainMacAddress(self, vmName):
dom_xml = f"virsh -c qemu:///system dumpxml --domain {vmName}"
return self.machine.execute(f"{dom_xml} | xmllint --xpath 'string(//domain/devices/interface/mac/@address)' - 2>&1 || true").strip()
return self.machine.execute(f"{dom_xml} | xmllint --xpath 'string(//domain/devices/interface/mac/@address)' -").strip()

def getLibvirtServiceName(self):
m = self.machine
Expand All @@ -142,9 +142,11 @@ def startLibvirt(self, m):
# Wait until we can get a list of domains
m.execute("until virsh list; do sleep 1; done")

# Wait for the network 'default' to become active
m.execute("virsh net-define /etc/libvirt/qemu/networks/default.xml || true")
m.execute("virsh net-start default || true")
# Create/activate network 'default' if necessary
m.execute("""if ! out=$(virsh net-info default); then
virsh net-define /etc/libvirt/qemu/networks/default.xml
fi
if ! echo "$out" | grep -q 'Active.*yes'; then virsh net-start default; fi""")
m.execute(r"until virsh net-info default | grep 'Active:\s*yes'; do sleep 1; done")

def createVm(self, name, graphics='none', ptyconsole=False, running=True, memory=128, connection='system', machine=None, os="cirros0.4.0"):
Expand Down Expand Up @@ -231,7 +233,7 @@ def prepareStorageDeviceOnISCSI(self, target_iqn):
""" % {"tgt": target_iqn, "ini": orig_iqn})

self.addCleanup(m.execute, "targetcli /backstores/ramdisk delete test")
self.addCleanup(m.execute, "targetcli /iscsi delete %s; iscsiadm -m node -o delete || true" % target_iqn)
self.addCleanup(m.execute, f"targetcli /iscsi delete {target_iqn}; iscsiadm -m node -o delete")
return orig_iqn

def run_admin(self, cmd, connectionName='system', machine=None):
Expand Down Expand Up @@ -370,7 +372,7 @@ def setUp(self):

# Stop all domains
for connection in ["system", "session"]:
cmd = f"for d in $(virsh -c qemu:///{connection} list --name); do virsh -c qemu:///{connection} destroy $d || true; done"
cmd = f"for d in $(virsh -c qemu:///{connection} list --name); do virsh -c qemu:///{connection} destroy $d; done"
if connection == "session":
cmd += f"; for d in $(virsh -c qemu:///{connection} list --all --name); do virsh -c qemu:///{connection} undefine $d; done"
cmd = f"runuser -l admin -c '{cmd}'"
Expand All @@ -381,7 +383,7 @@ def setUp(self):

# Stop all pools
for connection in ["system", "session"]:
cmd = f"for n in $(virsh -c qemu:///{connection} pool-list --name); do virsh -c qemu:///{connection} pool-destroy $n || true; done"
cmd = f"for n in $(virsh -c qemu:///{connection} pool-list --name); do virsh -c qemu:///{connection} pool-destroy $n; done"
if connection == "session":
cmd += f"; for d in $(virsh -c qemu:///{connection} pool-list --all --name); do virsh -c qemu:///{connection} pool-undefine $d; done"
cmd = f"runuser -l admin -c '{cmd}'"
Expand All @@ -392,7 +394,7 @@ def setUp(self):

# Stop all networks
for connection in ["system", "session"]:
cmd = f"for n in $(virsh -c qemu:///{connection} net-list --name); do virsh -c qemu:///{connection} net-destroy $n || true; done"
cmd = f"for n in $(virsh -c qemu:///{connection} net-list --name); do virsh -c qemu:///{connection} net-destroy $n; done"
if connection == "session":
cmd += f"; for d in $(virsh -c qemu:///{connection} net-list --all --name); do virsh -c qemu:///{connection} net-undefine $d; done"
cmd = f"runuser -l admin -c '{cmd}'"
Expand Down

0 comments on commit dd6d58f

Please sign in to comment.