test: increase unit test coverage

.github/workflows/tests.yml

@@ -33,3 +33,9 @@ jobs:
 
       - name: Run unit tests
         run: coverage run --source tes -m pytest -W ignore::DeprecationWarning
+        run: |
+          pytest \
+            --cov=tes \
+            --cov-branch \
+            --cov-report=term-missing \
+            --cov-fail-under=99

tes/client.py

@@ -120,17 +120,17 @@ def check_success(data: Task) -> bool:
             time.sleep(0.5)
 
     def _request_params(
-        self, data: Optional[str] = None,
-        params: Optional[Dict] = None
+        self, data: Optional[str] = None, params: Optional[Dict] = None
     ) -> Dict[str, Any]:
         kwargs: Dict[str, Any] = {}
         kwargs['timeout'] = self.timeout
         kwargs['headers'] = {}
         kwargs['headers']['Content-type'] = 'application/json'
-        kwargs['auth'] = (self.user, self.password)
-        if data:
+        if self.user is not None and self.password is not None:
+            kwargs['auth'] = (self.user, self.password)
+        if data is not None:
             kwargs['data'] = data
-        if params:
+        if params is not None:
             kwargs['params'] = params
         if self.token:
             kwargs['headers']['Authorization'] = f"Bearer {self.token}"

tes/models.py

@@ -10,22 +10,18 @@
 from typing import Any, Dict, List, Optional, Tuple, Type, Union
 
 
-@attrs
+@attrs(repr=False)
 class _ListOfValidator(object):
     type: Type = attrib()
 
     def __call__(self, inst, attr, value):
-        """
-        We use a callable class to be able to change the ``__repr__``.
-        """
         if not all([isinstance(n, self.type) for n in value]):
             raise TypeError(
-                "'{attr.name}' must be a list of {self.type!r} (got {value!r} "
-                "that is a list of {values[0].__class__!r}).",
-                attr, self.type, value,
+                f"'{attr.name}' must be a list of {self.type!r} (got "
+                f"{value!r}", attr
             )
 
-    def __repr__(self) -> str:
+    def __repr__(self):
         return f"<instance_of validator for type {self.type!r}>"
 
 
@@ -60,15 +56,15 @@ def strconv(value: Any) -> Any:
 # since an int64 value is encoded as a string in json we need to handle
 # conversion
 def int64conv(value: Optional[str]) -> Optional[int]:
-    if value is not None:
-        return int(value)
-    return value
+    if value is None:
+        return value
+    return int(value)
 
 
 def timestampconv(value: Optional[str]) -> Optional[datetime]:
-    if value is not None:
-        return dateutil.parser.parse(value)
-    return value
+    if value is None:
+        return value
+    return dateutil.parser.parse(value)
 
 
 def datetime_json_handler(x: Any) -> str:
@@ -294,7 +290,7 @@ def is_valid(self) -> Tuple[bool, Union[None, TypeError]]:
             for e in self.executors:
                 if e.image is None:
                     errs.append("Executor image must be provided")
-                if len(e.command) == 0:
+                if e.command is None or len(e.command) == 0:
                     errs.append("Executor command must be provided")
                 if e.stdin is not None:
                     if not os.path.isabs(e.stdin):
@@ -306,8 +302,8 @@ def is_valid(self) -> Tuple[bool, Union[None, TypeError]]:
                     if not os.path.isabs(e.stderr):
                         errs.append("Executor stderr must be an absolute path")
                 if e.env is not None:
-                    for k, v in e.env:
-                        if not isinstance(k, str) and not isinstance(k, str):
+                    for k, v in e.env.items():
+                        if not isinstance(k, str) and not isinstance(v, str):
                             errs.append(
                                 "Executor env keys and values must be StrType"
                             )
@@ -339,7 +335,7 @@ def is_valid(self) -> Tuple[bool, Union[None, TypeError]]:
                         errs.append("Volume paths must be absolute")
 
         if self.tags is not None:
-            for k, v in self.tags:
+            for k, v in self.tags.items():
                 if not isinstance(k, str) and not isinstance(k, str):
                     errs.append(
                         "Tag keys and values must be StrType"

tes/utils.py

@@ -27,21 +27,30 @@ def __init__(self, *args, **kwargs):
 
 
 def unmarshal(j: Any, o: Type, convert_camel_case=True) -> Any:
+    print(f"{j=}")
+    m: Any = None
     if isinstance(j, str):
-        m = json.loads(j)
-    elif isinstance(j, dict):
-        m = j
+        try:
+            m = json.loads(j)
+        except json.decoder.JSONDecodeError:
+            pass
     elif j is None:
         return None
     else:
-        raise TypeError("j must be a str, a dict or None")
+        m = j
+    print(f"{m=}")
+
+    if not isinstance(m, dict):
+        raise TypeError("j must be a dictionary, a JSON string evaluation to "
+                        "a dictionary, or None")
 
     d: Dict[str, Any] = {}
     if convert_camel_case:
         for k, v in m.items():
             d[camel_to_snake(k)] = v
     else:
         d = m
+    print(f"{d=}")
 
     fullOmap = {
         "Executor": {
@@ -64,6 +73,7 @@ def unmarshal(j: Any, o: Type, convert_camel_case=True) -> Any:
     }
 
     def _unmarshal(v: Any, obj: Type) -> Any:
+        print(f"v_private={v}")
         if isinstance(v, list):
             field = []
             for item in v:
@@ -74,23 +84,24 @@ def _unmarshal(v: Any, obj: Type) -> Any:
 
     r = {}
     for k, v in d.items():
+        print(f"{k=}")
+        print(f"{v=}")
         field = v
+        print(f"{field=}")
         omap = fullOmap.get(o.__name__, {})
+        print(f"{omap=}")
         if k in omap:
-            if isinstance(omap[k], tuple):
-                try:
-                    obj = omap[k][0]
-                    field = _unmarshal(v, obj)
-                except Exception:
-                    obj = omap[k][1]
-                    field = _unmarshal(v, obj)
-            else:
-                obj = omap[k]
-                field = _unmarshal(v, obj)
+            print(f"k_omap={k}")
+            obj = omap[k]
+            print(f"{obj=}")
+            field = _unmarshal(v, obj)
+            print(f"field_omap={field}")
         r[k] = field
+        print(f"{r[k]=}")
 
     try:
         output = o(**r)
+        print(f"{output=}")
     except Exception as e:
         msg = "%s could not be unmarshalled to type: %s" % (j, o.__name__) + \
               "\n" + \

tests/requirements.txt

@@ -2,4 +2,5 @@ coverage>=6.5.0
 coveralls>=3.3.1
 flake8>=5.0.4
 pytest>=7.2.1
+pytest-cov>=4.0.0
 requests_mock>=1.10.0

tests/test_client.py

@@ -24,13 +24,13 @@ class TestHTTPClient(unittest.TestCase):
     def test_cli(self):
         cli = HTTPClient(url="http://fakehost:8000//", timeout=5)
         self.assertEqual(cli.url, "http://fakehost:8000")
-        self.assertEqual(cli.timeout, 5)
+        self.assertAlmostEqual(cli.timeout, 5)
 
         with self.assertRaises(ValueError):
             HTTPClient(url="fakehost:8000", timeout=5)
 
         with self.assertRaises(ValueError):
-            HTTPClient(url="htpp://fakehost:8000", timeout="5")
+            HTTPClient(url="htpp://fakehost:8000", timeout="5")  # type: ignore
 
     def test_create_task(self):
         with requests_mock.Mocker() as m:
@@ -41,7 +41,7 @@ def test_create_task(self):
             )
             self.cli.create_task(self.task)
             self.assertEqual(m.last_request.text, self.task.as_json())
-            self.assertEqual(m.last_request.timeout, self.cli.timeout)
+            self.assertAlmostEqual(m.last_request.timeout, self.cli.timeout)
 
             m.post(
                 "%s/v1/tasks" % (self.mock_url),
@@ -50,6 +50,9 @@ def test_create_task(self):
             with self.assertRaises(requests.HTTPError):
                 self.cli.create_task(self.task)
 
+            with self.assertRaises(TypeError):
+                self.cli.create_task('not_a_task_object')  # type: ignore
+
     def test_get_task(self):
         with requests_mock.Mocker() as m:
             m.get(
@@ -61,11 +64,11 @@ def test_get_task(self):
                 }
             )
             self.cli.get_task(self.mock_id, "MINIMAL")
-            self.assertEqual(
+            self.assertAlmostEqual(
                 m.last_request.url,
                 "%s/v1/tasks/%s?view=MINIMAL" % (self.mock_url, self.mock_id)
             )
-            self.assertEqual(m.last_request.timeout, self.cli.timeout)
+            self.assertAlmostEqual(m.last_request.timeout, self.cli.timeout)
 
             m.get(
                 "%s/v1/tasks/%s" % (self.mock_url, self.mock_id),
@@ -84,11 +87,11 @@ def test_list_tasks(self):
                 }
             )
             self.cli.list_tasks()
-            self.assertEqual(
+            self.assertAlmostEqual(
                 m.last_request.url,
                 "%s/v1/tasks?view=MINIMAL" % (self.mock_url)
             )
-            self.assertEqual(m.last_request.timeout, self.cli.timeout)
+            self.assertAlmostEqual(m.last_request.timeout, self.cli.timeout)
 
             # empty response
             m.get(
@@ -97,7 +100,7 @@ def test_list_tasks(self):
                 json={}
             )
             self.cli.list_tasks()
-            self.assertEqual(
+            self.assertAlmostEqual(
                 m.last_request.url,
                 "%s/v1/tasks?view=MINIMAL" % (self.mock_url)
             )
@@ -117,11 +120,11 @@ def test_cancel_task(self):
                 json={}
             )
             self.cli.cancel_task(self.mock_id)
-            self.assertEqual(
+            self.assertAlmostEqual(
                 m.last_request.url,
                 "%s/v1/tasks/%s:cancel" % (self.mock_url, self.mock_id)
             )
-            self.assertEqual(m.last_request.timeout, self.cli.timeout)
+            self.assertAlmostEqual(m.last_request.timeout, self.cli.timeout)
 
             m.post(
                 "%s/v1/tasks/%s:cancel" % (self.mock_url, self.mock_id),
@@ -138,11 +141,11 @@ def test_get_service_info(self):
                 json={}
             )
             self.cli.get_service_info()
-            self.assertEqual(
+            self.assertAlmostEqual(
                 m.last_request.url,
                 "%s/v1/tasks/service-info" % (self.mock_url)
             )
-            self.assertEqual(m.last_request.timeout, self.cli.timeout)
+            self.assertAlmostEqual(m.last_request.timeout, self.cli.timeout)
 
             m.get(
                 "%s/v1/tasks/service-info" % (self.mock_url),
@@ -177,3 +180,25 @@ def test_wait(self):
                 ]
             )
             self.cli.wait(self.mock_id, timeout=2)
+
+    def test_request_params(self):
+
+        cli = HTTPClient(url="http://fakehost:8000", timeout=5)
+        vals = cli._request_params()
+        self.assertAlmostEqual(vals["timeout"], 5)
+        self.assertEqual(vals["headers"]["Content-type"], "application/json")
+        self.assertRaises(KeyError, lambda: vals["headers"]["Authorization"])
+        self.assertRaises(KeyError, lambda: vals["auth"])
+        self.assertRaises(KeyError, lambda: vals["data"])
+        self.assertRaises(KeyError, lambda: vals["params"])
+
+        cli = HTTPClient(url="http://fakehost:8000", user="user",
+                         password="password", token="token")
+        vals = cli._request_params(data='{"json": "string"}',
+                                   params={"query_param": "value"})
+        self.assertAlmostEqual(vals["timeout"], 10)
+        self.assertEqual(vals["headers"]["Content-type"], "application/json")
+        self.assertEqual(vals["headers"]["Authorization"], "Bearer token")
+        self.assertAlmostEqual(vals["auth"], ("user", "password"))
+        self.assertAlmostEqual(vals["data"], '{"json": "string"}')
+        self.assertAlmostEqual(vals["params"], {"query_param": "value"})