Repository: planetlabs/planet-client-python

Issue:
write tests for subscriptions api & CLI cancel/get/list functions
test expected functionality for the following subscriptions functions, test will be skipped as they will initially fail.

* get_subscription
* cancel_subscription
* list_subscriptions

related to #514 
write tests for subscriptions api create/update functions
test expected functionality for the following subscriptions functions, test will be skipped as they will initially fail.

* create_subscription
* update_subscription

related to #512 

Discussion:
sgillies: @jreiberkyle @cholmes this is coming together pretty quickly. Let me know if you have questions or comments. I'd like to merge tomorrow and move on to the next steps.

Patch:
diff --git a/planet/cli/data.py b/planet/cli/data.py
index 0feb3f3..481f039 100644
--- a/planet/cli/data.py
+++ b/planet/cli/data.py
@@ -5,7 +5,6 @@ from typing import List
 from contextlib import asynccontextmanager
 
 import click
-import planet
 from planet import DataClient, Session
 
 from .cmds import coro, translate_exceptions
@@ -31,7 +30,7 @@ async def data_client(ctx):
               help='Assign custom base Orders API URL.')
 def data(ctx, base_url):
     '''Commands for interacting with the Orders API'''
-    ctx.obj['AUTH'] = planet.Auth.from_file()
+    ctx.obj['AUTH'] = None
     ctx.obj['BASE_URL'] = base_url
 
 
diff --git a/planet/cli/orders.py b/planet/cli/orders.py
index 74ac4f1..92bdc6c 100644
--- a/planet/cli/orders.py
+++ b/planet/cli/orders.py
@@ -45,7 +45,7 @@ async def orders_client(ctx):
               help='Assign custom base Orders API URL.')
 def orders(ctx, base_url):
     '''Commands for interacting with the Orders API'''
-    ctx.obj['AUTH'] = planet.Auth.from_file()
+    ctx.obj['AUTH'] = None
     ctx.obj['BASE_URL'] = base_url
 
 
diff --git a/planet/cli/subscriptions.py b/planet/cli/subscriptions.py
index 077c641..0b9fe0b 100644
--- a/planet/cli/subscriptions.py
+++ b/planet/cli/subscriptions.py
@@ -7,8 +7,7 @@ import click
 from planet import Session
 from planet.cli.cmds import coro, translate_exceptions
 from planet.cli.io import echo_json
-from planet.clients.subscriptions import (PlaceholderSubscriptionsClient as
-                                          SubscriptionsClient)
+from planet.clients.subscriptions import SubscriptionsClient
 
 
 @click.group()
diff --git a/planet/clients/subscriptions.py b/planet/clients/subscriptions.py
index 2427854..e042e57 100644
--- a/planet/clients/subscriptions.py
+++ b/planet/clients/subscriptions.py
@@ -4,19 +4,69 @@ import itertools
 from typing import AsyncIterator, Dict, Optional, Set
 import uuid
 
-from planet.exceptions import ClientError
+from planet.exceptions import APIError
 
 # Collections of fake subscriptions and results for testing. Tests will
 # monkeypatch these attributes.
-_fake_subs: Optional[Dict[str, dict]] = None
-_fake_sub_results: Optional[Dict[str, list]] = None
+_fake_subs: Dict[str, dict] = {}
+_fake_sub_results: Dict[str, list] = {}
 
 
-class PlaceholderSubscriptionsClient:
-    """A placeholder client.
+async def _server_subscriptions_post(request):
+    missing_keys = {'name', 'delivery', 'source'} - request.keys()
+    if missing_keys:
+        raise RuntimeError(f"Request lacks required members: {missing_keys!r}")
 
-    This class and its methods are derived from tests of a skeleton
-    Subscriptions CLI. It is evolving into the real API client.
+    id = str(uuid.uuid4())
+    _fake_subs[id] = request
+    sub = _fake_subs[id].copy()
+    sub.update(id=id)
+    return sub
+
+
+async def _server_subscriptions_get(status=None, limit=None):
+    select_subs = (dict(**sub, id=sub_id) for sub_id,
+                   sub in _fake_subs.items()
+                   if not status or sub['status'] in status)
+    filtered_subs = itertools.islice(select_subs, limit)
+    for sub in filtered_subs:
+        yield sub
+
+
+async def _server_subscriptions_id_cancel_post(subscription_id):
+    sub = _fake_subs.pop(subscription_id)
+    sub.update(id=subscription_id)
+    return sub
+
+
+async def _server_subscriptions_id_put(subscription_id, request):
+    _fake_subs[subscription_id].update(**request)
+    sub = _fake_subs[subscription_id].copy()
+    sub.update(id=subscription_id)
+    return sub
+
+
+async def _server_subscriptions_id_get(subscription_id):
+    sub = _fake_subs[subscription_id].copy()
+    sub.update(id=subscription_id)
+    return sub
+
+
+async def _server_subscriptions_id_results_get(subscription_id,
+                                               status=None,
+                                               limit=None):
+    select_results = (result for result in _fake_sub_results[subscription_id]
+                      if not status or result['status'] in status)
+    filtered_results = itertools.islice(select_results, limit)
+    for result in filtered_results:
+        yield result
+
+
+class SubscriptionsClient:
+    """The Planet Subscriptions API client.
+
+    TODO: make requests to the production API. Currently, the backend
+    is fake and exists at the top of this class's module.
 
     """
 
@@ -24,13 +74,14 @@ class PlaceholderSubscriptionsClient:
         self._session = session
 
     async def list_subscriptions(self,
-                                 status: Set[str] = None,
+                                 status: Optional[Set[str]] = None,
                                  limit: int = 100) -> AsyncIterator[dict]:
         """Get account subscriptions with optional filtering.
 
-        The name of this method is based on the API's method name. This
-        method provides iteration over subcriptions, it does not return
-        a list.
+        Note:
+            The name of this method is based on the API's method name. This
+            method provides iteration over subcriptions, it does not return
+            a list.
 
         Args:
             status (Set[str]): pass subscriptions with status in this
@@ -38,30 +89,26 @@ class PlaceholderSubscriptionsClient:
                 set.
             limit (int): limit the number of subscriptions in the
                 results.
+            TODO: user_id
 
         Yields:
             dict: a description of a subscription.
 
         Raises:
-            ClientError
+            APIError: on an API server error.
+            ClientError: on a client error.
 
         """
-        # Temporary marker for behavior of module with unpatched state.
-        if _fake_subs is None:
-            raise NotImplementedError
-
-        if status:
-            select_subs = (dict(**sub, id=sub_id) for sub_id,
-                           sub in _fake_subs.items()
-                           if sub['status'] in status)
-        else:
-            select_subs = (
-                dict(**sub, id=sub_id) for sub_id, sub in _fake_subs.items())
-
-        filtered_subs = itertools.islice(select_subs, limit)
-
-        for sub in filtered_subs:
-            yield sub
+        try:
+            # TODO: replace with httpx request.
+            async for sub in _server_subscriptions_get(status=status,
+                                                       limit=limit):
+                yield sub
+        except Exception as server_error:
+            # TODO: remove "from server_error" clause. It's useful
+            # during development but may invite users to depend on the
+            # value of server_error.
+            raise APIError("Subscription failure") from server_error
 
     async def create_subscription(self, request: dict) -> dict:
         """Create a Subscription.
@@ -73,23 +120,20 @@ class PlaceholderSubscriptionsClient:
             dict: description of created subscription.
 
         Raises:
-            ClientError
+            APIError: on an API server error.
+            ClientError: on a client error.
 
         """
-        # Temporary marker for behavior of module with unpatched state.
-        if _fake_subs is None:
-            raise NotImplementedError
-
-        missing_keys = {'name', 'delivery', 'source'} - request.keys()
-        if missing_keys:
-            raise ClientError(
-                f"Request lacks required members: {missing_keys!r}")
-
-        id = str(uuid.uuid4())
-        _fake_subs[id] = request
-        sub = _fake_subs[id].copy()
-        sub.update(id=id)
-        return sub
+        try:
+            # TODO: replace with httpx request.
+            sub = await _server_subscriptions_post(request)
+        except Exception as server_error:
+            # TODO: remove "from server_error" clause. It's useful
+            # during development but may invite users to depend on the
+            # value of server_error.
+            raise APIError("Subscription failure") from server_error
+        else:
+            return sub
 
     async def cancel_subscription(self, subscription_id: str) -> dict:
         """Cancel a Subscription.
@@ -101,20 +145,20 @@ class PlaceholderSubscriptionsClient:
             dict: description of cancelled subscription.
 
         Raises:
-            ClientError
+            APIError: on an API server error.
+            ClientError: on a client error.
 
         """
-        # Temporary marker for behavior of module with unpatched state.
-        if _fake_subs is None:
-            raise NotImplementedError
-
         try:
-            sub = _fake_subs.pop(subscription_id)
-        except KeyError:
-            raise ClientError(f"No such subscription: {subscription_id!r}")
-
-        sub.update(id=subscription_id)
-        return sub
+            # TODO: replace with httpx request.
+            sub = await _server_subscriptions_id_cancel_post(subscription_id)
+        except Exception as server_error:
+            # TODO: remove "from server_error" clause. It's useful
+            # during development but may invite users to depend on the
+            # value of server_error.
+            raise APIError("Subscription failure.") from server_error
+        else:
+            return sub
 
     async def update_subscription(self, subscription_id: str,
                                   request: dict) -> dict:
@@ -128,21 +172,20 @@ class PlaceholderSubscriptionsClient:
             dict: description of the updated subscription.
 
         Raises:
-            ClientError
+            APIError: on an API server error.
+            ClientError: on a client error.
 
         """
-        # Temporary marker for behavior of module with unpatched state.
-        if _fake_subs is None:
-            raise NotImplementedError
-
         try:
-            _fake_subs[subscription_id].update(**request)
-            sub = _fake_subs[subscription_id].copy()
-        except KeyError:
-            raise ClientError(f"No such subscription: {subscription_id!r}")
-
-        sub.update(id=subscription_id)
-        return sub
+            # TODO: replace with httpx request.
+            sub = await _server_subscriptions_id_put(subscription_id, request)
+        except Exception as server_error:
+            # TODO: remove "from server_error" clause. It's useful
+            # during development but may invite users to depend on the
+            # value of server_error.
+            raise APIError("Subscription failure.") from server_error
+        else:
+            return sub
 
     async def get_subscription(self, subscription_id: str) -> dict:
         """Get a description of a Subscription.
@@ -154,30 +197,31 @@ class PlaceholderSubscriptionsClient:
             dict: description of the subscription.
 
         Raises:
-            ClientError
+            APIError: on an API server error.
+            ClientError: on a client error.
 
         """
-        # Temporary marker for behavior of module with unpatched state.
-        if _fake_subs is None:
-            raise NotImplementedError
-
         try:
-            sub = _fake_subs[subscription_id].copy()
-        except KeyError:
-            raise ClientError(f"No such subscription: {subscription_id!r}")
-
-        sub.update(id=subscription_id)
-        return sub
+            # TODO: replace with httpx request.
+            sub = await _server_subscriptions_id_get(subscription_id)
+        except Exception as server_error:
+            # TODO: remove "from server_error" clause. It's useful
+            # during development but may invite users to depend on the
+            # value of server_error.
+            raise APIError("Subscription failure.") from server_error
+        else:
+            return sub
 
     async def get_results(self,
                           subscription_id: str,
-                          status: Set[str] = None,
+                          status: Optional[Set[str]] = None,
                           limit: int = 100) -> AsyncIterator[dict]:
         """Get Results of a Subscription.
 
-        The name of this method is based on the API's method name. This
-        method provides iteration over results, it does not get a
-        single result description or return a list of descriptions.
+        Note:
+            The name of this method is based on the API's method name. This
+            method provides iteration over results, it does not get a
+            single result description or return a list of descriptions.
 
         Args:
             subscription_id (str): id of a subscription.
@@ -185,30 +229,23 @@ class PlaceholderSubscriptionsClient:
                 filter out results with status not in this set.
             limit (int): limit the number of subscriptions in the
                 results.
+            TODO: created, updated, completed, user_id
 
         Yields:
             dict: description of a subscription results.
 
         Raises:
-            ClientError
+            APIError: on an API server error.
+            ClientError: on a client error.
 
         """
-        # Temporary marker for behavior of module with unpatched state.
-        if _fake_sub_results is None:
-            raise NotImplementedError
-
         try:
-            if status:
-                select_results = (
-                    result for result in _fake_sub_results[subscription_id]
-                    if result['status'] in status)
-            else:
-                select_results = (
-                    result for result in _fake_sub_results[subscription_id])
-
-            filtered_results = itertools.islice(select_results, limit)
-        except KeyError:
-            raise ClientError(f"No such subscription: {subscription_id!r}")
-
-        for result in filtered_results:
-            yield result
+            # TODO: replace with httpx request.
+            async for result in _server_subscriptions_id_results_get(
+                    subscription_id, status=status, limit=limit):
+                yield result
+        except Exception as server_error:
+            # TODO: remove "from server_error" clause. It's useful
+            # during development but may invite users to depend on the
+            # value of server_error.
+            raise APIError("Subscription failure.") from server_error


Test patch:
diff --git a/tests/integration/test_subscriptions_api.py b/tests/integration/test_subscriptions_api.py
new file mode 100644
index 0000000..fd1c321
--- /dev/null
+++ b/tests/integration/test_subscriptions_api.py
@@ -0,0 +1,176 @@
+"""Tests of the Planet Subscriptions API client."""
+
+import pytest
+
+import planet.clients.subscriptions
+from planet.clients.subscriptions import SubscriptionsClient
+from planet.exceptions import ClientError
+from planet.http import Session
+
+
+@pytest.mark.xfail(reason="Client/server interaction not yet implemented")
+@pytest.mark.asyncio
+async def test_list_subscriptions_failure(monkeypatch):
+    """ClientError is raised if there is a server error."""
+    monkeypatch.setattr(planet.clients.subscriptions, "_fake_subs", 0)
+    with pytest.raises(ClientError):
+        async with Session() as session:
+            client = SubscriptionsClient(session)
+            _ = [sub async for sub in client.list_subscriptions()]
+
+
+@pytest.mark.xfail(reason="Client/server interaction not yet implemented")
+@pytest.mark.parametrize("status, count", [({"created"}, 100), ({"failed"}, 0),
+                                           (None, 100)])
+@pytest.mark.asyncio
+async def test_list_subscriptions_success(status, count, monkeypatch):
+    """Account subscriptions iterator yields expected descriptions."""
+    monkeypatch.setattr(planet.clients.subscriptions,
+                        '_fake_subs',
+                        {str(i): {
+                            'status': 'created'
+                        }
+                         for i in range(101)})
+    async with Session() as session:
+        client = SubscriptionsClient(session)
+        assert len([
+            sub async for sub in client.list_subscriptions(status=status)
+        ]) == count
+
+
+@pytest.mark.xfail(reason="Client/server interaction not yet implemented")
+@pytest.mark.asyncio
+async def test_create_subscription_failure(monkeypatch):
+    """ClientError is raised if there is a server error."""
+    monkeypatch.setattr(planet.clients.subscriptions, "_fake_subs", {})
+    with pytest.raises(ClientError):
+        async with Session() as session:
+            client = SubscriptionsClient(session)
+            _ = await client.create_subscription({"lol": "wut"})
+
+
+@pytest.mark.xfail(reason="Client/server interaction not yet implemented")
+@pytest.mark.asyncio
+async def test_create_subscription_success(monkeypatch):
+    """Subscription is created, description has the expected items."""
+    monkeypatch.setattr(planet.clients.subscriptions, "_fake_subs", {})
+    async with Session() as session:
+        client = SubscriptionsClient(session)
+        sub = await client.create_subscription({
+            'name': 'test', 'delivery': 'yes, please', 'source': 'test'
+        })
+        assert sub['name'] == 'test'
+
+
+@pytest.mark.xfail(reason="Client/server interaction not yet implemented")
+@pytest.mark.asyncio
+async def test_cancel_subscription_failure(monkeypatch):
+    """ClientError is raised if there is a server error."""
+    monkeypatch.setattr(planet.clients.subscriptions, "_fake_subs", {})
+    with pytest.raises(ClientError):
+        async with Session() as session:
+            client = SubscriptionsClient(session)
+            _ = await client.cancel_subscription("lolwut")
+
+
+@pytest.mark.xfail(reason="Client/server interaction not yet implemented")
+@pytest.mark.asyncio
+async def test_cancel_subscription_success(monkeypatch):
+    """Subscription is canceled, description has the expected items."""
+    monkeypatch.setattr(
+        planet.clients.subscriptions,
+        "_fake_subs", {
+            "test": {
+                'name': 'test', 'delivery': 'yes, please', 'source': 'test'
+            }
+        })
+    async with Session() as session:
+        client = SubscriptionsClient(session)
+        sub = await client.cancel_subscription("test")
+        assert sub['name'] == 'test'
+
+
+@pytest.mark.xfail(reason="Client/server interaction not yet implemented")
+@pytest.mark.asyncio
+async def test_update_subscription_failure(monkeypatch):
+    """ClientError is raised if there is a server error."""
+    monkeypatch.setattr(planet.clients.subscriptions, "_fake_subs", {})
+    with pytest.raises(ClientError):
+        async with Session() as session:
+            client = SubscriptionsClient(session)
+            _ = await client.update_subscription("lolwut", {})
+
+
+@pytest.mark.xfail(reason="Client/server interaction not yet implemented")
+@pytest.mark.asyncio
+async def test_update_subscription_success(monkeypatch):
+    """Subscription is created, description has the expected items."""
+    monkeypatch.setattr(
+        planet.clients.subscriptions,
+        "_fake_subs", {
+            "test": {
+                'name': 'test', 'delivery': 'yes, please', 'source': 'test'
+            }
+        })
+    async with Session() as session:
+        client = SubscriptionsClient(session)
+        sub = await client.update_subscription(
+            "test", {
+                'name': 'test', 'delivery': "no, thanks", 'source': 'test'
+            })
+        assert sub['delivery'] == "no, thanks"
+
+
+@pytest.mark.xfail(reason="Client/server interaction not yet implemented")
+@pytest.mark.asyncio
+async def test_get_subscription_failure(monkeypatch):
+    """ClientError is raised if there is a server error."""
+    monkeypatch.setattr(planet.clients.subscriptions, "_fake_subs", {})
+    with pytest.raises(ClientError):
+        async with Session() as session:
+            client = SubscriptionsClient(session)
+            _ = await client.get_subscription("lolwut")
+
+
+@pytest.mark.xfail(reason="Client/server interaction not yet implemented")
+@pytest.mark.asyncio
+async def test_get_subscription_success(monkeypatch):
+    """Subscription description fetched, has the expected items."""
+    monkeypatch.setattr(
+        planet.clients.subscriptions,
+        "_fake_subs", {
+            "test": {
+                'name': 'test', 'delivery': 'yes, please', 'source': 'test'
+            }
+        })
+    async with Session() as session:
+        client = SubscriptionsClient(session)
+        sub = await client.get_subscription("test")
+        assert sub['delivery'] == "yes, please"
+
+
+@pytest.mark.xfail(reason="Client/server interaction not yet implemented")
+@pytest.mark.asyncio
+async def test_get_results_failure(monkeypatch):
+    """ClientError is raised if there is a server error."""
+    monkeypatch.setattr(planet.clients.subscriptions, "_fake_sub_results", {})
+    with pytest.raises(ClientError):
+        async with Session() as session:
+            client = SubscriptionsClient(session)
+            _ = [res async for res in client.get_results("lolwut")]
+
+
+@pytest.mark.xfail(reason="Client/server interaction not yet implemented")
+@pytest.mark.asyncio
+async def test_get_results_success(monkeypatch):
+    """Subscription description fetched, has the expected items."""
+    monkeypatch.setattr(
+        planet.clients.subscriptions,
+        '_fake_sub_results',
+        {'42': [{
+            'id': f'r{i}', 'status': 'created'
+        } for i in range(101)]})
+    async with Session() as session:
+        client = SubscriptionsClient(session)
+        results = [res async for res in client.get_results("42")]
+        assert len(results) == 100
diff --git a/tests/integration/test_subscriptions_cli.py b/tests/integration/test_subscriptions_cli.py
index afdea4c..59f8f0d 100644
--- a/tests/integration/test_subscriptions_cli.py
+++ b/tests/integration/test_subscriptions_cli.py
@@ -22,7 +22,7 @@ import pytest
 
 from planet.cli import cli
 import planet.cli.subscriptions
-from planet.clients.subscriptions import PlaceholderSubscriptionsClient
+from planet.clients.subscriptions import SubscriptionsClient
 
 
 @pytest.fixture
@@ -33,8 +33,8 @@ def subscription_count():
 
         async def count_em():
             return len([
-                sub async for sub in
-                PlaceholderSubscriptionsClient().list_subscriptions()
+                sub
+                async for sub in SubscriptionsClient().list_subscriptions()
             ])
 
         return asyncio.run(count_em())
@@ -98,7 +98,7 @@ def test_subscriptions_create_failure(monkeypatch, subscription_count):
         catch_exceptions=True)
 
     assert result.exit_code == 1  # failure.
-    assert "Request lacks required members" in result.output
+    assert "Subscription failure" in result.output
     assert subscription_count() == 0
 
 
@@ -174,7 +174,7 @@ def test_subscriptions_cancel_failure(monkeypatch):
         catch_exceptions=True)
 
     assert result.exit_code == 1  # failure.
-    assert "No such subscription" in result.output
+    assert "Subscription failure" in result.output
 
 
 def test_subscriptions_cancel_success(monkeypatch, subscription_count):
@@ -213,7 +213,7 @@ def test_subscriptions_update_failure(monkeypatch):
         catch_exceptions=True)
 
     assert result.exit_code == 1  # failure.
-    assert "No such subscription" in result.output
+    assert "Subscription failure" in result.output
 
 
 def test_subscriptions_update_success(monkeypatch):
@@ -251,7 +251,7 @@ def test_subscriptions_describe_failure(monkeypatch):
         catch_exceptions=True)
 
     assert result.exit_code == 1  # failure.
-    assert "No such subscription" in result.output
+    assert "Subscription failure" in result.output
 
 
 def test_subscriptions_describe_success(monkeypatch):
@@ -286,7 +286,7 @@ def test_subscriptions_results_failure(monkeypatch):
         catch_exceptions=True)
 
     assert result.exit_code == 1  # failure.
-    assert "No such subscription" in result.output
+    assert "Subscription failure." in result.output
 
 
 @pytest.mark.parametrize('options,expected_count',