Merge lp:~fwereade/pyjuju/fix-mocking into lp:pyjuju

[William Reade]

Tests now run with no network.

Not sure how to reliably induce a flaky network to see whether it affects possibly-related timeouts.

Coverage of the code we were failing to mock correctly is now improved.

[Kapil Thangavelu (hazmat)]

looks good, and works nicely disconnected.

review: Approve

[Gustavo Niemeyer (niemeyer)]

Looks good. Just a minor suggestion:

[1]

24 + if not get_ami_kwargs:
25 + return
26 + get_ami = self.mocker.replace(
27 + "ensemble.providers.ec2.utils.get_current_ami")
28 + get_ami(**get_ami_kwargs)
29 + self.mocker.result(succeed("ami-714ba518"))

I suggest using something like this (untested):

get_ami(KWARGS)
def check_kwargs(**kwargs):
    self.assertEquals(kwargs, get_ami_kwargs)
    return succeed("ami-...")
self.mocker.call(check_kwargs)

The distinction between the two approaches is that with the original version it will
ignore all requests that do not match the provided kwargs, and pass them through.
This version will capture all calls, and ensure they match the expectation.

review: Approve

[lp:~fwereade/pyjuju/fix-mocking]

282. By William Reade on 2011-07-27

merge from trunk

283. By William Reade on 2011-07-27

improved test per niemeyer's suggestion

=== modified file 'ensemble/providers/ec2/tests/common.py'
--- ensemble/providers/ec2/tests/common.py	2011-07-14 05:40:06 +0000
+++ ensemble/providers/ec2/tests/common.py	2011-07-27 09:37:19 +0000
@@ -53,7 +53,7 @@
     default_zookeeper_host = Instance(
         "i-es-zoo", "running", private_dns_name="es.example.internal")
 
-    def _mock_launch_utils(self):
+    def _mock_launch_utils(self, **get_ami_kwargs):
         get_public_key = self.mocker.replace(
             "ensemble.providers.common.utils.get_user_authorized_keys")
 
@@ -62,12 +62,17 @@
 
         get_public_key(MATCH(match_config))
         self.mocker.result("zebra")
-        # AMI is not retrieve dynamically at the moment, default values are
-        # utilized.
-        #get_ami = self.mocker.replace(
-        #    "ensemble.providers.ec2.utils.get_current_ami")
-        #get_ami()
-        #self.mocker.result(succeed("ami-714ba518"))
+
+        if not get_ami_kwargs:
+            return
+        get_ami = self.mocker.replace(
+            "ensemble.providers.ec2.utils.get_current_ami")
+        get_ami(KWARGS)
+
+        def check_kwargs(**kwargs):
+            self.assertEquals(kwargs, get_ami_kwargs)
+            return succeed("ami-714ba518")
+        self.mocker.call(check_kwargs)
 
     def _mock_create_group(self):
         group_name = "ensemble-%s" % self.env_name

=== modified file 'ensemble/providers/ec2/tests/test_bootstrap.py'
--- ensemble/providers/ec2/tests/test_bootstrap.py	2011-07-18 07:07:03 +0000
+++ ensemble/providers/ec2/tests/test_bootstrap.py	2011-07-27 09:37:19 +0000
@@ -92,7 +92,7 @@
         self.ec2.describe_security_groups()
         self.mocker.result(succeed([]))
         self._mock_create_group()
-        self._mock_launch_utils()
+        self._mock_launch_utils(region="us-east-1")
         self._mock_launch()
         self.mocker.result(succeed([]))
         self._mock_save()
@@ -120,7 +120,7 @@
         self.ec2.describe_security_groups()
         self.mocker.result(succeed([
             SecurityGroup("ensemble-%s" % self.env_name, "")]))
-        self._mock_launch_utils()
+        self._mock_launch_utils(region="us-east-1")
         self._mock_launch()
         self.mocker.result(succeed([]))
         self._mock_save()
@@ -196,7 +196,7 @@
         self.ec2.describe_security_groups()
         self.mocker.result(succeed([
             SecurityGroup("ensemble-%s" % self.env_name, "")]))
-        self._mock_launch_utils()
+        self._mock_launch_utils(region="us-east-1")
         self._mock_launch()
         self.mocker.result(succeed(instances))
         self._mock_save()
@@ -223,7 +223,7 @@
         self.ec2.describe_security_groups()
         self.mocker.result(succeed([
             SecurityGroup("ensemble-%s" % self.env_name, "")]))
-        self._mock_launch_utils()
+        self._mock_launch_utils(region="us-east-1")
         self._mock_launch()
         self.mocker.result(succeed(instances))
         self._mock_save()

=== modified file 'ensemble/providers/ec2/tests/test_launch.py'
--- ensemble/providers/ec2/tests/test_launch.py	2011-07-18 07:07:03 +0000
+++ ensemble/providers/ec2/tests/test_launch.py	2011-07-27 09:37:19 +0000
@@ -1,4 +1,4 @@
-
+import random
 from yaml import load
 
 from twisted.internet.defer import succeed
@@ -56,7 +56,7 @@
         self.ec2.describe_security_groups()
         self.mocker.result(succeed([]))
         self._mock_create_group()
-        self._mock_launch_utils()
+        self._mock_launch_utils(region="us-east-1")
         self._mock_get_zookeeper_hosts()
         self._mock_launch(instance)
         self.mocker.replay()
@@ -77,7 +77,7 @@
         self.ec2.describe_security_groups()
         self.mocker.result(succeed([]))
         self._mock_create_group()
-        self._mock_launch_utils()
+        self._mock_launch_utils(region="us-east-1")
         self._mock_get_zookeeper_hosts()
 
         def verify_provider_type(data):
@@ -129,7 +129,7 @@
         self.ec2.describe_security_groups()
         self.mocker.result(succeed([]))
         self._mock_create_group()
-        self._mock_launch_utils()
+        self._mock_launch_utils(region="us-east-1")
         self._mock_get_zookeeper_hosts()
 
         def verify_machine_agent(data):
@@ -146,3 +146,47 @@
         provider = self.get_provider()
         d = provider.start_machine({"machine-id": "machine-1"})
         return d
+
+    def _mock_launch_with_ami_params(self, get_ami_kwargs):
+        instance = Instance("i-foobar", "running", dns_name="x1.example.com")
+        self.ec2.describe_security_groups()
+        self.mocker.result(succeed([]))
+        self._mock_create_group()
+        self._mock_launch_utils(**get_ami_kwargs)
+        self._mock_get_zookeeper_hosts()
+        self._mock_launch(instance)
+
+    def test_launch_options_known_ami(self):
+        self._mock_launch_with_ami_params({})
+        self.mocker.replay()
+
+        provider = self.get_provider()
+        # TODO shouldn't something be testing we actually launch
+        # an instance of the expected AMI?
+        provider.config["default-image-id"] = "ami-XXXXXX"
+        return provider.start_machine({"machine-id": "machine-1"})
+
+    def test_launch_options_region(self):
+        self._mock_launch_with_ami_params({"region":"somewhere-else-1"})
+        self.mocker.replay()
+
+        provider = self.get_provider()
+        provider.config["region"] = "somewhere-else-1"
+        return provider.start_machine({"machine-id": "machine-1"})
+
+    def test_launch_options_other(self):
+        self._mock_launch_with_ami_params({
+            "region": "us-east-1",
+            "ubuntu_release_name": "choleric",
+            "arch": "quantum86",
+            "ebs": "maybe",
+            "daily": "perhaps"})
+        self.mocker.replay()
+
+        provider = self.get_provider()
+        return provider.start_machine({
+            "machine-id": "machine-1",
+            "image_release_name": "choleric",
+            "image_arch": "quantum86",
+            "image_ebs": "maybe",
+            "image_daily": "perhaps"})

=== modified file 'ensemble/providers/ec2/utils.py'
--- ensemble/providers/ec2/utils.py	2011-07-13 03:39:28 +0000
+++ ensemble/providers/ec2/utils.py	2011-07-27 09:37:19 +0000
@@ -32,7 +32,6 @@
                     region="us-east-1", daily=False, desktop=False,
                     url_fetch=None):
     """Get the latest ami for the last release of ubuntu."""
-
     data = {}
     data["ubuntu_release_name"] = ubuntu_release_name
     data["version"] = daily and "daily" or "released"