From 3dda1dcdc045ab1b7380ede876274cfee2c900ba Mon Sep 17 00:00:00 2001 From: Cory Johns Date: Fri, 29 Sep 2017 12:48:19 -0400 Subject: [PATCH] Fix test failures (#163) * Update tox.ini to enable testing with other Py envs * Remove invalid failing test Fixes #162 * Fix error type when connecting * Fix KeyError on model name during integration tests * Fix model name collision * Gracefully downgrade when controller supports newer facades than we do Fixes #145 --- juju/client/_client.py | 23 ++++++++++------------- juju/client/connection.py | 2 +- juju/controller.py | 15 +++++++++------ tests/base.py | 31 +++++++++++++++++++++++++++---- tests/integration/test_model.py | 12 ------------ tox.ini | 2 ++ 6 files changed, 49 insertions(+), 36 deletions(-) diff --git a/juju/client/_client.py b/juju/client/_client.py index d510e11..2ef0ffd 100644 --- a/juju/client/_client.py +++ b/juju/client/_client.py @@ -1,7 +1,7 @@ # DO NOT CHANGE THIS FILE! This file is auto-generated by facade.py. # Changes will be overwritten/lost when the file is regenerated. -from juju.client._definitions import * +from juju.client._definitions import * # noqa from juju.client import _client1, _client2, _client3, _client4, _client5 @@ -15,22 +15,21 @@ CLIENTS = { } - def lookup_facade(name, version): """ Given a facade name and version, attempt to pull that facade out of the correct client.py file. """ - try: - facade = getattr(CLIENTS[str(version)], name) - except KeyError: - raise ImportError("No facades found for version {}".format(version)) - except AttributeError: - raise ImportError( - "No facade with name '{}' in version {}".format(name, version)) - return facade - + for _version in range(int(version), 0, -1): + try: + facade = getattr(CLIENTS[str(_version)], name) + return facade + except (KeyError, AttributeError): + continue + else: + raise ImportError("No supported version for facade: " + "{}".format(name)) class TypeFactory: @@ -363,5 +362,3 @@ class UserManagerFacade(TypeFactory): class VolumeAttachmentsWatcherFacade(TypeFactory): pass - - diff --git a/juju/client/connection.py b/juju/client/connection.py index 7457391..35aaf17 100644 --- a/juju/client/connection.py +++ b/juju/client/connection.py @@ -413,7 +413,7 @@ class Connection: endpoints.extend(new_endpoints) else: # ran out of endpoints without a successful login - raise Exception("Couldn't authenticate to {}".format( + raise JujuConnectionError("Couldn't authenticate to {}".format( self._endpoint)) response = result['response'] diff --git a/juju/controller.py b/juju/controller.py index 98dd59d..d48ca8a 100644 --- a/juju/controller.py +++ b/juju/controller.py @@ -1,6 +1,7 @@ import asyncio import logging +from . import errors from . import tag from . import utils from .client import client @@ -39,9 +40,11 @@ class Controller(object): """Connect to the current Juju controller. """ - self.connection = ( - await connection.Connection.connect_current_controller( - max_frame_size=self.max_frame_size)) + jujudata = connection.JujuData() + controller_name = jujudata.current_controller() + if not controller_name: + raise errors.JujuConnectionError('No current controller') + return await self.connect_controller(controller_name) async def connect_controller(self, controller_name): """Connect to a Juju controller by name. @@ -95,10 +98,11 @@ class Controller(object): credential = None log.debug('Creating model %s', model_name) - + if not config or 'authorized-keys' not in config: config = config or {} - config['authorized-keys'] = await utils.read_ssh_key(loop=self.loop) + config['authorized-keys'] = await utils.read_ssh_key( + loop=self.loop) model_info = await model_facade.CreateModel( tag.cloud(cloud_name), @@ -122,7 +126,6 @@ class Controller(object): return model - async def destroy_models(self, *uuids): """Destroy one or more models. diff --git a/tests/base.py b/tests/base.py index e1ec452..d065ba3 100644 --- a/tests/base.py +++ b/tests/base.py @@ -34,15 +34,22 @@ class CleanController(): class CleanModel(): def __init__(self): + self.user_name = None self.controller = None + self.controller_name = None self.model = None + self.model_name = None + self.model_uuid = None async def __aenter__(self): self.controller = Controller() - await self.controller.connect_current() + juju_data = JujuData() + self.controller_name = juju_data.current_controller() + self.user_name = juju_data.accounts()[self.controller_name]['user'] + await self.controller.connect_controller(self.controller_name) - model_name = 'model-{}'.format(uuid.uuid4()) - self.model = await self.controller.add_model(model_name) + self.model_name = 'model-{}'.format(uuid.uuid4()) + self.model = await self.controller.add_model(self.model_name) # save the model UUID in case test closes model self.model_uuid = self.model.info.uuid @@ -50,12 +57,28 @@ class CleanModel(): # Ensure that we connect to the new model by default. This also # prevents failures if test was started with no current model. self._patch_cm = mock.patch.object(JujuData, 'current_model', - return_value=model_name) + return_value=self.model_name) self._patch_cm.start() + # Ensure that the models data includes this model, since it doesn't + # get added to the client store by Controller.add_model(). + self._orig_models = JujuData().models + self._patch_models = mock.patch.object(JujuData, 'models', + side_effect=self._models) + self._patch_models.start() + return self.model + def _models(self): + result = self._orig_models() + models = result[self.controller_name]['models'] + full_model_name = '{}/{}'.format(self.user_name, self.model_name) + if full_model_name not in models: + models[full_model_name] = {'uuid': self.model_uuid} + return result + async def __aexit__(self, exc_type, exc, tb): + self._patch_models.stop() self._patch_cm.stop() await self.model.disconnect() await self.controller.destroy_model(self.model_uuid) diff --git a/tests/integration/test_model.py b/tests/integration/test_model.py index 8506786..041f75a 100644 --- a/tests/integration/test_model.py +++ b/tests/integration/test_model.py @@ -129,18 +129,6 @@ async def _deploy_in_loop(new_loop, model_name): await new_model.disconnect() -@base.bootstrapped -@pytest.mark.asyncio -async def test_explicit_loop(event_loop): - async with base.CleanModel() as model: - model_name = model.info.name - new_loop = asyncio.new_event_loop() - new_loop.run_until_complete( - _deploy_in_loop(new_loop, model_name)) - await model._wait_for_new('application', 'ubuntu') - assert 'ubuntu' in model.applications - - @base.bootstrapped @pytest.mark.asyncio async def test_explicit_loop_threaded(event_loop): diff --git a/tox.ini b/tox.ini index 789bbeb..849ff0f 100644 --- a/tox.ini +++ b/tox.ini @@ -9,6 +9,8 @@ skipsdist=True [testenv] usedevelop=True +# for testing with other python versions +commands = py.test -ra -v -s -x -n auto {posargs} passenv = HOME deps = -- 2.25.1