Fill in test coverage for nextflowtower

pyproject.toml

@@ -23,6 +23,7 @@ disallow_untyped_calls = true
 disallow_untyped_defs = true
 disallow_incomplete_defs = true
 check_untyped_defs = true
+implicit_reexport = true
 
 [tool.interrogate]
 verbose = true

src/orca/services/nextflowtower/__init__.py

@@ -1,11 +1,13 @@
 """Submodule for NextflowTower platforms (like Tower.nf)."""
 
+from orca.services.nextflowtower.client import NextflowTowerClient
 from orca.services.nextflowtower.client_factory import NextflowTowerClientFactory
 from orca.services.nextflowtower.config import NextflowTowerConfig
 from orca.services.nextflowtower.hook import NextflowTowerHook
 from orca.services.nextflowtower.ops import NextflowTowerOps
 
 __all__ = [
+    "NextflowTowerClient",
     "NextflowTowerConfig",
     "NextflowTowerClientFactory",
     "NextflowTowerOps",

src/orca/services/nextflowtower/client.py

@@ -19,8 +19,9 @@ class NextflowTowerClient:
     auth_token: str
     api_endpoint: str
 
+    @staticmethod
     def update_kwarg(
-        self, kwargs: dict[str, Any], key1: str, key2: str, default: Any
+        kwargs: dict[str, Any], key1: str, key2: str, default: Any
     ) -> None:
         """Ensure a default value for a nested key in kwargs.
 
@@ -62,11 +63,6 @@ def request(self, method: str, path: str, **kwargs) -> requests.Response:
         Returns:
             The raw Response object to allow for special handling
         """
-        valid_methods = {"GET", "PUT", "POST", "DELETE"}
-        if method not in valid_methods:
-            message = f"Method ({method}) not among valid options ({valid_methods})."
-            raise ValueError(message)
-
         url = f"{self.api_endpoint}/{path}"
 
         auth_header = f"Bearer {self.auth_token}"
@@ -89,43 +85,43 @@ def request_json(self, method: str, path: str, **kwargs) -> dict[str, Any]:
         response.raise_for_status()
         return response.json()
 
-    def request_paged(self, method: str, path: str, **kwargs) -> list[dict[str, Any]]:
-        """Iterate through pages of results for a given request.
+    # def request_paged(self, method: str, path: str, **kwargs) -> list[dict[str, Any]]:
+    #     """Iterate through pages of results for a given request.
 
-        See ``TowerClient.request`` for argument definitions.
+    #     See ``TowerClient.request`` for argument definitions.
 
-        Raises:
-            HTTPError: If the response doesn't match the expectation
-                for a paged endpoint.
+    #     Raises:
+    #         HTTPError: If the response doesn't match the expectation
+    #             for a paged endpoint.
 
-        Returns:
-            The cumulative list of items from all pages.
-        """
-        self.update_kwarg(kwargs, "params", "max", 50)
-        self.update_kwarg(kwargs, "params", "offset", 0)
+    #     Returns:
+    #         The cumulative list of items from all pages.
+    #     """
+    #     self.update_kwarg(kwargs, "params", "max", 50)
+    #     self.update_kwarg(kwargs, "params", "offset", 0)
 
-        num_items = 0
-        total_size = 1  # Artificial value for initiating the while-loop
+    #     num_items = 0
+    #     total_size = 1  # Artificial value for initiating the while-loop
 
-        all_items = list()
-        while num_items < total_size:
-            kwargs["params"]["offset"] = num_items
-            json = self.request_json(method, path, **kwargs)
+    #     all_items = list()
+    #     while num_items < total_size:
+    #         kwargs["params"]["offset"] = num_items
+    #         json = self.request_json(method, path, **kwargs)
 
-            if "totalSize" not in json:
-                message = f"'totalSize' not in response JSON ({json}) as expected."
-                raise HTTPError(message)
-            total_size = json.pop("totalSize")
+    #         if "totalSize" not in json:
+    #             message = f"'totalSize' not in response JSON ({json}) as expected."
+    #             raise HTTPError(message)
+    #         total_size = json.pop("totalSize")
 
-            if len(json) != 1:
-                message = f"Expected one other key aside from 'totalSize' ({json})."
-                raise HTTPError(message)
-            _, items = json.popitem()
+    #         if len(json) != 1:
+    #             message = f"Expected one other key aside from 'totalSize' ({json})."
+    #             raise HTTPError(message)
+    #         _, items = json.popitem()
 
-            num_items += len(items)
-            all_items.extend(items)
+    #         num_items += len(items)
+    #         all_items.extend(items)
 
-        return all_items
+    #     return all_items
 
     def get(self, path: str, **kwargs) -> dict[str, Any]:
         """Send an auth'ed GET request and parse the JSON response.

tests/services/nextflowtower/conftest.py

@@ -1,58 +1,23 @@
 import pytest
-from airflow.exceptions import AirflowNotFoundException
-from airflow.models.connection import Connection
 
-from orca.services.base.hook import BaseHook
 from orca.services.nextflowtower import (
+    NextflowTowerClient,
     NextflowTowerConfig,
-    NextflowTowerHook,
     NextflowTowerOps,
 )
 
 
 @pytest.fixture
-def mock_client(mocker):
-    mocker.patch("orca.services.nextflowtower.client_factory.NextflowTowerClient")
+def client(patch_os_environ):
+    yield NextflowTowerClient("foo", "bar")
 
 
 @pytest.fixture
-def config():
-    config = NextflowTowerConfig(
-        api_endpoint="https://api.tower.nf/",
-        auth_token="foo",
-        workspace=123456789,
-    )
-    yield config
+def config(patch_os_environ):
+    # Workspace name based on example in responses.py
+    yield NextflowTowerConfig("foo", "bar", "Foo-Bar/project-2")
 
 
 @pytest.fixture
-def mock_ops(config, mock_client):
+def ops(config):
     yield NextflowTowerOps(config)
-
-
-@pytest.fixture
-def connection_uri(config):
-    bare_url = config.api_endpoint.replace("https://", "")
-    host, schema = bare_url.rstrip("/").rsplit("/", maxsplit=1)
-    token = config.auth_token
-    project = config.project
-    yield f"sbg://:{token}@{host}/{schema}/?project={project}"
-
-
-@pytest.fixture
-def connection(connection_uri):
-    yield Connection(uri=connection_uri)
-
-
-@pytest.fixture
-def patch_get_connection(mocker, connection):
-    connection_mock = mocker.patch.object(BaseHook, "get_connection")
-    connection_mock.side_effect = AirflowNotFoundException
-    yield connection_mock
-
-
-@pytest.fixture
-def hook(patch_get_connection):
-    # The conn_id param doesn't matter because the `patch_get_connection`
-    # fixture will ensure that the same Connection object is returned
-    yield NextflowTowerHook("foo")

tests/services/nextflowtower/responses.py

@@ -11,12 +11,12 @@
 
 get_user_info = {
     "user": {
-        "id": 2,
-        "userName": "bgrande",
-        "email": "[email protected]",
+        "id": 100,
+        "userName": "foo",
+        "email": "[email protected]",
         "firstName": None,
         "lastName": None,
-        "organization": "@Sage-Bionetworks",
+        "organization": "@Foo-Bar",
         "description": None,
         "avatar": "REDACTED",
         "trusted": True,
@@ -36,3 +36,30 @@
     "needConsent": False,
     "defaultWorkspaceId": None,
 }
+
+
+get_user_workspaces_and_orgs = {
+    "orgsAndWorkspaces": [
+        {
+            "orgId": 123456789,
+            "orgName": "Foo-Bar",
+            "orgLogoUrl": None,
+            "workspaceId": None,
+            "workspaceName": None,
+        },
+        {
+            "orgId": 123456789,
+            "orgName": "Foo-Bar",
+            "orgLogoUrl": None,
+            "workspaceId": 54321,
+            "workspaceName": "project-1",
+        },
+        {
+            "orgId": 123456789,
+            "orgName": "Foo-Bar",
+            "orgLogoUrl": None,
+            "workspaceId": 98765,
+            "workspaceName": "project-2",
+        },
+    ]
+}

tests/services/nextflowtower/test_client.py

@@ -0,0 +1,64 @@
+import pytest
+from requests.exceptions import HTTPError
+
+from . import responses
+
+
+def test_that_update_kwargs_updates_an_empty_dictionary(client):
+    kwargs = {}
+    client.update_kwarg(kwargs, "foo", "bar", 123)
+    assert "foo" in kwargs
+    assert "bar" in kwargs["foo"]
+    assert kwargs["foo"]["bar"] == 123
+
+
+def test_that_update_kwargs_updates_an_nonempty_dictionary(client):
+    kwargs = {"foo": {"baz": 456}}
+    client.update_kwarg(kwargs, "foo", "bar", 123)
+    assert "bar" in kwargs["foo"]
+    assert "baz" in kwargs["foo"]
+    assert kwargs["foo"]["bar"] == 123
+
+
+def test_that_update_kwargs_fails_with_a_nondictionary_under_first_key(client):
+    kwargs = {"foo": 123}
+    with pytest.raises(ValueError):
+        client.update_kwarg(kwargs, "foo", "bar", 123)
+
+
+def test_that_update_kwargs_fails_with_an_incompatible_type(client):
+    kwargs = {"foo": {"bar": None}}
+    with pytest.raises(ValueError):
+        client.update_kwarg(kwargs, "foo", "bar", 123)
+
+
+def test_that_get_user_info_works(client, mocker):
+    mock = mocker.patch.object(client, "request_json")
+    mock.return_value = responses.get_user_info
+    response = client.get_user_info()
+    mock.assert_called_once()
+    assert response == responses.get_user_info["user"]
+
+
+def test_that_get_user_info_fails_with_nonstandard_response(client, mocker):
+    mock = mocker.patch.object(client, "request_json")
+    mock.return_value = {"message": "foobar"}
+    with pytest.raises(HTTPError):
+        client.get_user_info()
+
+
+def test_that_list_user_workspaces_works(client, mocker):
+    mocker.patch.object(client, "get_user_info")
+    mock = mocker.patch.object(client, "request_json")
+    mock.return_value = responses.get_user_workspaces_and_orgs
+    response = client.list_user_workspaces()
+    mock.assert_called_once()
+    assert len(response) == 2
+
+
+def test_that_list_user_workspaces_fails_with_nonstandard_response(client, mocker):
+    mocker.patch.object(client, "get_user_info")
+    mock = mocker.patch.object(client, "request_json")
+    mock.return_value = {"message": "foobar"}
+    with pytest.raises(HTTPError):
+        client.list_user_workspaces()

tests/services/nextflowtower/test_config.py

@@ -0,0 +1,22 @@
+import pytest
+from airflow.models.connection import Connection
+
+from orca.services.nextflowtower import NextflowTowerConfig
+
+
+def test_for_validation_error_when_setting_workspace_without_org():
+    bad_input = "some-workspace"
+    with pytest.raises(ValueError):
+        NextflowTowerConfig("foo", "bar", bad_input)
+
+
+def test_that_config_can_be_configured_with_full_org_workspace_name(patch_os_environ):
+    good_input = "some-org/some-workspace"
+    assert NextflowTowerConfig("foo", "bar", good_input)
+
+
+def test_that_parse_connection_can_process_connection_without_host():
+    connection = Connection(password="foo")
+    kwargs = NextflowTowerConfig.parse_connection(connection)
+    assert kwargs["auth_token"] == "foo"
+    assert kwargs["api_endpoint"] is None

tests/services/nextflowtower/test_integration.py

@@ -0,0 +1,12 @@
+import pytest
+
+from orca.services.nextflowtower import NextflowTowerClient, NextflowTowerConfig
+
+
+@pytest.mark.integration
+def test_that_a_valid_client_can_be_constructed_and_tested():
+    config = NextflowTowerConfig()
+    assert config.auth_token
+    assert config.api_endpoint
+    client = NextflowTowerClient(config.auth_token, config.api_endpoint)
+    assert client.list_user_workspaces()

tests/services/nextflowtower/test_ops.py

@@ -0,0 +1,34 @@
+import pytest
+
+from orca.errors import ConfigError
+from orca.services.nextflowtower import NextflowTowerConfig, NextflowTowerOps
+
+from . import responses
+
+
+def test_that_the_workspace_attribute_is_accessible(ops):
+    assert ops.workspace == "Foo-Bar/project-2"
+
+
+def test_for_an_error_when_the_workspace_attribute_is_missing(patch_os_environ):
+    config = NextflowTowerConfig()
+    ops = NextflowTowerOps(config)
+    with pytest.raises(ConfigError):
+        ops.workspace
+
+
+def test_that_the_workspace_id_attribute_is_accessible(ops, mocker):
+    mock = mocker.patch.object(NextflowTowerOps, "client")
+    response = responses.get_user_workspaces_and_orgs["orgsAndWorkspaces"]
+    mock.list_user_workspaces.return_value = response
+    assert ops.workspace_id == 98765
+
+
+def test_for_error_when_the_workspace_id_does_not_exist(ops, mocker):
+    mock = mocker.patch.object(NextflowTowerOps, "client")
+    response = responses.get_user_workspaces_and_orgs["orgsAndWorkspaces"]
+    # Get rid of relevant entry
+    response = [item for item in response if item["workspaceName"] != "project-2"]
+    mock.list_user_workspaces.return_value = response
+    with pytest.raises(ValueError):
+        ops.workspace_id