Merge lp:~fwereade/pyjuju/cobbler-zk-connect into lp:pyjuju

[William Reade]

This branch moves providers.ec2.connect over to providers.common.connect, makes a couple of small changes, and adds a bunch of tests. It should finally close the referenced bug (much of it was actually fixed in lp:~fwereade/ensemble/hide-instances, but it wasn't hooked up to orchestra until this branch).

[Gustavo Niemeyer (niemeyer)]

Looks good. +1 with these sorted:

[1]

+ raise EnvironmentPending("no machines have addresses assigned yet")

Was already like that, but can you please fix the message with s/no/No/?

[2]

+ def _cannot_connect(self, failure, machine):
+ failure.trap(NoConnection, ConnectionTimeoutException)
+ if not machine.launch_time:
+ raise EnvironmentPending(
+ "machine %s may not be ready (%s)"
+ % (machine.instance_id, str(failure.value)))
+
         five_minutes_ago = datetime.datetime.now() - timedelta(minutes=5)
         if machine.launch_time > five_minutes_ago:
- raise NoConnection("Can't yet connect to started machine: %s" %
- str(failure.value))
+ raise EnvironmentPending(
+ "machine %s is probably not ready (%s)"
+ % (machine.instance_id, str(failure.value)))
         return failure

Feels like a lot of subtlety and complexity for something simple.

Please kill both launch_time checks, and use this error message:

    "Can't connect to machine %s (perhaps still initializing): %s"

As a minor hint for future changes, please note the message
style (capital case, colon before underlying error message).

review: Approve

[lp:~fwereade/pyjuju/cobbler-zk-connect]

328. By William Reade on 2011-08-17

unused import

329. By William Reade on 2011-08-17

merge parent

330. By William Reade on 2011-08-17

ProviderMachine no longer has a .launch_time

331. By William Reade on 2011-08-18

merge trunk

[Jim Baker (jimbaker)]

+1, looks great once these minor problems are fixed in addition to what Gustavo mentioned.

[1]

+ def connect(self, share=False):
+ """Connect to the zookeeper ensemble running in the machine provider.
+
+ @param share: Requests sharing of the connection with other clients
+ attempting to connect to the same provider, if that's feasible.
+
+ returns an open C{txzookeeper.client.ZookeeperClient} and a
+ C{ensemble.storage.connection.TunnelProtocol}
+ """
+ return ZookeeperConnect(self).run(share=share)
+

Please write new docstrings using RST format.

[2]

     def run(self, share=False):
- client_deferred = self._provider.get_zookeeper_machines()
- client_deferred.addCallback(self._connect_to_machine, share)
- client_deferred.addCallback(self._wait_for_initialization)
- return client_deferred
+ d = self._provider.get_zookeeper_machines()
+ d.addCallback(self._pick_machine)
+ d.addCallback(self._connect_to_machine, share)
+ d.addCallback(self._wait_for_initialization)
+ return d

It's not difficult to follow the chaining here, but I think it's much easier when written using inlineCallbacks. Also, no docstring here.

[3]

+ def get_zookeeper_machines(self):
+ return succeed([
+ ProviderMachine("i-havenodns"),
+ self._second_zookeeper])

Docstring

[4]

+ # Give it some time to do it incorrectly.
+ yield self.sleep(0.1)

Is this sleep necessary, vs roundtripping with a poke?

[5]

 def machine_from_instance(instance):
- launch_time = None
- if instance.launch_time:
- launch_time = datetime.datetime.strptime(
- instance.launch_time[:19], "%Y-%m-%dT%H:%M:%S")
     return EC2ProviderMachine(
         instance.instance_id,
         instance.dns_name,
- instance.private_dns_name,
- launch_time)
+ instance.private_dns_name)

Docstring

[6]

- # XXX Replace the logic above with self.sleep(0.1) when it's merged.
+ yield self.sleep(0.1)

Again, another necessary sleep?

[7]

There are a number of new tests. None of them have docstrings.

review: Approve

[William Reade (fwereade)]

> [4]
>
> + # Give it some time to do it incorrectly.
> + yield self.sleep(0.1)
>
> Is this sleep necessary, vs roundtripping with a poke?

> [6]
>
> - # XXX Replace the logic above with self.sleep(0.1) when it's merged.
> + yield self.sleep(0.1)
>
> Again, another necessary sleep?

I don't understand your proposed solution: if we yield d, we'll block forever, but if we don't yield d and don't sleep we won't be able to confirm that d actually does wait for /initialized's existence. Am I missing something?

[lp:~fwereade/pyjuju/cobbler-zk-connect]

332. By William Reade on 2011-08-19

fixes per jimbaker's review; mostly dosctrings

333. By William Reade on 2011-08-19

fix thinko

[William Reade (fwereade)]

Just waiting on clarification for jimbaker's [4], [6], and on https://code.launchpad.net/~fwereade/ensemble/cobbler-zk-connect-error-messages/+merge/72033

[William Reade (fwereade)]

> Just waiting on clarification for jimbaker's [4], [6]

Explained on IRC: .poke_zk() is deterministic, and faster, and does exactly what we need.

[lp:~fwereade/pyjuju/cobbler-zk-connect]

334. By William Reade on 2011-08-19

remaining fixes per review

335. By William Reade on 2011-08-22

merge trunk

336. By William Reade on 2011-08-23

change dosctrings per review

337. By William Reade on 2011-08-23

merge trunk

338. By William Reade on 2011-08-25

merge trunk

339. By William Reade on 2011-08-26

merge lp:~fwereade/ensemble/cobbler-zk-connect-error-messages (error messages are more rigorously tested, and in some cases better) [f=831883][r=niemeyer,jimbaker]

=== modified file 'ensemble/agents/provision.py'
--- ensemble/agents/provision.py	2011-08-12 19:28:37 +0000
+++ ensemble/agents/provision.py	2011-08-26 13:16:39 +0000
@@ -4,7 +4,7 @@
 from zookeeper import NoNodeException
 
 from ensemble.environment.config import EnvironmentsConfig
-from ensemble.errors import MachinesNotFound, ProviderInteractionError
+from ensemble.errors import MachinesNotFound, ProviderError
 from ensemble.lib.twistutils import concurrent_execution_guard
 from ensemble.state.errors import (
     MachineStateNotFound, ServiceStateNotFound, ServiceUnitStateNotFound,
@@ -178,7 +178,7 @@
         # map of instance_id -> machine
         try:
             provider_machines = yield self.provider.get_machines()
-        except ProviderInteractionError:
+        except ProviderError:
             log.exception("Cannot get machine list")
             return
 
@@ -192,7 +192,7 @@
                     machine_state_id, provider_machines)
             except (StateChanged,
                     MachineStateNotFound,
-                    ProviderInteractionError):
+                    ProviderError):
                 log.exception("Cannot process machine %s", machine_state_id)
                 continue
             instance_ids.append(instance_id)
@@ -204,7 +204,7 @@
             machine = provider_machines[instance_id]
             try:
                 yield self.provider.shutdown_machine(machine)
-            except ProviderInteractionError:
+            except ProviderError:
                 log.exception("Cannot shutdown machine %s", instance_id)
                 continue
 

=== modified file 'ensemble/agents/tests/test_provision.py'
--- ensemble/agents/tests/test_provision.py	2011-08-11 05:59:30 +0000
+++ ensemble/agents/tests/test_provision.py	2011-08-26 13:16:39 +0000
@@ -313,7 +313,7 @@
 
         mock_provider = self.mocker.patch(self.agent.provider)
         mock_provider.start_machine({"machine-id": 0})
-        self.mocker.result(fail(ProviderInteractionError(OSError("Bad"))))
+        self.mocker.result(fail(ProviderInteractionError()))
 
         mock_provider.start_machine({"machine-id": 1})
         self.mocker.passthrough()
@@ -339,7 +339,7 @@
         mock_provider = self.mocker.patch(self.agent.provider)
 
         mock_provider.shutdown_machine(MATCH_MACHINE)
-        self.mocker.result(fail(ProviderInteractionError("Bad")))
+        self.mocker.result(fail(ProviderInteractionError()))
 
         mock_provider.shutdown_machine(MATCH_MACHINE)
         self.mocker.passthrough()
@@ -368,7 +368,7 @@
 
         mock_provider = self.mocker.patch(self.agent.provider)
         mock_provider.get_machines()
-        self.mocker.result(fail(ProviderInteractionError("Bad")))
+        self.mocker.result(fail(ProviderInteractionError()))
 
         mock_provider.get_machines()
         self.mocker.passthrough()

=== modified file 'ensemble/agents/tests/test_unit.py'
--- ensemble/agents/tests/test_unit.py	2011-07-29 14:42:04 +0000
+++ ensemble/agents/tests/test_unit.py	2011-08-26 13:16:39 +0000
@@ -118,10 +118,10 @@
         e = self.assertRaises(EnsembleError,
                               self.agent.configure,
                               options)
-        self.assertIn(
-            ("--unit-name must be provided in the command line,"
-            " or $ENSEMBLE_UNIT_NAME in the environment"),
-            str(e))
+        self.assertEquals(
+            str(e),
+            "--unit-name must be provided in the command line, or "
+            "$ENSEMBLE_UNIT_NAME in the environment")
 
     def test_agent_unit_name_cli_extraction(self):
         """The unit agent can parse its unit-name from the cli.

=== modified file 'ensemble/control/status.py'
--- ensemble/control/status.py	2011-07-20 23:35:48 +0000
+++ ensemble/control/status.py	2011-08-26 13:16:39 +0000
@@ -7,7 +7,7 @@
 from twisted.internet.defer import inlineCallbacks, returnValue
 import yaml
 
-from ensemble.errors import  ProviderInteractionError
+from ensemble.errors import  ProviderError
 from ensemble.environment.errors import EnvironmentsConfigError
 from ensemble.state.errors import UnitRelationStateNotFound
 from ensemble.state.formula import FormulaStateManager
@@ -264,10 +264,10 @@
             try:
                 pm = yield machine_provider.get_machine(instance_id)
                 m["dns-name"] = pm.dns_name
-            except ProviderInteractionError:
+            except ProviderError:
                 # The provider doesn't have machine information
                 log.error("Machine provider information missing: machine %s" % (
-                        machine_state.id))
+                          machine_state.id))
 
         machine_data[machine_state.id] = m
 

=== modified file 'ensemble/control/tests/test_status.py'
--- ensemble/control/tests/test_status.py	2011-08-08 21:51:54 +0000
+++ ensemble/control/tests/test_status.py	2011-08-26 13:16:39 +0000
@@ -458,7 +458,6 @@
         self.assertEqual(state["machines"][7]["instance-id"],
                          "pending")
 
-
     @inlineCallbacks
     def test_render_yaml(self):
         yield self.build_topology()
@@ -610,7 +609,6 @@
 
         self.assertIn("namespace:dummy-1", result)
 
-
     def test_render_dot_bad_clustering(self):
         """Test around Bug #792448.
 

=== modified file 'ensemble/errors.py'
--- ensemble/errors.py	2011-08-17 17:26:19 +0000
+++ ensemble/errors.py	2011-08-26 13:16:39 +0000
@@ -48,7 +48,7 @@
         self.path = path
 
     def __str__(self):
-        return "File was not found: %s" % (self.path,)
+        return "File was not found: %r" % (self.path,)
 
 
 class FormulaError(EnsembleError):
@@ -59,7 +59,7 @@
         self.message = message
 
     def __str__(self):
-        return "Error processing '%s': %s." % (self.path, self.message)
+        return "Error processing %r: %s" % (self.path, self.message)
 
 
 class FormulaInvocationError(EnsembleError):
@@ -70,8 +70,8 @@
         self.exit_code = exit_code
 
     def __str__(self):
-        return "Error processing '%s': exit code %s." % (self.path,
-                                                         self.exit_code)
+        return "Error processing %r: exit code %s." % (
+            self.path, self.exit_code)
 
 
 class FileAlreadyExists(EnsembleError):
@@ -84,7 +84,7 @@
         self.path = path
 
     def __str__(self):
-        return "File already exists, won't overwrite: %s" % (self.path,)
+        return "File already exists, won't overwrite: %r" % (self.path,)
 
 
 class InvalidEnsembleHeaderValue(EnsembleError):
@@ -101,7 +101,7 @@
 
     def __str__(self):
         return ("Expected a YAML file with an 'ensemble: %s' "
-                "header, found 'ensemble: %s': %s"
+                "header, found 'ensemble: %s': %r"
                 % (self.expected, self.found, self.path))
 
 
@@ -130,12 +130,6 @@
 class EnvironmentPending(NoConnection):
     """Raised when the ensemble environment is not accessible."""
 
-    def __init__(self, info="no details available"):
-        self._info = info
-
-    def __str__(self):
-        return "Ensemble environment is not accessible: %s" % self._info
-
 
 class ProviderError(EnsembleError):
     """Raised when an exception occurs in a provider."""
@@ -154,13 +148,7 @@
 
 
 class ProviderInteractionError(ProviderError):
-
-    def __init__(self, error):
-        self.error = error
-
-    def __str__(self):
-        return "ProviderError: Interaction with machine provider failed: %r" \
-               % self.error
+    """Raised when an unexpected error occurs interacting with a provider"""
 
 
 class CannotTerminateMachine(EnsembleError):

=== modified file 'ensemble/formula/bundle.py'
--- ensemble/formula/bundle.py	2011-06-05 20:19:18 +0000
+++ ensemble/formula/bundle.py	2011-08-26 13:16:39 +0000
@@ -18,13 +18,13 @@
         try:
             zf = ZipFile(path, 'r')
         except BadZipfile, exc:
-            raise FormulaError(path, "Must be a ZIP-file (%s)." % str(exc))
+            raise FormulaError(path, "must be a zip file (%s)" % exc)
 
         metadata = MetaData()
         if "metadata.yaml" not in zf.namelist():
             raise FormulaError(path,
-                               "Archive does not contain required file: "
-                               "\"metadata.yaml\".")
+                               "archive does not contain required file "
+                               "\"metadata.yaml\"")
 
         content = zf.read("metadata.yaml")
         metadata.parse(content)

=== modified file 'ensemble/formula/provider.py'
--- ensemble/formula/provider.py	2010-11-02 16:15:23 +0000
+++ ensemble/formula/provider.py	2011-08-26 13:16:39 +0000
@@ -25,5 +25,5 @@
         from .directory import FormulaDirectory
         return FormulaDirectory(specification)
 
-    raise FormulaError(specification, "Unable to process %s into a formula" % (
+    raise FormulaError(specification, "unable to process %s into a formula" % (
             specification))

=== modified file 'ensemble/formula/tests/test_bundle.py'
--- ensemble/formula/tests/test_bundle.py	2011-05-06 04:45:17 +0000
+++ ensemble/formula/tests/test_bundle.py	2011-08-26 13:16:39 +0000
@@ -38,8 +38,10 @@
         try:
             FormulaBundle(filename)
         except FormulaError, exc:
-            self.assertIn(filename, str(exc))
-            self.assertIn("zip", str(exc).lower())
+            self.assertEquals(
+                str(exc),
+                ("Error processing %r: " % filename) +
+                "must be a zip file (File is not a zip file)")
             return
 
         self.fail("Expected formula error.")
@@ -53,8 +55,10 @@
         try:
             FormulaBundle(filename)
         except FormulaError, exc:
-            self.assertIn(filename, str(exc))
-            self.assertIn("metadata.yaml", str(exc).lower())
+            self.assertEquals(
+                str(exc),
+                ("Error processing %r: " % filename) +
+                "archive does not contain required file \"metadata.yaml\"")
             return
 
         self.fail("Expected formula error.")

=== modified file 'ensemble/formula/tests/test_config.py'
--- ensemble/formula/tests/test_config.py	2011-08-24 00:18:27 +0000
+++ ensemble/formula/tests/test_config.py	2011-08-26 13:16:39 +0000
@@ -102,7 +102,7 @@
 
         error = self.assertRaises(ServiceConfigError,
                                   self.config.validate, dict(username="1234"))
-        self.assertIn(str(error), "Invalid value for username: 1234")
+        self.assertEquals(str(error), "Invalid value for username: 1234")
 
         data = self.config.validate(dict(username="12345678"))
         self.assertEqual(data, {"username": "12345678", "title": "My Title"})
@@ -114,15 +114,17 @@
         error = self.assertRaises(ServiceConfigError,
                           self.config.parse,
                           local_configuration)
-        self.assertIn("options.fails.validator: required value not found",
-                      str(error))
+        self.assertEquals(
+            str(error),
+            "Invalid options specification: options.fails.validator: "
+            "required value not found")
 
     def test_validate_types(self):
         self.config.parse(sample_configuration)
 
         error = self.assertRaises(ServiceConfigError,
                                   self.config.validate, {"skill-level": "NaN"})
-        self.assertIn(str(error), "Invalid value for skill-level: NaN")
+        self.assertEquals(str(error), "Invalid value for skill-level: NaN")
 
         data = self.config.validate({"skill-level": "9001"})
         # its over 9000!

=== modified file 'ensemble/formula/tests/test_repository.py'
--- ensemble/formula/tests/test_repository.py	2011-07-15 12:41:01 +0000
+++ ensemble/formula/tests/test_repository.py	2011-08-26 13:16:39 +0000
@@ -85,11 +85,11 @@
         self.assertTrue(formula.metadata)
 
     def test_find_formula_by_name_fails(self):
-        exc = self.assertRaises(
+        error = self.assertRaises(
             FormulaNotFound, self.repository1.find, "missing")
-        self.assertIn("missing", str(exc))
-        self.assertIn(self.unbundled_repo_path, str(exc),
-                      "Repository path not found in the error message.")
+        self.assertEquals(str(error),
+                          "Formula 'missing' not found in repository %s"
+                          % self.unbundled_repo_path)
 
     def test_find_formula_with_multiple_versions_returns_latest(self):
         # Copy the repository out of the codebase, so that we can hack it.

=== modified file 'ensemble/hooks/tests/test_arguments.py'
--- ensemble/hooks/tests/test_arguments.py	2011-06-11 02:37:35 +0000
+++ ensemble/hooks/tests/test_arguments.py	2011-08-26 13:16:39 +0000
@@ -52,8 +52,8 @@
         err = self.capture_stream("stderr")
         os.environ.pop("ENSEMBLE_AGENT_SOCKET", None)
         error = self.failUnlessRaises(SystemExit, cli.parse_args)
-        self.assertEquals("No ENSEMBLE_AGENT_SOCKET/-s option found",
-                          str(error))
+        self.assertEquals(str(error),
+                          "No ENSEMBLE_AGENT_SOCKET/-s option found")
         self.assertIn("No ENSEMBLE_AGENT_SOCKET/-s option found",
                       err.getvalue())
 

=== modified file 'ensemble/hooks/tests/test_cli.py'
--- ensemble/hooks/tests/test_cli.py	2011-06-29 09:02:33 +0000
+++ ensemble/hooks/tests/test_cli.py	2011-08-26 13:16:39 +0000
@@ -335,12 +335,15 @@
         # and check an error condition
         options = ["content=@missing"]
         error = self.assertRaises(EnsembleError, parse_keyvalue_pairs, options)
-        self.assertIn("No such file", str(error))
+        self.assertEquals(
+            str(error),
+            "No such file or directory: missing (argument:content)")
 
         # and check when fed non-kvpairs the error makes sense
         options = ["foobar"]
         error = self.assertRaises(EnsembleError, parse_keyvalue_pairs, options)
-        self.assertIn("Expected `option=value`. Found `foobar`", str(error))
+        self.assertEquals(
+            str(error), "Expected `option=value`. Found `foobar`")
 
     def test_parse_port_protocol(self):
         self.assertEqual(parse_port_protocol("80"), (80, "tcp"))

=== modified file 'ensemble/hooks/tests/test_executor.py'
--- ensemble/hooks/tests/test_executor.py	2011-07-06 18:00:07 +0000
+++ ensemble/hooks/tests/test_executor.py	2011-08-26 13:16:39 +0000
@@ -326,7 +326,7 @@
         error = yield self.assertFailure(
             self._executor.run_priority_hook(invoke, "foobar"),
             AssertionError)
-        self.assertIn("Executor must not be running", str(error))
+        self.assertEquals(str(error), "Executor must not be running")
 
     @inlineCallbacks
     def test_prioritize_with_queued(self):

=== modified file 'ensemble/hooks/tests/test_invoker.py'
--- ensemble/hooks/tests/test_invoker.py	2011-08-16 17:26:09 +0000
+++ ensemble/hooks/tests/test_invoker.py	2011-08-26 13:16:39 +0000
@@ -480,7 +480,7 @@
         error = self.failUnlessRaises(errors.FormulaError, exe, hook)
 
         self.assertEqual(error.path, hook)
-        self.assertIn(error.message, "hook is not executable")
+        self.assertEqual(error.message, "hook is not executable")
 
     @defer.inlineCallbacks
     def test_spawn_success(self):

=== modified file 'ensemble/lib/tests/test_schema.py'
--- ensemble/lib/tests/test_schema.py	2011-07-27 08:54:46 +0000
+++ ensemble/lib/tests/test_schema.py	2011-08-26 13:16:39 +0000
@@ -183,7 +183,7 @@
         exp = "([a-"
         error = self.assertRaises(
             SchemaError, Regex().coerce,  exp, PATH)
-        self.assertIn("expected regex", str(error))
+        self.assertEquals(str(error), "<path>: expected regex, got '([a-'")
 
 
 class ListTest(TestCase):

=== modified file 'ensemble/machine/__init__.py'
--- ensemble/machine/__init__.py	2011-08-08 17:37:46 +0000
+++ ensemble/machine/__init__.py	2011-08-26 13:16:39 +0000
@@ -10,10 +10,8 @@
     :class:`MachineProvider` api.
     """
 
-    def __init__(self, instance_id, dns_name=None,
-                 private_dns_name=None, launch_time=None):
+    def __init__(self, instance_id, dns_name=None, private_dns_name=None):
         self.instance_id = instance_id
         # ideally this would be ip_address, but txaws doesn't expose it.
         self.dns_name = dns_name
         self.private_dns_name = private_dns_name
-        self.launch_time = launch_time

=== modified file 'ensemble/machine/tests/test_unit_deployment.py'
--- ensemble/machine/tests/test_unit_deployment.py	2011-02-15 15:15:16 +0000
+++ ensemble/machine/tests/test_unit_deployment.py	2011-08-26 13:16:39 +0000
@@ -115,8 +115,8 @@
 
         self.failUnlessFailure(d, UnitDeploymentError)
 
-        def validate_result(result):
-            self.assertIn("No module named magichat", str(result))
+        def validate_result(error):
+            self.assertIn("No module named magichat", str(error))
 
         d.addCallback(validate_result)
         return d
@@ -170,7 +170,7 @@
         error = self.assertRaises(
             UnitDeploymentError,
             self.deployment.start, "0", get_test_zookeeper_address())
-        self.assertIn("Formula must be unpacked first.", str(error))
+        self.assertEquals(str(error), "Formula must be unpacked first.")
 
     @inlineCallbacks
     def test_service_unit_destroy(self):
@@ -269,7 +269,9 @@
         error = self.assertRaises(
             UnitDeploymentError,
             self.deployment.unpack_formula, self.formula)
-        self.assertIn("Invalid formula for deployment:", str(error))
+        self.assertEquals(
+            str(error),
+            "Invalid formula for deployment: %s" % self.formula.path)
 
     def test_is_running_no_pid_file(self):
         """

=== modified file 'ensemble/machine/unit.py'
--- ensemble/machine/unit.py	2011-02-15 15:15:16 +0000
+++ ensemble/machine/unit.py	2011-08-26 13:16:39 +0000
@@ -123,6 +123,6 @@
         """Unpack a formula to the service units directory."""
         if not isinstance(formula, FormulaBundle):
             raise UnitDeploymentError(
-                "Invalid formula for deployment: %r" % formula)
+                "Invalid formula for deployment: %s" % formula.path)
 
         formula.extract_to(os.path.join(self.directory, "formula"))

=== modified file 'ensemble/providers/common/base.py'
--- ensemble/providers/common/base.py	2011-08-23 18:47:42 +0000
+++ ensemble/providers/common/base.py	2011-08-26 13:16:39 +0000
@@ -4,7 +4,9 @@
 from twisted.internet.defer import inlineCallbacks, returnValue
 
 from ensemble.environment.errors import EnvironmentsConfigError
+
 from .bootstrap import Bootstrap
+from .connect import ZookeeperConnect
 from .findzookeepers import find_zookeepers
 from .state import SaveState, LoadState
 from .utils import get_user_authorized_keys
@@ -16,7 +18,6 @@
     To write a working subclass, you will need to override the following
     methods:
 
-        * connect
         * get_file_storage
         * start_machine
         * get_machines
@@ -57,19 +58,8 @@
     # Subclasses need to implement their own versions of everything
     # in the following block
 
-    def connect(self, share=False):
-        """Connect to the zookeeper ensemble running in the machine provider.
-
-        @param share: Requests sharing of the connection with other clients
-            attempting to connect to the same provider, if that's feasible.
-
-        returns an open C{txzookeeper.client.ZookeeperClient} and a
-        C{ensemble.storage.connection.TunnelProtocol}
-        """
-        raise NotImplementedError()
-
     def get_file_storage(self):
-        """Retrieve the provider C{FileStorage} abstraction."""
+        """Retrieve the provider FileStorage abstraction."""
         raise NotImplementedError()
 
     def start_machine(self, machine_data, master=False):
@@ -123,10 +113,23 @@
 
         @return: the first valid instance found as a single element list.
 
-        @raise: EnvironmentNotFound
+        @raise: `EnvironmentNotFound`
         """
         return find_zookeepers(self)
 
+    def connect(self, share=False):
+        """Attempt to connect to a running zookeeper node.
+
+        @param share: where feasible, attempt to share a connection with other
+            clients
+
+        @return: an open `txzookeeper.client.ZookeeperClient`
+
+        @raise: `EnvironmentNotFound` when no zookeepers exist
+        @raise: `EnvironmentPending` when zookeepers exist but connect fails
+        """
+        return ZookeeperConnect(self).run(share=share)
+
     def bootstrap(self):
         """Bootstrap an ensemble server in the provider."""
         return Bootstrap(self).run()
@@ -134,9 +137,11 @@
     def get_machine(self, instance_id):
         """Retrieve a provider machine by instance id.
 
-        @param instance_id: instance_id of the Machine you want to get.
-
-        @raise: MachinesNotFound
+        @param instance_id: instance_id of the `ProviderMachine` you want.
+
+        @return: a single `ProviderMachine`
+
+        @raise: `MachinesNotFound`
         """
         d = self.get_machines([instance_id])
         d.addCallback(itemgetter(0))
@@ -145,7 +150,7 @@
     def shutdown_machine(self, machine):
         """Terminate one machine associated with this provider.
 
-        @param machine: machine to shut down.
+        @param machine: ProviderMachine to shut down.
 
         @raise: MachinesNotFound
         """
@@ -166,14 +171,13 @@
     def save_state(self, state):
         """Save state to the provider.
 
-        @param state
-        @type dict
+        @param state: a dictionary to persist
         """
         return SaveState(self).run(state)
 
     def load_state(self):
         """Load state from the provider.
 
-        @return: a dictionary.
+        @return: a previously-persisted dictionary, or False
         """
         return LoadState(self).run()

=== renamed file 'ensemble/providers/ec2/connect.py' => 'ensemble/providers/common/connect.py'
--- ensemble/providers/ec2/connect.py	2011-08-17 08:58:51 +0000
+++ ensemble/providers/common/connect.py	2011-08-26 13:16:39 +0000
@@ -1,45 +1,55 @@
-import datetime
-from datetime import timedelta
-
 from twisted.internet.defer import inlineCallbacks, returnValue
 
+from txzookeeper.client import ConnectionTimeoutException
+
 from ensemble.errors import EnvironmentPending, NoConnection
 from ensemble.state.sshclient import SSHClient
 
 from .utils import log
 
 
-class EC2Connect(object):
+class ZookeeperConnect(object):
 
     def __init__(self, provider):
         self._provider = provider
 
+    @inlineCallbacks
     def run(self, share=False):
-        client_deferred = self._provider.get_zookeeper_machines()
-        client_deferred.addCallback(self._connect_to_machine, share)
-        client_deferred.addCallback(self._wait_for_initialization)
-        return client_deferred
-
-    def _connect_to_machine(self, machines, share):
+        """Attempt to connect to a running zookeeper node.
+
+        @param share: where feasible, attempt to share a connection with other
+            clients
+
+        @return: an open txzookeeper.client.ZookeeperClient
+
+        @raise: EnvironmentNotFound when no zookeepers exist
+        @raise: EnvironmentPending when zookeepers exist but connect fails
+        """
+        candidates = yield self._provider.get_zookeeper_machines()
+        chosen = yield self._pick_machine(candidates)
+        client = yield self._connect_to_machine(chosen, share)
+        yield self._wait_for_initialization(client)
+        returnValue(client)
+
+    def _pick_machine(self, machines):
         # TODO Should we pick a random entry from the nodes list?
+        for machine in machines:
+            if machine.dns_name:
+                return machine
+        raise EnvironmentPending("No machines have addresses assigned yet")
+
+    def _connect_to_machine(self, machine, share):
         log.info("Connecting to environment.")
-        machine = machines[0]
-        if not machine.dns_name:
-            raise EnvironmentPending("Machine %s has no address assigned yet"
-                                     % machine.instance_id)
-        client = SSHClient()
-        result = client.connect(machine.dns_name + ":2181",
-                                timeout=30, share=share)
-        result.addErrback(self._check_machine_age, machine)
+        result = SSHClient().connect(
+            machine.dns_name + ":2181", timeout=30, share=share)
+        result.addErrback(self._cannot_connect, machine)
         return result
 
-    def _check_machine_age(self, failure, machine):
-        failure.trap(NoConnection)
-        five_minutes_ago = datetime.datetime.now() - timedelta(minutes=5)
-        if machine.launch_time > five_minutes_ago:
-            raise NoConnection("Can't yet connect to started machine: %s" %
-                               str(failure.value))
-        return failure
+    def _cannot_connect(self, failure, machine):
+        failure.trap(NoConnection, ConnectionTimeoutException)
+        raise EnvironmentPending(
+            "Cannot connect to machine %s (perhaps still initializing): %s"
+            % (machine.instance_id, str(failure.value)))
 
     @inlineCallbacks
     def _wait_for_initialization(self, client):
@@ -50,4 +60,3 @@
             yield watch_d
         else:
             log.debug("Environment already initialized.")
-        returnValue(client)

=== modified file 'ensemble/providers/common/findzookeepers.py'
--- ensemble/providers/common/findzookeepers.py	2011-08-16 13:24:41 +0000
+++ ensemble/providers/common/findzookeepers.py	2011-08-26 13:16:39 +0000
@@ -26,5 +26,5 @@
 
     if machines:
         returnValue(machines)
-    raise EnvironmentNotFound("machines are not running (%s)."
+    raise EnvironmentNotFound("machines are not running (%s)"
                               % ", ".join(missing_instance_ids))

=== added file 'ensemble/providers/common/tests/test_connect.py'
--- ensemble/providers/common/tests/test_connect.py	1970-01-01 00:00:00 +0000
+++ ensemble/providers/common/tests/test_connect.py	2011-08-26 13:16:39 +0000
@@ -0,0 +1,133 @@
+from twisted.internet.defer import fail, inlineCallbacks, succeed
+
+import zookeeper
+
+from txzookeeper import ZookeeperClient
+from txzookeeper.client import ConnectionTimeoutException
+from txzookeeper.tests.utils import deleteTree
+
+from ensemble.errors import EnvironmentPending, NoConnection
+from ensemble.lib.testing import TestCase
+from ensemble.machine import ProviderMachine
+from ensemble.providers.common.base import MachineProviderBase
+from ensemble.state.sshclient import SSHClient
+from ensemble.tests.common import get_test_zookeeper_address
+
+
+class DummyProvider(MachineProviderBase):
+
+    def __init__(self, second_zookeeeper):
+        self._second_zookeeper = second_zookeeeper
+
+    def get_zookeeper_machines(self):
+        """
+        Return a pair of possible zookeepers, the first of which is invalid
+        """
+        return succeed([
+            ProviderMachine("i-havenodns"),
+            self._second_zookeeper])
+
+
+class ConnectTest(TestCase):
+
+    def mock_connect(self, share, result):
+        client = self.mocker.patch(SSHClient)
+        client.connect("foo.example.com:2181", timeout=30, share=share)
+        self.mocker.result(result)
+
+    def assert_connect_error(self, error, expect_type, expect_message):
+        self.mock_connect(False, fail(error))
+        self.mocker.replay()
+
+        provider = DummyProvider(ProviderMachine("i-exist", "foo.example.com"))
+        d = provider.connect()
+
+        def check_error(error):
+            self.assertEqual(str(error), expect_message)
+        self.assertFailure(d, expect_type)
+        d.addCallback(check_error)
+        return d
+
+    def test_none_have_dns(self):
+        """
+        `EnvironmentPending` should be raised if no zookeeper nodes have dns
+        names
+        """
+        provider = DummyProvider(ProviderMachine("i-havenodnseither"))
+        d = provider.connect()
+        self.assertFailure(d, EnvironmentPending)
+
+        def check(error):
+            self.assertEquals(
+                str(error), "No machines have addresses assigned yet")
+        d.addCallback(check)
+        return d
+
+    def test_share_kwarg(self):
+        """The `share` kwarg should be passed through to `SSHClient.connect`"""
+        client = self.mocker.mock(type=SSHClient)
+        self.mock_connect(True, succeed(client))
+        client.exists_and_watch("/initialized")
+        self.mocker.result((succeed(True), None))
+        self.mocker.replay()
+
+        provider = DummyProvider(ProviderMachine("i-amok", "foo.example.com"))
+        d = provider.connect(share=True)
+
+        def verify(result):
+            self.assertIdentical(result, client)
+        d.addCallback(verify)
+        return d
+
+    def test_no_connection(self):
+        """`NoConnection` errors should become `EnvironmentPending`s"""
+        return self.assert_connect_error(
+            NoConnection("KABOOM!"), EnvironmentPending,
+            "Cannot connect to machine i-exist (perhaps still initializing): "
+            "KABOOM!")
+
+    def test_txzookeeper_error(self):
+        """
+        `ConnectionTimeoutException` errors should become `EnvironmentPending`s
+        """
+        return self.assert_connect_error(
+            ConnectionTimeoutException("SPLAT!"), EnvironmentPending,
+            "Cannot connect to machine i-exist (perhaps still initializing): "
+            "SPLAT!")
+
+    def test_other_error(self):
+        """Other errors should propagate"""
+        return self.assert_connect_error(
+            TypeError("THUD!"), TypeError, "THUD!")
+
+    @inlineCallbacks
+    def test_wait_for_initialize(self):
+        """
+        A connection to a zookeeper that is running, but whose ensemble state
+        is not ready, should wait until that state is ready.
+        """
+        client = ZookeeperClient()
+        self.client = client # for poke_zk
+        self.mock_connect(False, succeed(client))
+        self.mocker.replay()
+
+        zookeeper.set_debug_level(0)
+        yield client.connect(get_test_zookeeper_address())
+
+        provider = DummyProvider(ProviderMachine("i-amok", "foo.example.com"))
+        d = provider.connect()
+        client_result = []
+        d.addCallback(client_result.append)
+
+        # Give it a chance to do it incorrectly.
+        yield self.poke_zk()
+
+        try:
+            self.assertEquals(client_result, [])
+            yield client.create("/initialized")
+            yield d
+            self.assertTrue(client_result, client_result)
+            self.assertIdentical(client_result[0], client)
+        finally:
+            deleteTree("/", client.handle)
+            client.close()

=== modified file 'ensemble/providers/common/tests/test_findzookeepers.py'
--- ensemble/providers/common/tests/test_findzookeepers.py	2011-08-16 13:44:01 +0000
+++ ensemble/providers/common/tests/test_findzookeepers.py	2011-08-26 13:16:39 +0000
@@ -37,12 +37,27 @@
         provider = self.get_provider(False)
         d = provider.get_zookeeper_machines()
         self.assertFailure(d, EnvironmentNotFound)
+
+        def check(error):
+            self.assertEquals(
+                str(error),
+                "Ensemble environment not found: is the environment "
+                "bootstrapped?")
+        d.addCallback(check)
         return d
 
     def test_empty_state(self):
         provider = self.get_provider({})
         d = provider.get_zookeeper_machines()
         self.assertFailure(d, EnvironmentNotFound)
+
+        def check(error):
+            self.assertEquals(
+                str(error),
+                "Ensemble environment not found: is the environment "
+                "bootstrapped?")
+        d.addCallback(check)
+        return d
         return d
 
     def test_get_machine_error_aborts(self):
@@ -56,14 +71,24 @@
         self.assertFailure(d, SomeError)
         return d
 
-    def test_bad_machine(self):
-        provider = self.get_provider({"zookeeper-instances": ["porter"]})
+    def test_bad_machines(self):
+        provider = self.get_provider({
+            "zookeeper-instances": ["porter", "carter"]})
         provider.get_machines(["porter"])
         self.mocker.result(fail(MachinesNotFound(["porter"])))
+        provider.get_machines(["carter"])
+        self.mocker.result(fail(MachinesNotFound(["carter"])))
         self.mocker.replay()
 
         d = provider.get_zookeeper_machines()
         self.assertFailure(d, EnvironmentNotFound)
+
+        def check(error):
+            self.assertEquals(
+                str(error),
+                "Ensemble environment not found: machines are not running "
+                "(porter, carter)")
+        d.addCallback(check)
         return d
 
     def test_good_machine(self):

=== modified file 'ensemble/providers/common/tests/test_utils.py'
--- ensemble/providers/common/tests/test_utils.py	2011-08-11 22:04:38 +0000
+++ ensemble/providers/common/tests/test_utils.py	2011-08-26 13:16:39 +0000
@@ -63,9 +63,7 @@
             OSError("Bad"))
         self.assertInstance(error, EnsembleError)
         self.assertEqual(
-            str(error),
-            "ProviderError: Interaction with machine provider failed: "
-            "OSError('Bad',)")
+            str(error), "Unexpected OSError interacting with provider: Bad")
 
     def test_convert_unknown_error_ignores_ensemble_error(self):
         error = self.assertRaises(
@@ -87,8 +85,7 @@
         self.assertInstance(failure.value, ProviderInteractionError)
         self.assertEqual(
             str(failure.value),
-            "ProviderError: Interaction with machine provider failed: "
-            "OSError('Bad',)")
+            "Unexpected OSError interacting with provider: Bad")
 
 
 class FormatCloudInitTest(TestCase):

=== modified file 'ensemble/providers/common/utils.py'
--- ensemble/providers/common/utils.py	2011-08-03 15:53:38 +0000
+++ ensemble/providers/common/utils.py	2011-08-26 13:16:39 +0000
@@ -31,7 +31,9 @@
         error = failure
 
     if not isinstance(error, EnsembleError):
-        error = ProviderInteractionError(error)
+        message = ("Unexpected %s interacting with provider: %s"
+                   % (type(error).__name__, str(error)))
+        error = ProviderInteractionError(message)
 
     if isinstance(failure, Failure):
         return Failure(error)

=== modified file 'ensemble/providers/ec2/__init__.py'
--- ensemble/providers/ec2/__init__.py	2011-08-23 18:47:42 +0000
+++ ensemble/providers/ec2/__init__.py	2011-08-26 13:16:39 +0000
@@ -9,9 +9,7 @@
 from ensemble.errors import (
     MachinesNotFound, ProviderError, ProviderInteractionError)
 from ensemble.providers.common.base import MachineProviderBase
-from ensemble.providers.common.utils import convert_unknown_error
 
-from .connect import EC2Connect
 from .files import FileStorage
 from .launch import EC2LaunchMachine
 from .machine import EC2ProviderMachine, machine_from_instance
@@ -48,25 +46,8 @@
         data.setdefault("secret-key", os.environ.get("AWS_SECRET_ACCESS_KEY"))
         return data
 
-    def _run_operation(self, operation, *args, **kw):
-        d = operation.run(*args, **kw)
-        d.addErrback(convert_unknown_error)
-        return d
-
-    def connect(self, share=False):
-        """Connect to the zookeeper ensemble running in the machine provider.
-
-        @param share: Requests sharing of the connection with other clients
-            attempting to connect to the same provider, if that's feasible.
-
-        returns an open C{txzookeeper.client.ZookeeperClient} and a
-        C{ensemble.storage.connection.TunnelProtocol}
-        """
-        connect = EC2Connect(self)
-        return self._run_operation(connect, share=share)
-
     def get_file_storage(self):
-        """Retrieve an S3-backed C{FileStorage}."""
+        """Retrieve an S3-backed `FileStorage`."""
         return FileStorage(self.s3, self.config["control-bucket"])
 
     def start_machine(self, machine_data, master=False):
@@ -75,7 +56,7 @@
         @param machine_data: a dictionary of data to pass along to the newly
            launched machine.
 
-        @param master: if True, machine will initialize the ensemble admin
+        @param master if True, machine will initialize the ensemble admin
             and run a provisioning agent.
         """
         return EC2LaunchMachine(self, master).run(machine_data)
@@ -84,8 +65,8 @@
     def get_machines(self, instance_ids=()):
         """List machines running in the provider.
 
-        @param instance_ids: ids of instances you want to get. Leave blank
-            to list all machines owned by ensemble.
+        @param instance_ids: ids of instances you want to get. Leave empty
+            to list all machines owned by this provider.
 
         @return: a list of EC2ProviderMachines.
 
@@ -96,13 +77,14 @@
             instances = yield self.ec2.describe_instances(*instance_ids)
         except EC2Error as error:
             code = error.get_error_codes()
+            message = error.get_error_messages()
             if code == "InvalidInstanceID.NotFound":
                 message = error.get_error_messages()
                 raise MachinesNotFound(
                     re.findall(r"\bi-[0-9a-f]{3,15}\b", message))
             raise ProviderInteractionError(
-                "EC2 error when looking up instances %s: %s"
-                % (", ".join(instance_ids), error.get_error_messages()))
+                "Unexpected EC2Error getting machines %s: %s"
+                % (", ".join(instance_ids), message))
 
         machines = []
         for instance in instances:

=== modified file 'ensemble/providers/ec2/launch.py'
--- ensemble/providers/ec2/launch.py	2011-08-10 17:23:06 +0000
+++ ensemble/providers/ec2/launch.py	2011-08-26 13:16:39 +0000
@@ -123,9 +123,8 @@
                 log.debug("Cannot delete security group %s: %s",
                           ensemble_machine_group, e)
                 raise ProviderInteractionError(
-                    "EC2 error when attempting to delete "
-                    "security group %s: %s" % (
-                    ensemble_machine_group, e))
+                        "Unexpected EC2Error deleting security group %s: %s"
+                    % (ensemble_machine_group, e.get_error_messages()))
         log.debug("Creating ensemble machine security group %s",
                   ensemble_machine_group)
         yield self._provider.ec2.create_security_group(

=== modified file 'ensemble/providers/ec2/machine.py'
--- ensemble/providers/ec2/machine.py	2011-08-11 05:59:30 +0000
+++ ensemble/providers/ec2/machine.py	2011-08-26 13:16:39 +0000
@@ -11,12 +11,7 @@
 
 
 def machine_from_instance(instance):
-    launch_time = None
-    if instance.launch_time:
-        launch_time = datetime.datetime.strptime(
-            instance.launch_time[:19], "%Y-%m-%dT%H:%M:%S")
     return EC2ProviderMachine(
         instance.instance_id,
         instance.dns_name,
-        instance.private_dns_name,
-        launch_time)
+        instance.private_dns_name)

=== modified file 'ensemble/providers/ec2/securitygroup.py'
--- ensemble/providers/ec2/securitygroup.py	2011-08-10 17:23:06 +0000
+++ ensemble/providers/ec2/securitygroup.py	2011-08-26 13:16:39 +0000
@@ -38,8 +38,8 @@
                   port, protocol, machine.instance_id)
     except EC2Error, e:
         raise ProviderInteractionError(
-            "EC2 error when attempting to open %s/%s on machine %s: %s" % (
-            port, protocol, machine.instance_id, e))
+            "Unexpected EC2Error opening %s/%s on machine %s: %s"
+            % (port, protocol, machine.instance_id, e.get_error_messages()))
 
 
 @inlineCallbacks
@@ -55,8 +55,8 @@
                   port, protocol, machine.instance_id)
     except EC2Error, e:
         raise ProviderInteractionError(
-            "EC2 error when attempting to close %s/%s on machine %s: %s" % (
-            port, protocol, machine.instance_id, e))
+            "Unexpected EC2Error closing %s/%s on machine %s: %s"
+            % (port, protocol, machine.instance_id, e.get_error_messages()))
 
 
 @inlineCallbacks
@@ -72,9 +72,8 @@
             _get_machine_group_name(provider, machine_id))
     except EC2Error, e:
         raise ProviderInteractionError(
-            "EC2 error when attempting to get opened ports "
-            "on machine %s: %s" % (
-                machine.instance_id, e))
+            "Unexpected EC2Error getting open ports on machine %s: %s"
+            % (machine.instance_id, e.get_error_messages()))
 
     opened_ports = set()  # made up of (port, protocol) pairs
     for ip_permission in security_groups[0].allowed_ips:

=== modified file 'ensemble/providers/ec2/tests/common.py'
--- ensemble/providers/ec2/tests/common.py	2011-08-22 23:03:03 +0000
+++ ensemble/providers/ec2/tests/common.py	2011-08-26 13:16:39 +0000
@@ -59,16 +59,6 @@
             "<error><Code>1</Code><Message>%s</Message></error>" % message,
             code)
 
-    def get_wrapped_ec2_error_text(self, entity_id, reason,
-                                   format="The instance ID %r does not exist"):
-        """By convention, `EC2Error` is wrapped as a `ProviderError`"""
-        message = format % entity_id
-        return (
-            "ProviderError: Interaction with machine provider failed: "
-            "\"EC2 error when attempting to %s %s: "
-            "Error Message: %s\"" % (
-                reason, entity_id, message))
-
     def setUp(self):
         # mock out the aws services
         service_factory = self.mocker.replace(

=== modified file 'ensemble/providers/ec2/tests/test_connect.py'
--- ensemble/providers/ec2/tests/test_connect.py	2011-08-17 08:58:51 +0000
+++ ensemble/providers/ec2/tests/test_connect.py	2011-08-26 13:16:39 +0000
@@ -1,6 +1,3 @@
-import datetime
-from datetime import timedelta
-
 from yaml import dump
 
 from twisted.internet.defer import inlineCallbacks, succeed, fail
@@ -8,24 +5,17 @@
 import zookeeper
 
 from txzookeeper import ZookeeperClient
+from txzookeeper.client import ConnectionTimeoutException
 from txzookeeper.tests.utils import deleteTree
 
 from ensemble.lib.testing import TestCase
 
-from ensemble.errors import NoConnection
+from ensemble.errors import EnvironmentPending, NoConnection
 from ensemble.state.sshclient import SSHClient
 from ensemble.providers.ec2.tests.common import EC2TestMixin
-from ensemble.errors import ProviderInteractionError
 from ensemble.tests.common import get_test_zookeeper_address
 
 
-def _aws_minutes_ago(minutes_ago):
-    now = datetime.datetime.now()
-    delta = timedelta(minutes=minutes_ago, seconds=1)
-    then = now - delta
-    return then.strftime("%Y-%m-%dT%H:%M:%S.000Z")
-
-
 class EC2ConnectTest(EC2TestMixin, TestCase):
 
     def mock_find_zookeepers(self, instance):
@@ -41,7 +31,25 @@
         client.connect("foo.example.com:2181", timeout=30, share=share)
         self.mocker.result(result)
 
+    def assert_connect_error(self, error, expect_type, expect_message):
+        instance = self.get_instance("i-foobar", dns_name="foo.example.com")
+        self.mock_connect(False, instance, fail(error))
+        self.mocker.replay()
+
+        provider = self.get_provider()
+        d = provider.connect()
+
+        def check_error(error):
+            self.assertEqual(str(error), expect_message)
+        self.assertFailure(d, expect_type)
+        d.addCallback(check_error)
+        return d
+
     def test_no_dns_name(self):
+        """
+        `EnvironmentPending` should be raised if no zookeeper nodes have dns
+        names
+        """
         instance = self.get_instance("i-foobar")
         self.mock_find_zookeepers(instance)
         self.mocker.replay()
@@ -51,17 +59,14 @@
 
         def check_error(error):
             self.assertEqual(
-                str(error),
-                "Ensemble environment is not accessible: Machine i-foobar has "
-                "no address assigned yet")
+                str(error), "No machines have addresses assigned yet")
 
         self.assertFailure(d, NoConnection)
         d.addCallback(check_error)
         return d
 
     def test_provider_connect_forwards_share_option(self):
-        """Test connecting to a running ensemble zookeeper server within ec2.
-        """
+        """The `share` kwarg should be passed through to `SSHClient.connect`"""
         instance = self.get_instance("i-foobar", dns_name="foo.example.com")
         connected_client = self.mocker.mock(type=SSHClient)
         self.mock_connect(True, instance, succeed(connected_client))
@@ -78,14 +83,37 @@
         d.addCallback(verify_result)
         return d
 
+    def test_no_connection(self):
+        """`NoConnection` errors should become `EnvironmentPending`s"""
+        return self.assert_connect_error(
+            NoConnection("KABOOM!"), EnvironmentPending,
+            "Cannot connect to machine i-foobar (perhaps still initializing): "
+            "KABOOM!")
+
+    def test_txzookeeper_error(self):
+        """
+        `ConnectionTimeoutException` errors should become `EnvironmentPending`s
+        """
+        return self.assert_connect_error(
+            ConnectionTimeoutException("SPLAT!"), EnvironmentPending,
+            "Cannot connect to machine i-foobar (perhaps still initializing): "
+            "SPLAT!")
+
+    def test_other_error(self):
+        """Other errors should propagate"""
+        return self.assert_connect_error(
+            TypeError("THUD!"), TypeError, "THUD!")
+
     @inlineCallbacks
     def test_provider_connect_waits_on_initialization(self):
         """
-        Test connecting to a running ensemble zookeeper server within ec2.
+        A connection to a zookeeper that is running, but whose ensemble state
+        is not ready, should wait until that state is ready.
         """
         # Hand back a real connected client to test the wait on initialization.
         instance = self.get_instance("i-foobar", dns_name="foo.example.com")
         connected_client = ZookeeperClient()
+        self.client = connected_client # for poke_zk
         self.mock_connect(False, instance, succeed(connected_client))
 
         self.mocker.replay()
@@ -99,13 +127,8 @@
         client_deferred = provider.connect()
         client_deferred.addCallback(client_result.append)
 
-        # Give it some time to do it incorrectly.
-        from twisted.internet.defer import Deferred
-        from twisted.internet import reactor
-        sleep_deferred = Deferred()
-        reactor.callLater(0.1, sleep_deferred.callback, None)
-        yield sleep_deferred
-        # XXX Replace the logic above with self.sleep(0.1) when it's merged.
+        # Give it a chance to do it incorrectly.
+        yield self.poke_zk()
 
         try:
             self.assertEquals(client_result, [])
@@ -118,68 +141,3 @@
         finally:
             deleteTree("/", connected_client.handle)
             connected_client.close()
-
-    def test_provider_connection_fails_with_recent_machine(self):
-        """
-        If we can't connect to a recently bootstrapped machine,
-        we show an error indicating the machine might still be
-        initializing.
-        """
-        instance = self.get_instance(
-            "i-foobar", dns_name="foo.example.com",
-            launch_time=_aws_minutes_ago(1))
-        self.mock_connect(False, instance, fail(NoConnection("BADABOOM!")))
-        self.mocker.replay()
-
-        provider = self.get_provider()
-        d = provider.connect()
-
-        def check_error(error):
-            self.assertEqual(
-                str(error), "Can't yet connect to started machine: BADABOOM!")
-
-        self.assertFailure(d, NoConnection)
-        d.addCallback(check_error)
-        return d
-
-    def test_provider_connection_fails_with_old_machine(self):
-        """
-        If the machine has started for a while, though, we won't
-        tweak the error message in any way.
-        """
-        instance = self.get_instance(
-            "i-foobar", dns_name="foo.example.com",
-            launch_time=_aws_minutes_ago(5))
-        self.mock_connect(False, instance, fail(NoConnection("BADABOOM!")))
-        self.mocker.replay()
-
-        provider = self.get_provider()
-        d = provider.connect()
-
-        def check_error(error):
-            self.assertEqual(str(error), "BADABOOM!")
-
-        self.assertFailure(d, NoConnection)
-        d.addCallback(check_error)
-        return d
-
-    def test_recent_machine_error_tweaking_works_with_no_conn_only(self):
-        """
-        If the error raised is not NoConnection, then it should not
-        be tweaked even if it is a recent machine.
-        """
-        instance = self.get_instance(
-            "i-foobar", dns_name="foo.example.com",
-            launch_time=_aws_minutes_ago(1))
-        self.mock_connect(False, instance, fail(ValueError("BADAPOFT!")))
-        self.mocker.replay()
-
-        provider = self.get_provider()
-        d = provider.connect()
-
-        def check_error(error):
-            self.assertEqual(str(error.error), "BADAPOFT!")
-
-        self.assertFailure(d, ProviderInteractionError)
-        d.addCallback(check_error)
-        return d

=== modified file 'ensemble/providers/ec2/tests/test_getmachines.py'
--- ensemble/providers/ec2/tests/test_getmachines.py	2011-08-17 08:58:51 +0000
+++ ensemble/providers/ec2/tests/test_getmachines.py	2011-08-26 13:16:39 +0000
@@ -132,9 +132,8 @@
         def verify(error):
             self.assertEquals(
                 str(error),
-                "ProviderError: Interaction with machine provider failed: "
-                "\"EC2 error when looking up instances i-brokeit, i-msorry: "
-                "unhelpful noises ('splat! kerpow!')\"")
+                "Unexpected EC2Error getting machines i-brokeit, i-msorry: "
+                "unhelpful noises ('splat! kerpow!')")
         d.addCallback(verify)
         return d
 

=== modified file 'ensemble/providers/ec2/tests/test_launch.py'
--- ensemble/providers/ec2/tests/test_launch.py	2011-08-16 13:55:11 +0000
+++ ensemble/providers/ec2/tests/test_launch.py	2011-08-26 13:16:39 +0000
@@ -141,10 +141,9 @@
             ProviderInteractionError)
         self.assertEqual(
             str(ex),
-            self.get_wrapped_ec2_error_text(
-                "ensemble-moon-machine-1", "delete security group",
-                "There are active instances using security group %r"
-                ))
+            "Unexpected EC2Error deleting security group "
+            "ensemble-moon-machine-1: There are active instances using "
+            "security group 'ensemble-moon-machine-1'")
 
     def test_provider_type_machine_variable(self):
         """The provider type is available via cloud-init."""

=== modified file 'ensemble/providers/ec2/tests/test_machine.py'
--- ensemble/providers/ec2/tests/test_machine.py	2011-07-20 07:10:30 +0000
+++ ensemble/providers/ec2/tests/test_machine.py	2011-08-26 13:16:39 +0000
@@ -1,5 +1,3 @@
-import datetime
-
 from txaws.ec2.model import Instance
 
 from ensemble.lib.testing import TestCase
@@ -13,27 +11,10 @@
         instance = Instance(
             "i-foobar", "ignored",
             dns_name="public",
-            private_dns_name="private",
-            launch_time="2009-04-27T02:23:18.000Z")
-
-        machine = machine_from_instance(instance)
-        self.assertTrue(isinstance(machine, EC2ProviderMachine))
-        self.assertEquals(machine.instance_id, instance.instance_id)
-        self.assertEquals(machine.dns_name, instance.dns_name)
-        self.assertEquals(machine.private_dns_name, instance.private_dns_name)
-        self.assertEquals(machine.launch_time,
-                          datetime.datetime(2009, 4, 27, 2, 23, 18))
-
-    def test_machine_from_instance_no_launch_time(self):
-        instance = Instance(
-            "i-foobar", "ignored",
-            dns_name="public",
-            private_dns_name="private",
-            launch_time="")
-
-        machine = machine_from_instance(instance)
-        self.assertTrue(isinstance(machine, EC2ProviderMachine))
-        self.assertEquals(machine.instance_id, instance.instance_id)
-        self.assertEquals(machine.dns_name, instance.dns_name)
-        self.assertEquals(machine.private_dns_name, instance.private_dns_name)
-        self.assertEquals(machine.launch_time, None)
+            private_dns_name="private")
+
+        machine = machine_from_instance(instance)
+        self.assertTrue(isinstance(machine, EC2ProviderMachine))
+        self.assertEquals(machine.instance_id, instance.instance_id)
+        self.assertEquals(machine.dns_name, instance.dns_name)
+        self.assertEquals(machine.private_dns_name, instance.private_dns_name)

=== modified file 'ensemble/providers/ec2/tests/test_provider.py'
--- ensemble/providers/ec2/tests/test_provider.py	2011-08-11 19:49:32 +0000
+++ ensemble/providers/ec2/tests/test_provider.py	2011-08-26 13:16:39 +0000
@@ -127,4 +127,7 @@
         config["authorized-keys-path"] = "File path"
         error = self.assertRaises(EnvironmentsConfigError,
                                   MachineProvider, "some-env-name", config)
-        self.assertIn("authorized-keys", str(error))
+        self.assertEquals(
+            str(error),
+            "Environment config cannot define both authorized-keys and "
+            "authorized-keys-path. Pick one!")

=== modified file 'ensemble/providers/ec2/tests/test_securitygroup.py'
--- ensemble/providers/ec2/tests/test_securitygroup.py	2011-08-10 17:19:09 +0000
+++ ensemble/providers/ec2/tests/test_securitygroup.py	2011-08-26 13:16:39 +0000
@@ -89,8 +89,8 @@
             ProviderInteractionError)
         self.assertEqual(
             str(ex),
-            self.get_wrapped_ec2_error_text(
-                "i-foobar", "open 80/tcp on machine"))
+            "Unexpected EC2Error opening 80/tcp on machine i-foobar: "
+            "The instance ID 'i-foobar' does not exist")
 
     @inlineCallbacks
     def test_close_provider_port_unknown_instance(self):
@@ -108,8 +108,8 @@
             ProviderInteractionError)
         self.assertEqual(
             str(ex),
-            self.get_wrapped_ec2_error_text(
-                "i-foobar", "close 80/tcp on machine"))
+            "Unexpected EC2Error closing 80/tcp on machine i-foobar: "
+            "The instance ID 'i-foobar' does not exist")
 
     @inlineCallbacks
     def test_get_provider_opened_ports_unknown_instance(self):
@@ -125,5 +125,5 @@
             ProviderInteractionError)
         self.assertEqual(
             str(ex),
-            self.get_wrapped_ec2_error_text(
-                "i-foobar", "get opened ports on machine"))
+            "Unexpected EC2Error getting open ports on machine i-foobar: "
+            "The instance ID 'i-foobar' does not exist")

=== modified file 'ensemble/providers/orchestra/__init__.py'
--- ensemble/providers/orchestra/__init__.py	2011-08-25 16:11:49 +0000
+++ ensemble/providers/orchestra/__init__.py	2011-08-26 13:16:39 +0000
@@ -20,7 +20,7 @@
         self.cobbler = CobblerClient(config)
 
     def get_file_storage(self):
-        """Retrieve the provider C{FileStorage} abstraction."""
+        """Return a WebDAV-backed FileStorage abstraction."""
         if "storage-url" not in self.config:
             return FileStorage(
                 "http://%(orchestra-server)s/webdav" % self.config)
@@ -41,10 +41,10 @@
     def get_machines(self, instance_ids=()):
         """List machines running in the provider.
 
-        @param instance_ids: ids of instances you want to get. Leave blank
-            to list all machines owned by ensemble.
+        @param instance_ids: ids of instances you want to get. Leave empty
+            to list all machines owned by this provider.
 
-        @return: a list of EC2ProviderMachines.
+        @return: a list of OrchestraMachines.
 
         @raise: MachinesNotFound
         """

=== modified file 'ensemble/providers/orchestra/cobbler.py'
--- ensemble/providers/orchestra/cobbler.py	2011-08-22 13:01:02 +0000
+++ ensemble/providers/orchestra/cobbler.py	2011-08-26 13:16:39 +0000
@@ -3,8 +3,7 @@
 from twisted.internet.defer import inlineCallbacks, returnValue
 from twisted.web.xmlrpc import Proxy
 
-from ensemble.errors import (
-    MachinesNotFound, ProviderError, ProviderInteractionError)
+from ensemble.errors import MachinesNotFound, ProviderError
 
 
 class CobblerCaller(object):
@@ -62,7 +61,7 @@
         def check(actual):
             if actual != expect:
                 raise ProviderError(
-                    "Bad result from call to %s with %s (got %r, expected %r)"
+                    "Bad result from call to %s with %s: got %r, expected %r"
                     % (name, args, actual, expect))
             return actual
 
@@ -84,8 +83,9 @@
 
         def extract_name(systems):
             if len(systems) > 1:
-                raise ProviderInteractionError(
-                    "Got multiple instances with id %s" % instance_id)
+                raise ProviderError(
+                    "Got multiple names for machine %s: %s"
+                    % (instance_id, ", ".join(systems)))
             if not systems:
                 raise MachinesNotFound([instance_id])
             return systems[0]

=== modified file 'ensemble/providers/orchestra/machine.py'
--- ensemble/providers/orchestra/machine.py	2011-08-11 04:30:55 +0000
+++ ensemble/providers/orchestra/machine.py	2011-08-26 13:16:39 +0000
@@ -6,4 +6,8 @@
 
 
 def machine_from_dict(d):
+    """Convert a `dict` into an `OrchestraMachine`.
+
+    Expects dicts as returned by `CobblerClient.get_system[s]`
+    """
     return OrchestraMachine(d["uid"], d["name"], d["name"])

=== modified file 'ensemble/providers/orchestra/tests/test_cobbler.py'
--- ensemble/providers/orchestra/tests/test_cobbler.py	2011-08-12 21:35:34 +0000
+++ ensemble/providers/orchestra/tests/test_cobbler.py	2011-08-26 13:16:39 +0000
@@ -3,8 +3,7 @@
 from twisted.internet.defer import fail, succeed
 from twisted.web.xmlrpc import Proxy
 
-from ensemble.errors import (
-    MachinesNotFound, ProviderError, ProviderInteractionError)
+from ensemble.errors import MachinesNotFound, ProviderError
 from ensemble.lib.testing import TestCase
 from ensemble.providers.orchestra.cobbler import CobblerCaller, CobblerClient
 
@@ -243,11 +242,14 @@
         d.addCallback(verify)
         return d
 
-    def check_bad_call(self, d, method):
+    def check_bad_call(self, d, method, args):
         self.assertFailure(d, ProviderError)
 
         def verify(error):
-            self.assertIn("Bad result from call to %s" % method, str(error))
+            self.assertEquals(
+                str(error),
+                "Bad result from call to %s with %s: got False, expected True"
+                % (method, args))
         d.addCallback(verify)
         return d
 
@@ -261,11 +263,13 @@
         self.mock_get_name(succeed(["some-name", "another-name"]))
         self.mocker.replay()
         d = self.call_cobbler_method(method_name, *args)
-        self.assertFailure(d, ProviderInteractionError)
+        self.assertFailure(d, ProviderError)
 
         def check_error(error):
-            self.assertIn("Got multiple instances with id some-uid",
-                          str(error))
+            self.assertEquals(
+                str(error),
+                "Got multiple names for machine some-uid: some-name, "
+                "another-name")
         d.addCallback(check_error)
         return d
 
@@ -301,7 +305,8 @@
         self.mock_modify_system(succeed(False), key, value)
         self.mocker.replay()
         d = self.call_cobbler_method(method_name, *args)
-        return self.check_bad_call(d, "modify_system")
+        return self.check_bad_call(
+            d, "modify_system", ("some-handle", key, value))
 
     def check_modify_system_error(self, key, value, method_name, *args):
         self.mock_modify_system(fail(SomeError()), key, value)
@@ -314,7 +319,7 @@
         self.mock_save_system(succeed(False))
         self.mocker.replay()
         d = self.call_cobbler_method(method_name, *args)
-        return self.check_bad_call(d, "save_system")
+        return self.check_bad_call(d, "save_system", ("some-handle",))
 
     def check_save_system_error(self, method_name, *args):
         self.mock_save_system(fail(SomeError()))

=== added file 'ensemble/providers/orchestra/tests/test_connect.py'
--- ensemble/providers/orchestra/tests/test_connect.py	1970-01-01 00:00:00 +0000
+++ ensemble/providers/orchestra/tests/test_connect.py	2011-08-26 13:16:39 +0000
@@ -0,0 +1,111 @@
+from yaml import dump
+
+from twisted.internet.defer import fail, inlineCallbacks, succeed
+
+import zookeeper
+
+from txzookeeper import ZookeeperClient
+from txzookeeper.client import ConnectionTimeoutException
+from txzookeeper.tests.utils import deleteTree
+
+from ensemble.errors import EnvironmentPending, NoConnection
+from ensemble.lib.testing import TestCase
+from ensemble.state.sshclient import SSHClient
+from ensemble.tests.common import get_test_zookeeper_address
+
+from .common import OrchestraTestMixin
+
+
+class ConnectTest(TestCase, OrchestraTestMixin):
+
+    def mock_connect(self, share, result):
+        self.setup_mocks()
+        self.getPage("http://somewhe.re/webdav/provider-state")
+        self.mocker.result(succeed(dump(
+            {"zookeeper-instances": ["foo"]})))
+        self.proxy_m.callRemote("get_systems")
+        self.mocker.result(succeed([{"uid": "foo",
+                                     "name": "foo.example.com",
+                                     "mgmt_classes": ["acquired"]}]))
+        client = self.mocker.patch(SSHClient)
+        client.connect("foo.example.com:2181", timeout=30, share=share)
+        self.mocker.result(result)
+
+    def assert_connect_error(self, error, expect_type, expect_message):
+        self.mock_connect(False, fail(error))
+        self.mocker.replay()
+
+        d = self.get_provider().connect()
+
+        def check_error(error):
+            self.assertEqual(str(error), expect_message)
+        self.assertFailure(d, expect_type)
+        d.addCallback(check_error)
+        return d
+
+    def test_share_kwarg(self):
+        """The `share` kwarg should be passed through to `SSHClient.connect`"""
+        client = self.mocker.mock(type=SSHClient)
+        self.mock_connect(True, succeed(client))
+        client.exists_and_watch("/initialized")
+        self.mocker.result((succeed(True), None))
+        self.mocker.replay()
+
+        d = self.get_provider().connect(share=True)
+
+        def verify(result):
+            self.assertIdentical(result, client)
+        d.addCallback(verify)
+        return d
+
+    def test_no_connection(self):
+        """`NoConnection` errors should become `EnvironmentPending`s"""
+        return self.assert_connect_error(
+            NoConnection("KABOOM!"), EnvironmentPending,
+            "Cannot connect to machine foo (perhaps still initializing): "
+            "KABOOM!")
+
+    def test_txzookeeper_error(self):
+        """
+        `ConnectionTimeoutException` errors should become `EnvironmentPending`s
+        """
+        return self.assert_connect_error(
+            ConnectionTimeoutException("SPLAT!"), EnvironmentPending,
+            "Cannot connect to machine foo (perhaps still initializing): "
+            "SPLAT!")
+
+    def test_other_error(self):
+        """Other errors should propagate"""
+        return self.assert_connect_error(
+            TypeError("THUD!"), TypeError, "THUD!")
+
+    @inlineCallbacks
+    def test_wait_for_initialize(self):
+        """
+        A connection to a zookeeper that is running, but whose ensemble state
+        is not ready, should wait until that state is ready.
+        """
+        client = ZookeeperClient()
+        self.client = client # for poke_zk
+        self.mock_connect(False, succeed(client))
+        self.mocker.replay()
+
+        zookeeper.set_debug_level(0)
+        yield client.connect(get_test_zookeeper_address())
+
+        d = self.get_provider().connect()
+        client_result = []
+        d.addCallback(client_result.append)
+
+        # Give it an opportunity to do it incorrectly.
+        yield self.poke_zk()
+
+        try:
+            self.assertEquals(client_result, [])
+            yield client.create("/initialized")
+            yield d
+            self.assertTrue(client_result, client_result)
+            self.assertIdentical(client_result[0], client)
+        finally:
+            deleteTree("/", client.handle)
+            client.close()

=== modified file 'ensemble/providers/orchestra/tests/test_launch.py'
--- ensemble/providers/orchestra/tests/test_launch.py	2011-08-11 23:27:45 +0000
+++ ensemble/providers/orchestra/tests/test_launch.py	2011-08-26 13:16:39 +0000
@@ -30,9 +30,13 @@
 
         d = self.get_provider().start_machine({"machine-id": "32767"})
         self.assertFailure(d, ProviderError)
-        self.assertIn("Could not find any Cobbler systems marked as available "
-                      "and configured for network boot.",
-                      str(d))
+
+        def verify(error):
+            self.assertEquals(
+                str(error),
+                "Could not find any Cobbler systems marked as available and "
+                "configured for network boot.")
+        d.addCallback(verify)
         return d
 
     def test_no_acceptable_machines(self):
@@ -43,9 +47,13 @@
 
         d = self.get_provider().start_machine({"machine-id": "32767"})
         self.assertFailure(d, ProviderError)
-        self.assertIn("All available Cobbler systems were also marked as "
-                      "acquired (instances: bad-system-uid).",
-                      str(d))
+
+        def verify(error):
+            self.assertEquals(
+                str(error),
+                "All available Cobbler systems were also marked as acquired "
+                "(instances: bad-system-uid).")
+        d.addCallback(verify)
         return d
 
     def test_actually_launch(self):

=== modified file 'ensemble/providers/orchestra/tests/test_provider.py'
--- ensemble/providers/orchestra/tests/test_provider.py	2011-08-23 16:07:09 +0000
+++ ensemble/providers/orchestra/tests/test_provider.py	2011-08-26 13:16:39 +0000
@@ -53,7 +53,10 @@
         config["authorized-keys-path"] = "File path"
         error = self.assertRaises(EnvironmentsConfigError,
                                   MachineProvider, "some-env-name", config)
-        self.assertIn("authorized-keys", str(error))
+        self.assertEquals(
+            str(error),
+            "Environment config cannot define both authorized-keys and "
+            "authorized-keys-path. Pick one!")
 
     @inlineCallbacks
     def test_open_port(self):
@@ -75,4 +78,3 @@
         yield MachineProvider("blah", CONFIG).get_opened_ports(None, None)
         self.assertIn(
             "Firewalling is not yet implemented in Orchestra", log.getvalue())
-

=== modified file 'ensemble/state/service.py'
--- ensemble/state/service.py	2011-08-08 03:22:15 +0000
+++ ensemble/state/service.py	2011-08-26 13:16:39 +0000
@@ -855,7 +855,8 @@
         cleared.
         """
         if not isinstance(hook_names, (list, tuple)):
-            raise AssertionError("hook names must be a list %r" % hook_names)
+            raise AssertionError("Hook names must be a list: got %r"
+                                 % hook_names)
 
         if "*" in hook_names and len(hook_names) > 1:
             msg = "Ambigious to debug all hooks and named hooks %r" % (

=== modified file 'ensemble/state/sshclient.py'
--- ensemble/state/sshclient.py	2011-08-15 00:41:54 +0000
+++ ensemble/state/sshclient.py	2011-08-26 13:16:39 +0000
@@ -193,6 +193,8 @@
             else:
                 returnValue(self)
         self.close()
+        # we raise ConnectionTimeoutException (rather than one of our own, with
+        # the same meaning) to maintain ZookeeperClient interface
         raise ConnectionTimeoutException(
             "could not connect before timeout after %d retries" % num_retries)
 

=== modified file 'ensemble/state/tests/test_security.py'
--- ensemble/state/tests/test_security.py	2011-08-05 22:55:52 +0000
+++ ensemble/state/tests/test_security.py	2011-08-26 13:16:39 +0000
@@ -316,7 +316,7 @@
         principal = Principal("zebra", "zoo")
         error = yield self.assertFailure(self.db.get(principal.name),
                                    PrincipalNotFound)
-        self.assertIn("zebra", str(error))
+        self.assertEquals(str(error), "Principal 'zebra' not found")
 
 
 class PolicyTest(TestCase):

=== modified file 'ensemble/state/tests/test_service.py'
--- ensemble/state/tests/test_service.py	2011-08-22 23:02:38 +0000
+++ ensemble/state/tests/test_service.py	2011-08-26 13:16:39 +0000
@@ -1111,8 +1111,10 @@
         error = yield self.assertFailure(
             unit_state.enable_hook_debug(["*", "db-relation-changed"]),
             ValueError)
-        self.assertIn("Ambigious to debug all hooks and named hooks",
-                      str(error))
+        self.assertEquals(
+            str(error),
+            "Ambigious to debug all hooks and named hooks "
+            "['*', 'db-relation-changed']")
 
     @inlineCallbacks
     def test_enable_debug_requires_sequence(self):
@@ -1123,7 +1125,7 @@
         error = yield self.assertFailure(
             unit_state.enable_hook_debug(None),
             AssertionError)
-        self.assertIn("hook names must be a list", str(error))
+        self.assertEquals(str(error), "Hook names must be a list: got None")
 
     @inlineCallbacks
     def test_enable_named_debug_hook(self):
@@ -1144,8 +1146,10 @@
         """
         unit_state = yield self.get_unit_state()
         yield unit_state.enable_hook_debug(["*"])
-        yield self.assertFailure(unit_state.enable_hook_debug(["*"]),
-                                 ServiceUnitDebugAlreadyEnabled)
+        error = yield self.assertFailure(unit_state.enable_hook_debug(["*"]),
+                                         ServiceUnitDebugAlreadyEnabled)
+        self.assertEquals(
+            str(error), "Service unit 'wordpress/0' is already in debug mode.")
 
     @inlineCallbacks
     def test_enable_debug_hook_lifetime(self):

=== modified file 'ensemble/tests/test_errors.py'
--- ensemble/tests/test_errors.py	2011-08-17 17:26:19 +0000
+++ ensemble/tests/test_errors.py	2011-08-26 13:16:39 +0000
@@ -23,20 +23,20 @@
 
     def test_FileNotFound(self):
         error = FileNotFound("/path")
-        self.assertEquals(str(error), "File was not found: /path")
+        self.assertEquals(str(error), "File was not found: '/path'")
         self.assertIsEnsembleError(error)
 
     def test_FileAlreadyExists(self):
         error = FileAlreadyExists("/path")
         self.assertEquals(str(error),
-                          "File already exists, won't overwrite: /path")
+                          "File already exists, won't overwrite: '/path'")
         self.assertIsEnsembleError(error)
 
     def test_InvalidEnsembleHeaderValue(self):
         error = InvalidEnsembleHeaderValue("/path", "xpctd", "fnd")
         self.assertEquals(str(error),
                           "Expected a YAML file with an 'ensemble: xpctd' "
-                          "header, found 'ensemble: fnd': /path")
+                          "header, found 'ensemble: fnd': '/path'")
         self.assertIsEnsembleError(error)
 
     def test_NoConnection(self):
@@ -62,12 +62,9 @@
         self.assertIsEnsembleError(error)
 
     def test_ProviderInteractionError(self):
-        error = ProviderInteractionError(OSError("Bad Stuff"))
+        error = ProviderInteractionError("Bad Stuff")
         self.assertIsEnsembleError(error)
-        self.assertEquals(
-            str(error),
-            "ProviderError: Interaction with machine provider failed: "
-            "OSError('Bad Stuff',)")
+        self.assertEquals(str(error), "Bad Stuff")
 
     def test_CannotTerminateMachine(self):
         error = CannotTerminateMachine(0, "environment would be destroyed")
@@ -101,11 +98,4 @@
 
     def testEnvironmentPendingWithInfo(self):
         error = EnvironmentPending("problem")
-        self.assertEquals(str(error),
-                          "Ensemble environment is not accessible: problem")
-
-    def testEnvironmentPendingNoInfo(self):
-        error = EnvironmentPending()
-        self.assertEquals(str(error),
-                          "Ensemble environment is not accessible: no "
-                          "details available")
+        self.assertEquals(str(error), "problem")

=== modified file 'ensemble/unit/tests/test_formula.py'
--- ensemble/unit/tests/test_formula.py	2011-08-18 21:33:55 +0000
+++ ensemble/unit/tests/test_formula.py	2011-08-26 13:16:39 +0000
@@ -191,4 +191,5 @@
                 self.client, "local:mickey-21", formula_directory),
             FormulaStateNotFound)
 
-        self.assertIn("Formula 'local:mickey-21' was not found", str(error))
+        self.assertEquals(str(error),
+                          "Formula 'local:mickey-21' was not found")