[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
On Fri, Feb 12, 2016 at 06:29:48PM +0100, Pino Toscano wrote:
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
That's because I didn't read that far down the page :-(
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.
Great, thanks. 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/
participants (2)
-
Pino Toscano -
Richard W.M. Jones