Merge pull request #385 from planetlabs/v2-cli-tests-327

Convert CLI tests to integration tests, increase overall test coverage, slight change to API url building logistics

Author: jreiberkyle

planet/auth.py

@@ -21,15 +21,14 @@
 import httpx
 import jwt
 
-from . import constants, http, models
+from . import http, models
+from .constants import PLANET_BASE_URL, SECRET_FILE_PATH
 from .exceptions import AuthException
 
-
 LOGGER = logging.getLogger(__name__)
 
-BASE_URL = constants.PLANET_BASE_URL + 'v0/auth/'
+BASE_URL = f'{PLANET_BASE_URL}/v0/auth'
 ENV_API_KEY = 'PL_API_KEY'
-SECRET_FILE_PATH = os.path.join(os.path.expanduser('~'), '.planet.json')
 
 
 class Auth(metaclass=abc.ABCMeta):
@@ -155,11 +154,12 @@ def __init__(
     ):
         """
         Parameters:
-            base_url: Alternate authentication api base URL.
+            base_url: The base URL to use. Defaults to production
+                authentication API base url.
         """
         self._base_url = base_url or BASE_URL
-        if not self._base_url.endswith('/'):
-            self._base_url += '/'
+        if self._base_url.endswith('/'):
+            self._base_url = self._base_url[:-1]
 
     def login(
         self,
@@ -179,7 +179,7 @@ def login(
              A JSON object containing an `api_key` property with the user's
         API_KEY.
         '''
-        url = self._base_url + 'login'
+        url = f'{self._base_url}/login'
         data = {'email': email,
                 'password': password
                 }

planet/cli/auth.py

@@ -43,7 +43,7 @@ def auth(ctx, base_url):
 ))
 def init(ctx, email, password):
     '''Obtain and store authentication information'''
-    base_url = ctx.obj["BASE_URL"]
+    base_url = ctx.obj['BASE_URL']
     plauth = planet.Auth.from_login(email, password, base_url=base_url)
     plauth.write()
     click.echo('Initialized')

planet/cli/orders.py

@@ -46,8 +46,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'''
-    auth = planet.Auth.from_file()
-    ctx.obj['AUTH'] = auth
+    ctx.obj['AUTH'] = planet.Auth.from_file()
     ctx.obj['BASE_URL'] = base_url
 
 
@@ -157,8 +156,6 @@ def read_file_json(ctx, param, value):
         json_value = json.load(value)
     except json.decoder.JSONDecodeError:
         raise click.ClickException('File does not contain valid json.')
-    except click.FileError as e:
-        raise click.ClickException(e)
 
     return json_value
 

planet/clients/orders.py

@@ -20,15 +20,16 @@
 import typing
 import uuid
 
-from .. import constants, exceptions
+from .. import exceptions
+from ..constants import PLANET_BASE_URL
 from ..http import Session
 from ..models import Order, Orders, Request, Response, StreamingBody
 
 
-BASE_URL = constants.PLANET_BASE_URL + 'compute/ops/'
-STATS_PATH = 'stats/orders/v2/'
-ORDERS_PATH = 'orders/v2/'
-BULK_PATH = 'bulk/orders/v2/'
+BASE_URL = f'{PLANET_BASE_URL}compute/ops'
+STATS_PATH = '/stats/orders/v2'
+ORDERS_PATH = '/orders/v2'
+BULK_PATH = '/bulk/orders/v2'
 
 # Order states https://developers.planet.com/docs/orders/ordering/#order-states
 ORDERS_STATES_COMPLETE = ['success', 'partial', 'cancelled', 'failed']
@@ -52,16 +53,13 @@ class OrdersClient():
         >>> from planet import Session, OrdersClient
         >>>
         >>> async def main():
-        ...     auth = ('example_api_key', '')
-        ...     async with Session(auth=auth) as sess:
+        ...     async with Session() as sess:
         ...         cl = OrdersClient(sess)
         ...         # use client here
         ...
         >>> asyncio.run(main())
 
         ```
-
-
     """
     def __init__(
         self,
@@ -77,8 +75,8 @@ def __init__(
         self._session = session
 
         self._base_url = base_url or BASE_URL
-        if not self._base_url.endswith('/'):
-            self._base_url += '/'
+        if self._base_url.endswith('/'):
+            self._base_url = self._base_url[:-1]
 
     @staticmethod
     def _check_order_id(oid):
@@ -92,17 +90,10 @@ def _check_order_id(oid):
             raise OrdersClientException(msg)
 
     def _orders_url(self):
-        return self._base_url + ORDERS_PATH
+        return f'{self._base_url}{ORDERS_PATH}'
 
     def _stats_url(self):
-        return self._base_url + STATS_PATH
-
-    def _order_url(self, order_id):
-        self._check_order_id(order_id)
-        return self._orders_url() + order_id
-
-    def _bulk_url(self):
-        return self._base_url + BULK_PATH
+        return f'{self._base_url}{STATS_PATH}'
 
     def _request(self, url, method, data=None, params=None, json=None):
         return Request(url, method=method, data=data, params=params, json=json)
@@ -190,7 +181,8 @@ async def get_order(
             OrdersClientException: If order_id is not valid UUID.
             planet.exceptions.APIException: On API error.
         '''
-        url = self._order_url(order_id)
+        self._check_order_id(order_id)
+        url = f'{self._orders_url()}/{order_id}'
 
         req = self._request(url, method='GET')
 
@@ -224,7 +216,9 @@ async def cancel_order(
             OrdersClientException: If order_id is not valid UUID.
             planet.exceptions.APIException: On API error.
         '''
-        url = self._order_url(order_id)
+        self._check_order_id(order_id)
+        url = f'{self._orders_url()}/{order_id}'
+
         req = self._request(url, method='PUT')
 
         try:
@@ -252,7 +246,7 @@ async def cancel_orders(
         Raises:
             planet.exceptions.APIException: On API error.
         '''
-        url = self._bulk_url() + 'cancel'
+        url = f'{self._base_url}{BULK_PATH}/cancel'
         cancel_body = {}
         if order_ids:
             for oid in order_ids:

planet/constants.py

@@ -12,5 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 '''Constants used across the code base'''
+import os
 
-PLANET_BASE_URL = 'https://api.planet.com/'
+PLANET_BASE_URL = 'https://api.planet.com'
+
+SECRET_FILE_PATH = os.path.join(os.path.expanduser('~'), '.planet.json')

setup.cfg

@@ -10,7 +10,7 @@ addopts =
     --cov=tests
     --cov-report=term-missing
     --cov-report=xml
-    --cov-fail-under 95
+    --cov-fail-under 97
     -rxXs
 
 [coverage:run]

tests/conftest.py

@@ -19,6 +19,7 @@
 
 from planet.auth import _SecretFile
 
+
 _here = Path(os.path.abspath(os.path.dirname(__file__)))
 _test_data_path = _here / 'data'
 
@@ -29,6 +30,8 @@ def test_secretfile_read():
     def mockreturn(self):
         return {'key': 'testkey'}
 
+    # monkeypatch fixture is not available above a function scope
+    # usage: https://docs.pytest.org/en/6.2.x/reference.html#pytest.MonkeyPatch
     with pytest.MonkeyPatch.context() as mp:
         mp.setattr(_SecretFile, 'read', mockreturn)
         yield

tests/integration/conftest.py

@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import contextlib
+import copy
 
 import pytest
 
@@ -33,3 +34,14 @@ def cm(ex, msg):
         with pytest.raises(ex, match=f'^{msg}$') as pt:
             yield pt
     return cm
+
+
+@pytest.fixture
+def order_descriptions(order_description):
+    order1 = order_description
+    order1['id'] = 'oid1'
+    order2 = copy.deepcopy(order_description)
+    order2['id'] = 'oid2'
+    order3 = copy.deepcopy(order_description)
+    order3['id'] = 'oid3'
+    return [order1, order2, order3]

tests/integration/test_auth_api.py

@@ -23,19 +23,18 @@
 from planet.auth import AuthClient
 
 
-TEST_URL = 'http://MockNotRealURL/'
+TEST_URL = 'http://MockNotRealURL/api/path'
+TEST_LOGIN_URL = f'{TEST_URL}/login'
 
 LOGGER = logging.getLogger(__name__)
 
 
 @respx.mock
 def test_AuthClient_success():
-    login_url = TEST_URL + 'login'
-
     payload = {'api_key': 'iamakey'}
     resp = {'token': jwt.encode(payload, 'key')}
     mock_resp = httpx.Response(HTTPStatus.OK, json=resp)
-    respx.post(login_url).return_value = mock_resp
+    respx.post(TEST_LOGIN_URL).return_value = mock_resp
 
     cl = AuthClient(base_url=TEST_URL)
     auth_data = cl.login('email', 'password')
@@ -45,8 +44,6 @@ def test_AuthClient_success():
 
 @respx.mock
 def test_AuthClient_invalid_email():
-    login_url = TEST_URL + 'login'
-
     resp = {
         "errors": {
             "email": [
@@ -58,7 +55,7 @@ def test_AuthClient_invalid_email():
         "success": False
     }
     mock_resp = httpx.Response(400, json=resp)
-    respx.post(login_url).return_value = mock_resp
+    respx.post(TEST_LOGIN_URL).return_value = mock_resp
 
     cl = AuthClient(base_url=TEST_URL)
     with pytest.raises(exceptions.APIException,
@@ -68,16 +65,14 @@ def test_AuthClient_invalid_email():
 
 @respx.mock
 def test_AuthClient_invalid_password():
-    login_url = TEST_URL + 'login'
-
     resp = {
         "errors": None,
         "message": "Invalid email or password",
         "status": 401,
         "success": False
     }
     mock_resp = httpx.Response(401, json=resp)
-    respx.post(login_url).return_value = mock_resp
+    respx.post(TEST_LOGIN_URL).return_value = mock_resp
 
     cl = AuthClient(base_url=TEST_URL)
     with pytest.raises(exceptions.APIException,