Re: [PATCH 03/10] uitests: Don't fail when running on older libvirt versions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 09/04/2018 07:26 PM, Povilas Kanapickas wrote:
On 04/09/2018 20:21, Cole Robinson wrote:
On 09/01/2018 06:21 PM, Povilas Kanapickas wrote:
Instead of completely failing test when older libvirt is used, we can
verify the error message instead.


Actually this is fixed in git already, I fixed it over the weekend while
doing some other work

commit 83a9e613e9fa7d7c7d9ac9b227a8a535618cadb3
Author: Cole Robinson <crobinso@xxxxxxxxxx>
Date:   Fri Aug 31 16:47:10 2018 -0400

     tests: uitests: Fix breakage

Signed-off-by: Povilas Kanapickas <povilas@xxxxxxxx>
---
   tests/uitests/addhardware.py | 15 +++++++++++++++
   1 file changed, 15 insertions(+)

diff --git a/tests/uitests/addhardware.py b/tests/uitests/addhardware.py
index ae5e1153..541752bf 100644
--- a/tests/uitests/addhardware.py
+++ b/tests/uitests/addhardware.py
@@ -27,6 +27,19 @@ class AddHardware(uiutils.UITestCase):
           uiutils.check_in_loop(lambda: tab.showing)
           return tab
   +    def _cancel_hw_change_if_config_unsupported(self, addhw_window,
alert_msg):
+        try:
+            alert = self.app.root.find("vmm dialog", "alert")
+        except Exception:
+            return  # no alert, ignore
+
+        # Verify that the found alert is the actual alert we expected
+        alert.find_fuzzy("Error adding device", "label")
+        alert.find_fuzzy(alert_msg, "label")
+        alert.find("Close", "push button").click()
+        uiutils.check_in_loop(lambda: alert.dead)
+
+        addhw_window.find("Cancel", "push button").click()
         ##############
       # Test cases #
@@ -533,6 +546,8 @@ class AddHardware(uiutils.UITestCase):
           tab.find("Passthrough", "menu item").click()
           tab.find("Device Path:", "text").text = "/tmp/foo"
           finish.click()
+        self._cancel_hw_change_if_config_unsupported(addhw,
+            'Unknown TPM frontend model')  # TPM supported since
libvirt 4.5
           uiutils.check_in_loop(lambda: details.active)
             # Add RNG


In general though I wouldn't want to do this type of handling in the
test suite to catch an error on older libvirt, I'd rather adapt the test
case to work on both versions, or skip the test entirely and only run
when libvirt is new enough.

 From users perspective virt-manager should work on various versions of
libvirt, shouldn't it? What if alert was dismissed as follows:

if libvirt_version < 4.5.0:
     self._check_hw_change_unsupported(...)

The _check_hw_change_unsupported function would be written to fail if
the alert is not present.

Then in theory virt-manager could be tested on various versions of
libvirt and we could verify both that it works correctly on newest
versions and also that it does not do strange things on older versions
of libvirt. Does that make sense?


I get your point but I do not think the maintenance burden is worth it. For the test suite, my general aim is:

- All tests should pass on latest libvirt
- The test suite shouldn't fail on older libvirt

The last bit is on a best effort basis, basically I or another dev fixes issues as they hit them, typically when trying to run the test suite on an older distro version.

If we aimed to keep the test suite fully 'passing' on multiple libvirt versions, we'd have a bunch of functional code like you introduced here that would slowly rot over time, for a minimal target audience. It's not worth the maintenance burden IMO

Thanks,
Cole

_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list




[Index of Archives]     [Linux Virtualization]     [KVM Development]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]     [Video 4 Linux]

  Powered by Linux