Merge pull request #422 from hpidcock/various-fixes

jujubot · web-flow · commit 91194f08a34c · 2020-05-18T11:24:34.000+10:00
#422 - Fix positional argument usage in facade calls. - Add get shim to facade types. - Fix SSH await on unit - Fix integration tests Fixes #420
diff --git a/.github/workflows/tox.yaml b/.github/workflows/tox.yaml
@@ -1,40 +1,17 @@
-name: Run tests with Tox
+name: Unit Tests
 
-on: [push]
+on: [push, pull_request]
 
 jobs:
-  build:
-
+  unit-tests:
     runs-on: ubuntu-latest
-    strategy:
-      matrix:
-        python: [3.6, 3.7, 3.8]
-        juju: [stable, edge]
-
     steps:
       - uses: actions/checkout@v2
       - name: Setup Python
         uses: actions/setup-python@v1
         with:
-          python-version: ${{ matrix.python }}
+          python-version: 3.8
       - name: Install Tox and any other packages
         run: pip install tox
       - name: Run Lint and Unit Tests
         run: tox -e lint,py3
-      - name: Install Snaps
-        run: |
-          sudo snap install lxd
-          sudo snap install juju --classic --channel ${{ matrix.juju }}
-          sudo snap install juju-wait --classic
-      - name: Configure LXD
-        run: |
-          sudo lxd waitready --timeout 30
-          sudo chmod 666 /var/snap/lxd/common/lxd/unix.socket
-          lxd init --auto --network-address='[::]' --network-port=8443 --storage-backend=dir
-          lxc network set lxdbr0 ipv6.address none
-      - name: Bootstrap Juju Controller
-        run: juju bootstrap localhost test --config 'identity-url=https://api.staging.jujucharms.com/identity' --config 'allow-model-access=true' --config 'test-mode=true'
-      - name: Run Integration Tests
-        run: tox -e integration,serial
-        env:
-          TEST_AGENTS: '{"agents":[{"url":"https://api.staging.jujucharms.com/identity","username":"libjuju-ci@yellow"}],"key":{"private":"88OOCxIHQNguRG7zFg2y2Hx5Ob0SeVKKBRnjyehverc=","public":"fDn20+5FGyN2hYO7z0rFUyoHGUnfrleslUNtoYsjNSs="}}'
diff --git a/juju/client/facade.py b/juju/client/facade.py
@@ -654,6 +654,16 @@ def __setitem__(self, key, value):
         attr = self._toPy[key]
         setattr(self, attr, value)
 
+    # legacy: generated definitions used to not correctly
+    # create typed objects and would use dict instead (from JSON)
+    # so we emulate some dict methods.
+    def get(self, key, default=None):
+        try:
+            attr = self._toPy[key]
+        except KeyError:
+            return default
+        return getattr(self, attr, default)
+
 
 class Schema(dict):
     def __init__(self, schema):
diff --git a/juju/controller.py b/juju/controller.py
@@ -447,7 +447,7 @@ async def reset_user_password(self, username):
         user_facade = client.UserManagerFacade.from_connection(
             self.connection())
         entity = client.Entity(tag.user(username))
-        results = await user_facade.ResetPassword([entity])
+        results = await user_facade.ResetPassword(entities=[entity])
         secret_key = results.results[0].secret_key
         return await self.get_user(username, secret_key=secret_key)
 
@@ -755,7 +755,7 @@ async def create_offer(self, model_uuid, endpoint, offer_name=None, application_
         params.model_tag = tag.model(model_uuid)
 
         facade = client.ApplicationOffersFacade.from_connection(self.connection())
-        return await facade.Offer([params])
+        return await facade.Offer(offers=[params])
 
     async def list_offers(self, model_name):
         """
@@ -766,7 +766,7 @@ async def list_offers(self, model_name):
         params.model_name = model_name
 
         facade = client.ApplicationOffersFacade.from_connection(self.connection())
-        return await facade.ListApplicationOffers([params])
+        return await facade.ListApplicationOffers(filters=[params])
 
     async def remove_offer(self, model_uuid, offer, force=False):
         """
@@ -791,15 +791,15 @@ async def remove_offer(self, model_uuid, offer, force=False):
             raise RemoveError("removing offer will also remove relations, use force and try again.")
 
         facade = client.ApplicationOffersFacade.from_connection(self.connection())
-        return await facade.DestroyOffers(force, [url.string()])
+        return await facade.DestroyOffers(force=force, offer_urls=[url.string()])
 
     async def get_consume_details(self, endpoint):
         """
         get_consume_details returns the details necessary to pass to another
         model to consume the specified offers represented by the urls.
         """
         facade = client.ApplicationOffersFacade.from_connection(self.connection())
-        offers = await facade.GetConsumeDetails([endpoint])
+        offers = await facade.GetConsumeDetails(offer_urls=[endpoint])
         if len(offers.results) != 1:
             raise JujuAPIError("expected to find one result")
         result = offers.results[0]
diff --git a/juju/machine.py b/juju/machine.py
@@ -215,7 +215,8 @@ async def ssh(
             '-q',
             destination
         ]
-        cmd.extend(ssh_opts.split() if isinstance(ssh_opts, str) else ssh_opts)
+        if ssh_opts:
+            cmd.extend(ssh_opts.split() if isinstance(ssh_opts, str) else ssh_opts)
         cmd.extend([command])
         loop = self.model.loop
         process = await asyncio.create_subprocess_exec(*cmd, loop=loop)
diff --git a/juju/unit.py b/juju/unit.py
@@ -99,7 +99,7 @@ async def destroy(self):
         log.debug(
             'Destroying %s', self.name)
 
-        return await app_facade.DestroyUnits([self.name])
+        return await app_facade.DestroyUnits(unit_names=[self.name])
     remove = destroy
 
     def get_resources(self, details=False):
@@ -222,7 +222,7 @@ def set_meter_status(self):
         """
         raise NotImplementedError()
 
-    def ssh(
+    async def ssh(
             self, command, user=None, proxy=False, ssh_opts=None):
         """Execute a command over SSH on this unit.
 
@@ -232,7 +232,7 @@ def ssh(
         :param str ssh_opts: Additional options to the `ssh` command
 
         """
-        self.machine.ssh(command, user, proxy, ssh_opts)
+        return await self.machine.ssh(command, user, proxy, ssh_opts)
 
     def status_history(self, num=20, utc=False):
         """Get status history for this unit.
diff --git a/tests/integration/test_client.py b/tests/integration/test_client.py
@@ -13,7 +13,7 @@ async def test_user_info(event_loop):
 
         um = client.UserManagerFacade.from_connection(controller_conn)
         result = await um.UserInfo(
-            [client.Entity('user-admin')], True)
+            entities=[client.Entity('user-admin')], include_disabled=True)
         await controller_conn.close()
 
         assert isinstance(result, client.UserInfoResults)
diff --git a/tests/integration/test_connection.py b/tests/integration/test_connection.py
@@ -66,7 +66,7 @@ async def test_full_status(event_loop):
 
         c = client.ClientFacade.from_connection(model.connection())
 
-        await c.FullStatus(None)
+        await c.FullStatus(patterns=None)
 
 
 @base.bootstrapped
diff --git a/tests/integration/test_errors.py b/tests/integration/test_errors.py
@@ -18,7 +18,7 @@ async def test_juju_api_error(event_loop):
 
     async with base.CleanModel() as model:
         with pytest.raises(JujuAPIError):
-            await model.add_machine(constraints={'mem': 'foo'})
+            await model.add_machine(constraints={'mem': -50})
 
 
 @base.bootstrapped
@@ -65,4 +65,4 @@ async def test_juju_error_in_result(event_loop):
         app_facade = client.ApplicationFacade.from_connection(model.connection())
 
         with pytest.raises(JujuError):
-            return await app_facade.GetCharmURL('foo')
+            return await app_facade.GetCharmURL(application='foo')
diff --git a/tests/unit/test_definitions.py b/tests/unit/test_definitions.py
@@ -4,69 +4,93 @@
 
 
 class TestDefinitions(unittest.TestCase):
+
+    def test_dict_legacy(self):
+        status = client.FullStatus.from_json({
+            'relations': [{
+                'endpoints': [{
+                    'application': 'application',
+                    'name': 'name',
+                    'role': 'role',
+                    'subordinate': True
+                }],
+                'id': 1,
+                'interface': 'interface',
+                'key': 'key',
+                'scope': 'scope',
+                'status': []
+            }]
+        })
+        if status.relations != status['relations']:
+            raise Exception('subscript not equal to attr')
+        if status.relations != status.get('relations'):
+            raise Exception('get() not equal to attr')
+        if status.get('non-existing', 'foo') != 'foo':
+            raise Exception('get defaulting missing attr')
+
     def test_parse(self):
         status = client.FullStatus.from_json({
-            "relations": [{
-                "endpoints": [{
-                    "application": "application",
-                    "name": "name",
-                    "role": "role",
-                    "subordinate": True
+            'relations': [{
+                'endpoints': [{
+                    'application': 'application',
+                    'name': 'name',
+                    'role': 'role',
+                    'subordinate': True
                 }],
-                "id": 1,
-                "interface": "interface",
-                "key": "key",
-                "scope": "scope",
-                "status": []
+                'id': 1,
+                'interface': 'interface',
+                'key': 'key',
+                'scope': 'scope',
+                'status': []
             }],
-            "applications": {
-                "app": {
-                    "can-upgrade-to": "something",
-                    "charm": "charm",
-                    "charm-profile": "profile",
-                    "charm-version": "2",
-                    "endpoint-bindings": None,
-                    "err": None,
-                    "exposed": True,
-                    "int": 0,
-                    "life": "life",
-                    "meter-statuses": {},
-                    "provider-id": "provider-id",
-                    "public-address": "1.1.1.1",
-                    "relations": {
-                        "a": ["b", "c"],
+            'applications': {
+                'app': {
+                    'can-upgrade-to': 'something',
+                    'charm': 'charm',
+                    'charm-profile': 'profile',
+                    'charm-version': '2',
+                    'endpoint-bindings': None,
+                    'err': None,
+                    'exposed': True,
+                    'int': 0,
+                    'life': 'life',
+                    'meter-statuses': {},
+                    'provider-id': 'provider-id',
+                    'public-address': '1.1.1.1',
+                    'relations': {
+                        'a': ['b', 'c'],
                     },
-                    "series": "focal",
-                    "status": {},
-                    "subordinate-to": ["other"],
-                    "units": {
-                        "unit-id": {
-                            "address": "1.1.1.1",
-                            "agent-status": {},
-                            "charm": "charm",
-                            "leader": True,
-                            "machine": "machine-0",
-                            "opened-ports": ["1234"],
-                            "provider-id": "provider",
-                            "public-address": "1.1.1.2",
-                            "subordinates": {},
-                            "workload-status": {},
-                            "workload-version": "1.2"
+                    'series': 'focal',
+                    'status': {},
+                    'subordinate-to': ['other'],
+                    'units': {
+                        'unit-id': {
+                            'address': '1.1.1.1',
+                            'agent-status': {},
+                            'charm': 'charm',
+                            'leader': True,
+                            'machine': 'machine-0',
+                            'opened-ports': ['1234'],
+                            'provider-id': 'provider',
+                            'public-address': '1.1.1.2',
+                            'subordinates': {},
+                            'workload-status': {},
+                            'workload-version': '1.2'
                         }
                     },
-                    "workload-version": "1.2"
+                    'workload-version': '1.2'
                 }
             }
         })
         if status.relations != status['relations']:
-            raise Exception("subscript not equal to attr")
+            raise Exception('subscript not equal to attr')
         if status['relations'][0]['endpoints'][0]['application'] != 'application':
-            raise Exception("failed to use complex subscript")
+            raise Exception('failed to use complex subscript')
         if status.applications['app'].relations['a'][0] != 'b':
-            raise Exception("object with array type is invalid")
+            raise Exception('object with array type is invalid')
         if not isinstance(status.relations[0], client.RelationStatus):
-            raise Exception("status relation is not a RelationStatus")
+            raise Exception('status relation is not a RelationStatus')
         if not isinstance(status.relations[0].endpoints[0], client.EndpointStatus):
-            raise Exception("status relation endpoint is not a EndpointStatus")
+            raise Exception('status relation endpoint is not a EndpointStatus')
         if not isinstance(status.applications['app'], client.ApplicationStatus):
-            raise Exception("status application is not a ApplicationStatus")
+            raise Exception('status application is not a ApplicationStatus')
