Daniel, thank you very much for your help! <div><br></div><div>However, i'm running into my infamous  "import libvirt" problem again when running the tests (see output below), i really though i'd fixed that one for sure -.-'. </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br>======================================================================<br>
ERROR: test_diagnostic_full (nova.tests.test_libvirt.LibvirtConnTestCase)<br>----------------------------------------------------------------------<br>Traceback (most recent call last):<br>  File "/home/gsd/nova/nova/tests/test_libvirt.py", line 1923, in test_diagnostic_full<br>
    actual = conn.get_diagnostics({"name": "testvirt"})<br>  File "/home/gsd/nova/nova/virt/libvirt/driver.py", line 2678, in get_diagnostics<br>    return diagnostics.get_diagnostics(dom)<br>
  File "/home/gsd/nova/nova/virt/libvirt/diagnostics.py", line 97, in get_diagnostics<br>    import libvirt<br>ImportError: No module named libvirt</blockquote><div><div><br></div><div><br><br><div class="gmail_quote">
On Fri, Jul 6, 2012 at 1:11 PM, Daniel P. Berrange <span dir="ltr"><<a href="mailto:berrange@redhat.com" target="_blank">berrange@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">On Fri, Jul 06, 2012 at 11:59:51AM +0100, Leander Bessa Beernaert wrote:<br>
> Hello,<br>
><br>
> I've been working on implementing the "diagnostics" command for libvirt -<br>
> <a href="https://review.openstack.org/#/c/8839/" target="_blank">https://review.openstack.org/#/c/8839/</a> . Now i need to create the unit test<br>
> for this new operation. I've been looking at the code to try and figure out<br>
> an easy way to replicate this, but i'm a bit lost.<br>
><br>
> What i need to do is simulate a connection to libvirt, create a fake<br>
> instance with a predefined characteristics, retrieve the virtdomain and<br>
> then verify the results from the get_diagnostics command. I'm more at loss<br>
> as to how exactly do i setup a fake connection a create a fake instance? I<br>
> thought about creating a dummy implementation as i've seen being used<br>
> around the tests, however that doesn't give me any access to real method.<br>
> Do note that I'm relatively new to python world, so there's a lot of things<br>
> i can't grasp yes :s.<br>
<br>
</div></div>As you say, figuring out the libvirt Nova test framework is not entirely<br>
easy, particularly if you're new to Python / Nova. So to help you get<br>
your patch through review, here is one suitable test case you can add to<br>
test_libvirt.py for exercising the get_diagnostics API:<br>
<br>
diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py<br>
index eed43b8..f3fa5ff 100644<br>
--- a/nova/tests/test_libvirt.py<br>
+++ b/nova/tests/test_libvirt.py<br>
@@ -1938,6 +1938,91 @@ class LibvirtConnTestCase(test.TestCase):<br>
         got = jsonutils.loads(conn.get_cpu_info())<br>
         self.assertEqual(want, got)<br>
<br>
+    def test_diagnostic_full(self):<br>
+        xml = """<br>
+                <domain type='kvm'><br>
+                    <devices><br>
+                        <disk type='file'><br>
+                            <source file='filename'/><br>
+                            <target dev='vda' bus='virtio'/><br>
+                        </disk><br>
+                        <disk type='block'><br>
+                            <source dev='/path/to/dev/1'/><br>
+                            <target dev='vdb' bus='virtio'/><br>
+                        </disk><br>
+                        <interface type='network'><br>
+                            <mac address='52:54:00:a4:38:38'/><br>
+                            <source network='default'/><br>
+                            <target dev='vnet0'/><br>
+                        </interface><br>
+                    </devices><br>
+                </domain><br>
+            """<br>
+<br>
+        class DiagFakeDomain(FakeVirtDomain):<br>
+<br>
+            def __init__(self):<br>
+                super(DiagFakeDomain, self).__init__(fake_xml=xml)<br>
+<br>
+            def vcpus(self):<br>
+                return ([(0, 1, 15340000000L, 0),<br>
+                         (1, 1, 1640000000L, 0),<br>
+                         (2, 1, 3040000000L, 0),<br>
+                         (3, 1, 1420000000L, 0)],<br>
+                        [(True, False),<br>
+                         (True, False),<br>
+                         (True, False),<br>
+                         (True, False)])<br>
+<br>
+            def blockStats(self, path):<br>
+                return (169L, 688640L, 0L, 0L, -1L)<br>
+<br>
+            def interfaceStats(self, path):<br>
+                return (4408L, 82L, 0L, 0L, 0L, 0L, 0L, 0L)<br>
+<br>
+            def memoryStats(self):<br>
+                return {'actual': 220160L, 'rss': 200164L}<br>
+<br>
+            def maxMemory(self):<br>
+                return 280160L<br>
+<br>
+        def fake_lookup_name(name):<br>
+            return DiagFakeDomain()<br>
+<br>
+        self.mox.StubOutWithMock(libvirt_driver.LibvirtDriver, '_conn')<br>
+        libvirt_driver.LibvirtDriver._conn.lookupByName = fake_lookup_name<br>
+<br>
+        conn = libvirt_driver.LibvirtDriver(False)<br>
+        actual = conn.get_diagnostics( { "name": "testvirt" })<br>
+        expect = {'cpu0_time': 15340000000L,<br>
+                  'cpu1_time': 1640000000L,<br>
+                  'cpu2_time': 3040000000L,<br>
+                  'cpu3_time': 1420000000L,<br>
+                  'vda_read': 688640L,<br>
+                  'vda_read_req': 169L,<br>
+                  'vda_write': 0L,<br>
+                  'vda_write_req': 0L,<br>
+                  'vda_errors': -1L,<br>
+                  'vdb_read': 688640L,<br>
+                  'vdb_read_req': 169L,<br>
+                  'vdb_write': 0L,<br>
+                  'vdb_write_req': 0L,<br>
+                  'vdb_errors': -1L,<br>
+                  'memory': 280160L,<br>
+                  'memory-actual': 220160L,<br>
+                  'memory-rss': 200164L,<br>
+                  'vnet0_rx': 4408L,<br>
+                  'vnet0_rx_drop': 0L,<br>
+                  'vnet0_rx_errors': 0L,<br>
+                  'vnet0_rx_packets': 82L,<br>
+                  'vnet0_tx': 0L,<br>
+                  'vnet0_tx_drop': 0L,<br>
+                  'vnet0_tx_errors': 0L,<br>
+                  'vnet0_tx_packets': 0L,<br>
+                  }<br>
+<br>
+        self.assertEqual(actual, expect)<br>
+<br>
<br>
 class HostStateTestCase(test.TestCase):<br>
<br>
<br>
A description of what I'm doing here:<br>
<br>
 * First we mock up an XML document that describes our test<br>
   guest, with 2 disks and 1 NIC.<br>
<br>
 * We create the FakeVirtDomain() class which provides a stub<br>
   implementation of the various libvirt APIs that the<br>
   get_diagnostics method will call.<br>
<br>
 * The fake APIs will return a hardcoded set of statistics<br>
<br>
 * We use StubOutWithMock() to make sure that the libvirt connection<br>
   returns an instance of our FakeVirtDomain class<br>
<br>
 * Finally we can call the get_diagnostics() method, and compare<br>
   the data it returns against what we want.<br>
<br>
<br>
Now this test case only tests the "normal" operation of the API. As noted<br>
in the review comments, it is possible for the methods like 'vcpus',<br>
'intefaceStats', 'blockStats', etc to raise libvirt exceptions. So in<br>
addition to what I've shown here, you will also want to add some more<br>
test cases, which verify that exception handling works correctly.<br>
<br>
To do this, just copy my example test case above, and make some small<br>
changes.  eg change the impl of 'vcpus' to be this:<br>
<br>
   def vcps():<br>
      raise libvirtException("fake failure")<br>
<br>
and then remove the 'cpuN_time' elements from the 'expected' data hash<br>
<br>
and similarly for the interfaceStats/blockStats/memoryStats methods<br>
which can also raise errors.<br>
<br>
Regards,<br>
Daniel<br>
<span class="HOEnZb"><font color="#888888">--<br>
|: <a href="http://berrange.com" target="_blank">http://berrange.com</a>      -o-    <a href="http://www.flickr.com/photos/dberrange/" target="_blank">http://www.flickr.com/photos/dberrange/</a> :|<br>
|: <a href="http://libvirt.org" target="_blank">http://libvirt.org</a>              -o-             <a href="http://virt-manager.org" target="_blank">http://virt-manager.org</a> :|<br>
|: <a href="http://autobuild.org" target="_blank">http://autobuild.org</a>       -o-         <a href="http://search.cpan.org/~danberr/" target="_blank">http://search.cpan.org/~danberr/</a> :|<br>
|: <a href="http://entangle-photo.org" target="_blank">http://entangle-photo.org</a>       -o-       <a href="http://live.gnome.org/gtk-vnc" target="_blank">http://live.gnome.org/gtk-vnc</a> :|<br>
</font></span></blockquote></div><br></div></div>