Libguestfs - Feb 2016 - Re: [PATCH] python: tests: use more targeted assert*() functions/checks

- use assertIsInstance, assertNotEqual, and assertIsNotNone as more
  specific checks (will produce better logging)
- use assertRaises when expecting exceptions being thrown
- when testing internal_test_rhashtable, instead of checking type and
  elements of the return values just check the return value as a whole
  (easier and already getting all the work needed by unittest)
---
 python/t/080-version.py        | 6 +++---
 python/t/420-log-messages.py   | 2 +-
 python/t/800-explicit-close.py | 6 +-----
 python/t/810-rhbz811650.py     | 5 +----
 python/t/820-rhbz912499.py     | 2 +-
 python/t/900-python-dict.py    | 9 +--------
 6 files changed, 8 insertions(+), 22 deletions(-)

diff --git a/python/t/080-version.py b/python/t/080-version.py
index e8e1e25..cda4872 100644
--- a/python/t/080-version.py
+++ b/python/t/080-version.py
@@ -34,13 +34,13 @@ class Test080Version (unittest.TestCase):
         self.assertEqual (self.version['major'], 1)
 
     def test_minor (self):
-        self.assertTrue (isinstance (self.version['minor'], cl))
+        self.assertIsInstance (self.version['minor'], cl)
 
     def test_release (self):
-        self.assertTrue (isinstance (self.version['release'], cl))
+        self.assertIsInstance (self.version['release'], cl)
 
     def test_extra (self):
-        self.assertTrue (isinstance (self.version['extra'], str))
+        self.assertIsInstance (self.version['extra'], str)
 
 if __name__ == '__main__':
     unittest.main ()
diff --git a/python/t/420-log-messages.py b/python/t/420-log-messages.py
index 002098e..6b7c06c 100644
--- a/python/t/420-log-messages.py
+++ b/python/t/420-log-messages.py
@@ -51,7 +51,7 @@ class Test420LogMessages (unittest.TestCase):
 
         g.close ()
 
-        self.assertFalse (log_invoked == 0)
+        self.assertNotEqual (log_invoked, 0)
 
 if __name__ == '__main__':
     unittest.main ()
diff --git a/python/t/800-explicit-close.py b/python/t/800-explicit-close.py
index 9b425af..4086829 100644
--- a/python/t/800-explicit-close.py
+++ b/python/t/800-explicit-close.py
@@ -37,11 +37,7 @@ class Test800ExplicitClose (unittest.TestCase):
         # Expect an exception if we call a method on a closed handle.
         g = guestfs.GuestFS (python_return_dict=True)
         g.close ()
-        try:
-            g.set_memsize (512)
-            raise Exception("expected an exception from previous
statement")
-        except guestfs.ClosedHandle:
-            pass
+        self.assertRaises (guestfs.ClosedHandle, g.set_memsize, 512)
         del g
 
         # Verify that the handle is really being closed by g.close, by
diff --git a/python/t/810-rhbz811650.py b/python/t/810-rhbz811650.py
index c812c30..c37abac 100644
--- a/python/t/810-rhbz811650.py
+++ b/python/t/810-rhbz811650.py
@@ -31,10 +31,7 @@ class Test810RHBZ811650 (unittest.TestCase):
         # Because error() wasn't being called, guestfs_last_error
         # would return NULL, causing a segfault in the Python bindings
         # (RHBZ#811650).
-        try:
-            g.launch ()
-        except:
-            pass
+        self.assertRaises (RuntimeError, g.launch)
 
     def tearDown (self):
         os.unlink ("rhbz811650.img")
diff --git a/python/t/820-rhbz912499.py b/python/t/820-rhbz912499.py
index 3cf9bba..e8f1d4b 100644
--- a/python/t/820-rhbz912499.py
+++ b/python/t/820-rhbz912499.py
@@ -89,7 +89,7 @@ class Test820RHBZ912499 (unittest.TestCase):
     def test_rhbz912499 (self):
         dom = conn.createXML (self.xml,
                               libvirt.VIR_DOMAIN_START_AUTODESTROY)
-        self.assertFalse (dom == None)
+        self.assertIsNotNone (dom)
 
         print ("temporary domain %s is running" % self.domname)
 
diff --git a/python/t/900-python-dict.py b/python/t/900-python-dict.py
index 7d78e86..5ea5a53 100644
--- a/python/t/900-python-dict.py
+++ b/python/t/900-python-dict.py
@@ -27,7 +27,6 @@ class Test900PythonDict (unittest.TestCase):
         g = guestfs.GuestFS (python_return_dict=False)
 
         r = g.internal_test_rhashtable ("5")
-        self.assertTrue (isinstance (r, list))
         self.assertEqual (r, [ ("0","0"),
("1","1"), ("2","2"),
                                ("3","3"),
("4","4") ])
 
@@ -35,13 +34,7 @@ class Test900PythonDict (unittest.TestCase):
         g = guestfs.GuestFS (python_return_dict=True)
 
         r = g.internal_test_rhashtable ("5")
-        self.assertTrue (isinstance (r, dict))
-        self.assertEqual (sorted (r.keys()),
["0","1","2","3","4"])
-        self.assertEqual (r["0"], "0")
-        self.assertEqual (r["1"], "1")
-        self.assertEqual (r["2"], "2")
-        self.assertEqual (r["3"], "3")
-        self.assertEqual (r["4"], "4")
+        self.assertEqual (r, {"0": "0", "1":
"1", "2": "2", "3": "3",
"4": "4"})
 
 if __name__ == '__main__':
     unittest.main ()
-- 
2.5.0

On Fri, Feb 12, 2016 at 06:04:47PM +0100, Pino Toscano
wrote:
> - use assertIsInstance, assertNotEqual, and assertIsNotNone as more
>   specific checks (will produce better logging)
> - use assertRaises when expecting exceptions being thrown
> - when testing internal_test_rhashtable, instead of checking type and
>   elements of the return values just check the return value as a whole
>   (easier and already getting all the work needed by unittest)
Good find!  I was going by the documentation which didn't mention any
of these asserts.

ACK, assuming it also works with python3, ie this passes:

  PYTHON=/usr/bin/python3 ./configure && make && make -C
test-data check && make -C python check

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

On Friday 12 February 2016 17:13:17 Richard W.M. Jones
wrote:
> On Fri, Feb 12, 2016 at 06:04:47PM +0100, Pino Toscano wrote:
> > - use assertIsInstance, assertNotEqual, and assertIsNotNone as more
> >   specific checks (will produce better logging)
> > - use assertRaises when expecting exceptions being thrown
> > - when testing internal_test_rhashtable, instead of checking type and
> >   elements of the return values just check the return value as a whole
> >   (easier and already getting all the work needed by unittest)
> 
> Good find!  I was going by the documentation which didn't mention any
> of these asserts.
Hm I see all of them in
  https://docs.python.org/2.7/library/unittest.html#test-cases
> ACK, assuming it also works with python3, ie this passes:
> 
>   PYTHON=/usr/bin/python3 ./configure && make && make -C
test-data check && make -C python check
Usually I try on a Python3-only distribution (== Archlinux).  Since
they passed there too, I pushed it.

Thanks,
-- 
Pino Toscano