Overall this looks good.

The known_bugs addition looks pretty nifty (but I worry a bit about the added complexity of it). The fake image service looks like a really nice addition.

Below are a few femto-nits:


658	+    # Not supported till Python 2.7 / 3.1
659	+    #@unittest.skipIf(
660	+    #    'server_create_metadata_throws_unhashable_type' in KNOWN_BUGS,
661	+    #    'KNOWN_BUG:server_create_metadata_throws_unhashable_type')

If we're going to go this route, perhaps we could roll our own decorator which the TestRunner can use to skip the relevant tests. This would make adoption of this under 2.6 just as easy as 2.7+.

Something along the lines of:

@tests_known_bug("my_known_bug")
def test_my_known_bug(self):
    self.assert_(False)


586	+#        print driver.LoggingVolumeDriver.all_logs()
587	+#
588	+#
589	+#        create_actions = driver.LoggingVolumeDriver.logs_like(
590	+#                            'create_volume',
591	+#                            id=created_volume_id)
592	+#        print create_actions
593	+#        self.assertEquals(1, len(create_actions))
594	+#        create_action = create_actions[0]
595	+#        self.assertEquals(create_action['id'], created_server_id)
596	+#        self.assertEquals(create_action['availability_zone'], 'nova')
597	+#        self.assertEquals(create_action['size'], 1)
598	+#
599	+#        export_actions = driver.LoggingVolumeDriver.logs_like(
600	+#                            'create_export',
601	+#                            id=created_server_id)
602	+#        self.assertEquals(1, len(export_actions))
603	+#        export_action = export_actions[0]
604	+#        self.assertEquals(export_action['id'], created_server_id)
605	+#        self.assertEquals(export_action['availability_zone'], 'nova')
606	+#
607	+#        delete_actions = driver.LoggingVolumeDriver.logs_like(
608	+#                            'delete_volume',
609	+#                            id=created_server_id)
610	+#        self.assertEquals(1, len(delete_actions))
611	+#        delete_action = export_actions[0]
612	+#        self.assertEquals(delete_action['id'], created_server_id)

Was this left in on purpose? If so, it probably should have a TODO with it.


> 509	+    def test_get_servers(self):
> 510	+        """Simple check that listing servers works"""
> 511	+        servers = self.api.get_servers()
> 512	+        for server in servers:
> 513	+            print("server: %s" % server)

Can we put an assert in here? Perhaps just a assertNotRaises or something.

> 334	+            raise exception.Error("No way to create an image through API??")

This line (and a few others) need i18n treatment, the _("blah").

> 628	+            #if found_server['status'] != 'deleting':
> 629	+            #    break

Left in by accident, or does that need a TODO/FIXME?


> 129	+            raise exception.NotFound

Might be helpful to pass in a string with the Exception which includes the Id of the image not found. Something like

  raise exception.NotFound(_("Image %(image_id)s not found) % locals())

> 489	+import time

Should be moved up to top section of imports (per HACKING)

> 641	+        #TODO(justinsb): This is FUBAR

Agree with the comment, but it could use a bit more detail. Perhaps a couple of words on the specific issue and maybe a hint of a fix.