[FEATURE] Adds type validation GitHub action (#96)

- Adds type checking to github CI, updates typing & fixes some MyPy issues

.github/workflows/ci.yml

@@ -23,6 +23,23 @@ jobs:
       - name: Run Pre-commit Hooks
         run: pre-commit run --all-files
 
+  type_check:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: '3.9'
+
+      - name: Install Test Dependencies
+        run: pip install .[test]
+
+      - name: Type Checking with Mypy
+        run: mypy
+
   tests:
     runs-on: ubuntu-latest
     strategy:

ChangeLog.md

@@ -8,6 +8,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ## [unreleased] - 2024-11-01
 ### Added
 - Added pre-commit hooks and Github CI action for code formatting and linting.
+- Added MyPy with strict settings to enforce type hints (and Github CI action).
 
 ## [v0.3.11] - 2024-11-01
 ### Fixed

docs/quickstart/assessment/standalone_assessment.py

@@ -322,15 +322,15 @@
 # Set up the HTML page template, for serving to the built-in Python web server
 class LearnosityServer(BaseHTTPRequestHandler):
 
-    def createResponse(self,response):
+    def createResponse(self, response: str) -> None:
          # Send headers and data back to the client.
             self.send_response(200)
             self.send_header("Content-type", "text/html")
             self.end_headers()
             # Send the response to the client.
             self.wfile.write(response.encode("utf-8"))
 
-    def do_GET(self):
+    def do_GET(self) -> None:
 
         if self.path.endswith("/"):
 
@@ -542,7 +542,7 @@ def do_GET(self):
 
                     self.createResponse(response)
 
-def main():
+def main() -> None:
     web_server = HTTPServer((host, port), LearnosityServer)
     print("Server started http://%s:%s. Press Ctrl-c to quit." % (host, port))
     try:

learnosity_sdk/request/init.py

@@ -35,13 +35,13 @@ class Init(object):
 
     def __init__(
             self, service: str, security: Dict[str, Any], secret: str,
-            request: Optional[Dict[str, Any]] = None, action:Optional[str] = None) -> None:
-        # Using None as a default value will throw mypy typecheck issues. This should be addressed
+            request: Optional[Union[Dict[str, Any], str]] = None, action:Optional[str] = None) -> None:
         self.service = service
         self.security = security.copy()
         self.secret = secret
         self.request = request
-        if hasattr(request, 'copy'):
+        # TODO: Fix improper handling when request is a string
+        if isinstance(request, dict):
             self.request = request.copy()
         self.action = action
 
@@ -193,7 +193,7 @@ def set_service_options(self) -> None:
         elif self.service == 'assess':
             self.sign_request_data = False
 
-            if 'questionsApiActivity' in self.request:
+            if self.request is not None and 'questionsApiActivity' in self.request:
                 questionsApi = self.request['questionsApiActivity']
 
                 if 'domain' in self.security:
@@ -223,14 +223,14 @@ def set_service_options(self) -> None:
                 self.request['questionsApiActivity'].update(questionsApi)
 
         elif self.service == 'items' or self.service == 'reports':
-            if 'user_id' not in self.security and \
-                    'user_id' in self.request:
+            if self.request is not None and ('user_id' not in self.security and 'user_id' in self.request):
                 self.security['user_id'] = self.request['user_id']
 
         elif self.service == 'events':
             self.sign_request_data = False
             hashed_users = {}
-            for user in self.request.get('users', []):
+            users = self.request.get('users', []) if self.request is not None else []
+            for user in users:
                 concat = "{}{}".format(user, self.secret)
                 hashed_users[user] = hashlib.sha256(concat.encode('utf-8')).hexdigest()
 
@@ -244,7 +244,7 @@ def hash_list(self, l: Iterable[Any]) -> str:
         return '$02$' + signature
 
     def add_telemetry_data(self) -> None:
-        if self.__telemetry_enabled:
+        if self.request is not None and self.__telemetry_enabled:
             if 'meta' in self.request:
                 self.request['meta']['sdk'] = self.get_sdk_meta()
             else:

pyproject.toml

@@ -0,0 +1,3 @@
+[tool.mypy]
+strict = true
+files = ["learnosity_sdk", "docs", "tests"]

setup.py

@@ -24,6 +24,9 @@
     'pytest-cov >=2.8.1',
     'pytest-subtests',
     'responses >=0.8.1',
+    'types-requests',
+    'types-Jinja2',
+    'mypy',
 ]
 
 # Extract the markdown content of the README to be sent to Pypi as the project description page.

tests/integration/test_dataapi.py

@@ -1,3 +1,4 @@
+from typing import Any, Dict, List, cast
 import unittest
 import os
 from learnosity_sdk.request import DataApi
@@ -33,7 +34,7 @@
 class IntegrationTestDataApiClient(unittest.TestCase):
 
     @staticmethod
-    def __build_base_url():
+    def __build_base_url() -> str:
         env = os.environ
         env_domain = ''
         region_domain = '.learnosity.com'
@@ -52,7 +53,7 @@ def __build_base_url():
 
         return base_url
 
-    def test_real_request(self):
+    def test_real_request(self) -> None:
         """Make a request against Data Api to ensure the SDK works"""
         client = DataApi()
         res = client.request(self.__build_base_url() + items_endpoint, security, consumer_secret, items_request,
@@ -62,9 +63,10 @@ def test_real_request(self):
         assert len(returned_json['data']) > 0
 
         returned_ref = returned_json['data'][0]['reference']
-        assert returned_ref in items_request['references']
+        references: List[str] = cast(List[str], items_request['references'])
+        assert returned_ref in references
 
-    def test_paging(self):
+    def test_paging(self) -> None:
         """Verify that paging works"""
         client = DataApi()
         pages = client.request_iter(self.__build_base_url() + items_endpoint, security, consumer_secret,
@@ -78,14 +80,14 @@ def test_paging(self):
         assert len(results) == 2
         assert results == {'item_2', 'item_3'}
 
-    def test_real_request_with_special_characters(self):
+    def test_real_request_with_special_characters(self) -> None:
         """Make a request against Data Api to ensure the SDK works"""
         client = DataApi()
 
         # Add a reference containing special characters to ensure
         # signature creation works with special characters in the request
-        local_items_request = items_request.copy()  # prevent modifying the base fixture
-        local_items_request['references'] = items_request['references'].copy()  # prevent modifying the base fixture's list
+        local_items_request: Dict[str, Any] = items_request.copy()  # prevent modifying the base fixture
+        local_items_request['references'] = cast(List[str], items_request['references'])[:]  # prevent modifying the base fixture's list
         local_items_request['references'].append('тест')
 
         res = client.request(self.__build_base_url() + items_endpoint, security, consumer_secret, items_request,
@@ -95,9 +97,9 @@ def test_real_request_with_special_characters(self):
         assert len(returned_json['data']) > 0
 
         returned_ref = returned_json['data'][0]['reference']
-        assert returned_ref in items_request['references']
+        assert returned_ref in cast(List[str], items_request['references'])
 
-    def test_real_question_request(self):
+    def test_real_question_request(self) -> None:
         """Make a request against Data Api to ensure the SDK works"""
         client = DataApi()
 
@@ -114,7 +116,7 @@ def test_real_question_request(self):
 
         assert keys == {'py-sdk-test-2019-1', 'py-sdk-test-2019-2'}
 
-    def test_question_paging(self):
+    def test_question_paging(self) -> None:
         """Verify that paging works"""
         client = DataApi()
 

tests/unit/test_dataapi.py

@@ -1,3 +1,4 @@
+from typing import Any, Dict, cast
 import unittest
 import responses
 from learnosity_sdk.request import DataApi
@@ -8,7 +9,7 @@ class UnitTestDataApiClient(unittest.TestCase):
     Tests to ensure that the Data API client functions correctly.
     """
 
-    def setUp(self):
+    def setUp(self) -> None:
         # This test uses the consumer key and secret for the demos consumer
         # this is the only consumer with publicly available keys
         self.security = {
@@ -44,7 +45,7 @@ def setUp(self):
         self.invalid_json = "This is not valid JSON!"
 
     @responses.activate
-    def test_request(self):
+    def test_request(self) -> None:
         """
         Verify that `request` sends a request after it has been signed
         """
@@ -55,10 +56,10 @@ def test_request(self):
                              self.action)
         assert res.json() == self.dummy_responses[0]
         assert responses.calls[0].request.url == self.endpoint
-        assert 'signature' in responses.calls[0].request.body
+        assert 'signature' in cast(Dict[str, Any], responses.calls[0].request.body)
 
     @responses.activate
-    def test_request_iter(self):
+    def test_request_iter(self) -> None:
         """Verify that `request_iter` returns an iterator of pages"""
         for dummy in self.dummy_responses:
             responses.add(responses.POST, self.endpoint, json=dummy)
@@ -74,7 +75,7 @@ def test_request_iter(self):
         assert results[1]['data'][0]['id'] == 'b'
 
     @responses.activate
-    def test_results_iter(self):
+    def test_results_iter(self) -> None:
         """Verify that `result_iter` returns an iterator of results"""
         self.dummy_responses[1]['data'] = {'id': 'b'}
         for dummy in self.dummy_responses:
@@ -89,7 +90,7 @@ def test_results_iter(self):
         assert results[1]['id'] == 'b'
 
     @responses.activate
-    def test_results_iter_error_status(self):
+    def test_results_iter_error_status(self) -> None:
         """Verify that a DataApiException is raised http status is not ok"""
         for dummy in self.dummy_responses:
             responses.add(responses.POST, self.endpoint, json={}, status=500)
@@ -99,10 +100,12 @@ def test_results_iter_error_status(self):
                                      self.request, self.action))
 
     @responses.activate
-    def test_results_iter_no_meta_status(self):
+    def test_results_iter_no_meta_status(self) -> None:
         """Verify that a DataApiException is raised when 'meta' 'status' is None"""
         for response in self.dummy_responses:
-            response['meta']['status'] = None
+            # This is for typing purposes only, and should always be True
+            if isinstance(response['meta'], dict):
+                response['meta']['status'] = None
 
         for dummy in self.dummy_responses:
             responses.add(responses.POST, self.endpoint, json=dummy)
@@ -112,7 +115,7 @@ def test_results_iter_no_meta_status(self):
                                      self.request, self.action))
 
     @responses.activate
-    def test_results_iter_invalid_response_data(self):
+    def test_results_iter_invalid_response_data(self) -> None:
         """Verify that a DataApiException is raised response data isn't valid JSON"""
         for dummy in self.dummy_responses:
             responses.add(responses.POST, self.endpoint, json=None)

tests/unit/test_init.py

@@ -1,10 +1,11 @@
 import collections
+from typing import Dict, Optional
 import unittest
 
 import learnosity_sdk.request
 
 ServiceTestSpec = collections.namedtuple(
-    "TestSpec", [
+    "ServiceTestSpec", [
             "service",
             "valid",
             "security",  # security can be None to use the default, or an Dict to extend the default
@@ -111,7 +112,7 @@ class TestServiceRequests(unittest.TestCase):
     domain = 'localhost'
     timestamp = '20140626-0528'
 
-    def test_init_generate(self):
+    def test_init_generate(self) -> None:
         """
         Test that Init.generate() generates the desired initOptions
         """
@@ -125,7 +126,7 @@ def test_init_generate(self):
                 self.assertFalse(init.is_telemetry_enabled(), 'Telemetry still enabled')
                 self.assertEqual(t.signature, init.generate_signature(), 'Signature mismatch')
 
-    def test_no_parameter_mangling(self):
+    def test_no_parameter_mangling(self) -> None:
         """ Test that Init.generate() does not modify its parameters """
         learnosity_sdk.request.Init.enable_telemetry()
         for t in ServiceTests:
@@ -143,7 +144,7 @@ def test_no_parameter_mangling(self):
                 self.assertEqual(security, security_copy, 'Original security modified by SDK')
                 self.assertEqual(t.request, request_copy, 'Original request modified by SDK')
 
-    def _prepare_security(self, add_security=None):
+    def _prepare_security(self, add_security: Optional[Dict[str, str]]=None) -> Dict[str, str]:
         # TODO(cera): Much more validation
         security = {
             'consumer_key': self.key,

tests/unit/test_sdk.py

@@ -1,8 +1,9 @@
 import collections
+from typing import Any, Dict
 import unittest
 
 SdkTestSpec = collections.namedtuple(
-    "TestSpec", ["import_line", "object"])
+    "SdkTestSpec", ["import_line", "object"])
 
 SdkImportTests = [
     SdkTestSpec(
@@ -34,9 +35,9 @@
     ),
 ]
 
-def _run_test(t):
-    globals = {}
-    locals = {}
+def _run_test(t: Any) -> None:
+    globals: Dict[str, Any] = {}
+    locals: Dict[str, Any] = {}
     exec(t.import_line, globals, locals)
     eval(t.object, globals, locals)
 
@@ -45,15 +46,15 @@ class TestSdkImport(unittest.TestCase):
     Tests importing the SDK
     """
 
-    def test_sdk_imports(self):
+    def test_sdk_imports(self) -> None:
         for t in SdkImportTests:
             _run_test(t)
 
-class TestSdkImport(unittest.TestCase):
+class TestSdkModuleImport(unittest.TestCase):
     """
     Tests importing the modules
     """
 
-    def test_sdk_imports(self):
+    def test_sdk_imports(self) -> None:
         for t in SdkModuleTests:
             _run_test(t)

tests/unit/test_uuid.py

@@ -3,7 +3,7 @@
 from learnosity_sdk.utils import Uuid
 
 class TestUuid(unittest.TestCase):
-    def test_generate(self):
+    def test_generate(self) -> None:
         """
         Tests correctness of the generate() method in Uuid.
         """
@@ -12,5 +12,5 @@ def test_generate(self):
         prog = re.compile('[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}')
         result = prog.match(generated)
 
-        assert result != None
+        assert result is not None
         assert result.group() == generated