Merge lp:~ken-pepple/nova/lp828596 into lp:~hudson-openstack/nova/trunk

[Ken Pepple]

added unit tests to instance_types for rainy day paths

[Jay Pipes (jaypipes)]

rock on. nice work, Ken! :)

review: Approve

[Brian Waldon (bcwaldon)]

Looks good, Ken.

review: Approve

[Alex Meade (alex-meade)]

review: Approve

[Ed Leafe (ed-leafe)]

Line 13: Are you trying to append the letter 'z' to the nonexistant_flavor value? If so, that is not the correct syntax. The statement:
    nonexistant_flavor.join("z")
will *always* return the letter "z" by itself.

Perhaps you meant:
    nonexistant_flavor = "%s%s" % (nonexistant_flavor, "z")

Oh, and it should be spelled 'nonexistent' :)

review: Needs Fixing

[Mandell (mdegerne)]

Agree with Ed, except that you could also do:
     nonexistant_flavor = nonexistant_flavor.join(["z","z"])

I really don't know which is more efficient in python.

review: Needs Fixing (community)

[Ken Pepple (ken-pepple)]

> Line 13: Are you trying to append the letter 'z' to the nonexistant_flavor
> value? If so, that is not the correct syntax. The statement:
> nonexistant_flavor.join("z")
> will *always* return the letter "z" by itself.

woof. i mixed ruby in python. fixed.

[Ken Pepple (ken-pepple)]

> Agree with Ed, except that you could also do:
> nonexistant_flavor = nonexistant_flavor.join(["z","z"])
>
> I really don't know which is more efficient in python.

i went with the x += "z" syntax ... not sure if this will ever get exercised as the instance_flavors are set at migration time.

[Ed Leafe (ed-leafe)]

Ken, looks good, and I feel *really* petty for bringing this up, but...

Can you fix the spelling of the method name "test_non_existant_inst_type_shouldnt_delete"? I promise I'll approve once that's changed. :)

[Ken Pepple (ken-pepple)]

> Ken, looks good, and I feel *really* petty for bringing this up, but...
>
> Can you fix the spelling of the method name
> "test_non_existant_inst_type_shouldnt_delete"? I promise I'll approve once
> that's changed. :)

fair enough. but for the record, I prefer the french spelling - http://www.online-dictionary.biz/french/english/meaning/existant

[Ed Leafe (ed-leafe)]

Kissing up to Thierry, eh? ;-)

And apologies for the nit-pickiness. I blame the nuns who beat that stuff into me.

review: Approve

[Ed Leafe (ed-leafe)]

Since Mandell's objections were the same as mine, and they have been properly addressed, I'm going to override his 'Needs Fixing' and mark this as approved.

=== modified file 'nova/tests/test_instance_types.py'
--- nova/tests/test_instance_types.py	2011-04-27 21:03:05 +0000
+++ nova/tests/test_instance_types.py	2011-08-24 23:05:28 +0000
@@ -47,6 +47,29 @@
         self.id = max_id["id"] + 1
         self.name = str(int(time.time()))
 
+    def _nonexistent_flavor_name(self):
+        """return an instance type name not in the DB"""
+        nonexistent_flavor = "sdfsfsdf"
+        flavors = instance_types.get_all_types()
+        while nonexistent_flavor in flavors:
+            nonexistent_flavor += "z"
+        else:
+            return nonexistent_flavor
+
+    def _nonexistent_flavor_id(self):
+        """return an instance type ID not in the DB"""
+        nonexistent_flavor = 2700
+        flavor_ids = [value["id"] for key, value in\
+                    instance_types.get_all_types().iteritems()]
+        while nonexistent_flavor in flavor_ids:
+            nonexistent_flavor += 1
+        else:
+            return nonexistent_flavor
+
+    def _existing_flavor(self):
+        """return first instance type name"""
+        return instance_types.get_all_types().keys()[0]
+
     def test_instance_type_create_then_delete(self):
         """Ensure instance types can be created"""
         starting_inst_list = instance_types.get_all_types()
@@ -84,10 +107,11 @@
                 exception.InvalidInput,
                 instance_types.create, self.name, 256, 1, "aa", self.flavorid)
 
-    def test_non_existant_inst_type_shouldnt_delete(self):
+    def test_non_existent_inst_type_shouldnt_delete(self):
         """Ensures that instance type creation fails with invalid args"""
         self.assertRaises(exception.ApiError,
-                          instance_types.destroy, "sfsfsdfdfs")
+                          instance_types.destroy,
+                          self._nonexistent_flavor_name())
 
     def test_repeated_inst_types_should_raise_api_error(self):
         """Ensures that instance duplicates raises ApiError"""
@@ -97,3 +121,43 @@
         self.assertRaises(
                 exception.ApiError,
                 instance_types.create, new_name, 256, 1, 120, self.flavorid)
+
+    def test_will_not_destroy_with_no_name(self):
+        """Ensure destroy sad path of no name raises error"""
+        self.assertRaises(exception.ApiError,
+                          instance_types.destroy,
+                          self._nonexistent_flavor_name())
+
+    def test_will_not_purge_without_name(self):
+        """Ensure purge without a name raises error"""
+        self.assertRaises(exception.InvalidInstanceType,
+                          instance_types.purge, None)
+
+    def test_will_not_purge_with_wrong_name(self):
+        """Ensure purge without correct name raises error"""
+        self.assertRaises(exception.ApiError,
+                          instance_types.purge,
+                          self._nonexistent_flavor_name())
+
+    def test_will_not_get_bad_default_instance_type(self):
+        """ensures error raised on bad default instance type"""
+        FLAGS.default_instance_type = self._nonexistent_flavor_name()
+        self.assertRaises(exception.InstanceTypeNotFoundByName,
+                          instance_types.get_default_instance_type)
+
+    def test_will_not_get_instance_type_by_name_with_no_name(self):
+        """Ensure get by name returns default flavor with no name"""
+        self.assertEqual(instance_types.get_default_instance_type(),
+                              instance_types.get_instance_type_by_name(None))
+
+    def test_will_not_get_instance_type_with_bad_name(self):
+        """Ensure get by name returns default flavor with bad name"""
+        self.assertRaises(exception.InstanceTypeNotFound,
+                          instance_types.get_instance_type,
+                          self._nonexistent_flavor_name())
+
+    def test_will_not_get_flavor_by_bad_flavor_id(self):
+        """Ensure get by flavor raises error with wrong flavorid"""
+        self.assertRaises(exception.InstanceTypeNotFound,
+                          instance_types.get_instance_type_by_name,
+                          self._nonexistent_flavor_id())