Add unit test coverage and CI coverage reporting

.azdo/ci-pr.yaml

@@ -1,3 +1,3 @@
 # Azure DevOps PR validation pipeline
 # Matches GitHub Actions workflow for consistency
 
@@ -42,7 +42,7 @@
 
         - script: |
             python -m pip install --upgrade pip
-            python -m pip install flake8 black build
+            python -m pip install flake8 black build diff-cover
             python -m pip install -e .[dev]
           displayName: 'Install dependencies'
 
@@ -60,18 +60,30 @@
         - script: |
             python -m build
           displayName: 'Build package'
-        
+
         - script: |
             python -m pip install dist/*.whl
           displayName: 'Install wheel'
-        
+
         - script: |
-            pytest
+            PYTHONPATH=src pytest --junitxml=test-results.xml --cov --cov-report=xml
           displayName: 'Test with pytest'
-
+
+        - script: |
+            git fetch origin main
+            diff-cover coverage.xml --compare-branch=origin/main --fail-under=90
+          displayName: 'Diff coverage (90% for new changes)'
+
         - task: PublishTestResults@2
           condition: succeededOrFailed()
           inputs:
-            testResultsFiles: '**/test-*.xml'
+            testResultsFiles: '**/test-results.xml'
             testRunTitle: 'Python 3.12'
           displayName: 'Publish test results'
+
+        - task: PublishCodeCoverageResults@2
+          condition: succeededOrFailed()
+          inputs:
+            summaryFileLocation: '**/coverage.xml'
+            pathToSources: '$(Build.SourcesDirectory)/src'
+          displayName: 'Publish code coverage'

.github/workflows/python-package.yml

@@ -18,6 +18,8 @@ jobs:
 
     steps:
     - uses: actions/checkout@v4
+      with:
+        fetch-depth: 0
 
     - name: Set up Python 3.12
       uses: actions/setup-python@v5
@@ -27,7 +29,7 @@ jobs:
     - name: Install dependencies
       run: |
         python -m pip install --upgrade pip
-        python -m pip install flake8 black build
+        python -m pip install flake8 black build diff-cover
         python -m pip install -e .[dev]
 
     - name: Check format with black
@@ -44,11 +46,30 @@ jobs:
     - name: Build package
       run: |
         python -m build
-    
+
     - name: Install wheel
       run: |
         python -m pip install dist/*.whl
-    
+
     - name: Test with pytest
       run: |
-        pytest
+        PYTHONPATH=src pytest --junitxml=test-results.xml --cov --cov-report=xml
+
+    - name: Diff coverage (90% for new changes)
+      run: |
+        git fetch origin ${{ github.base_ref }}
+        diff-cover coverage.xml --compare-branch=origin/${{ github.base_ref }} --fail-under=90
+
+    - name: Upload test results
+      if: always()
+      uses: actions/upload-artifact@v4
+      with:
+        name: test-results
+        path: test-results.xml
+
+    - name: Upload coverage report
+      if: always()
+      uses: actions/upload-artifact@v4
+      with:
+        name: coverage-report
+        path: coverage.xml

pyproject.toml

@@ -93,6 +93,14 @@ select = [
 
 [tool.pytest.ini_options]
 testpaths = ["tests/unit"]
+
+[tool.coverage.run]
+source = ["src/PowerPlatform"]
+
+[tool.coverage.report]
+fail_under = 90
+show_missing = true
+
 markers = [
     "e2e: end-to-end tests requiring a live Dataverse environment (DATAVERSE_URL)",
 ]

tests/unit/core/test_auth.py

@@ -0,0 +1,35 @@
+# Copyright (c) Microsoft Corporation.
+# Licensed under the MIT license.
+
+import unittest
+from unittest.mock import MagicMock
+
+from azure.core.credentials import TokenCredential
+
+from PowerPlatform.Dataverse.core._auth import _AuthManager, _TokenPair
+
+
+class TestAuthManager(unittest.TestCase):
+    """Tests for _AuthManager credential validation and token acquisition."""
+
+    def test_non_token_credential_raises(self):
+        """_AuthManager raises TypeError when credential does not implement TokenCredential."""
+        with self.assertRaises(TypeError) as ctx:
+            _AuthManager("not-a-credential")
+        self.assertEqual(
+            str(ctx.exception),
+            "credential must implement azure.core.credentials.TokenCredential.",
+        )
+
+    def test_acquire_token_returns_token_pair(self):
+        """_acquire_token calls get_token and returns a _TokenPair with scope and token."""
+        mock_credential = MagicMock(spec=TokenCredential)
+        mock_credential.get_token.return_value = MagicMock(token="my-access-token")
+
+        manager = _AuthManager(mock_credential)
+        result = manager._acquire_token("https://org.crm.dynamics.com/.default")
+
+        mock_credential.get_token.assert_called_once_with("https://org.crm.dynamics.com/.default")
+        self.assertIsInstance(result, _TokenPair)
+        self.assertEqual(result.resource, "https://org.crm.dynamics.com/.default")
+        self.assertEqual(result.access_token, "my-access-token")

tests/unit/core/test_http_client.py

@@ -0,0 +1,123 @@
+# Copyright (c) Microsoft Corporation.
+# Licensed under the MIT license.
+
+import unittest
+from unittest.mock import MagicMock, patch, call
+
+import requests
+
+from PowerPlatform.Dataverse.core._http import _HttpClient
+
+
+class TestHttpClientTimeout(unittest.TestCase):
+    """Tests for automatic timeout selection in _HttpClient._request."""
+
+    def _make_response(self, status=200):
+        resp = MagicMock(spec=requests.Response)
+        resp.status_code = status
+        return resp
+
+    def test_get_uses_10s_default_timeout(self):
+        """GET requests use 10s default when no timeout is specified."""
+        client = _HttpClient(retries=1)
+        with patch("requests.request", return_value=self._make_response()) as mock_req:
+            client._request("get", "https://example.com/data")
+            _, kwargs = mock_req.call_args
+            self.assertEqual(kwargs["timeout"], 10)
+
+    def test_post_uses_120s_default_timeout(self):
+        """POST requests use 120s default when no timeout is specified."""
+        client = _HttpClient(retries=1)
+        with patch("requests.request", return_value=self._make_response()) as mock_req:
+            client._request("post", "https://example.com/data")
+            _, kwargs = mock_req.call_args
+            self.assertEqual(kwargs["timeout"], 120)
+
+    def test_delete_uses_120s_default_timeout(self):
+        """DELETE requests use 120s default when no timeout is specified."""
+        client = _HttpClient(retries=1)
+        with patch("requests.request", return_value=self._make_response()) as mock_req:
+            client._request("delete", "https://example.com/data")
+            _, kwargs = mock_req.call_args
+            self.assertEqual(kwargs["timeout"], 120)
+
+    def test_default_timeout_overrides_per_method_default(self):
+        """Explicit default_timeout on the client overrides per-method defaults."""
+        client = _HttpClient(retries=1, timeout=30.0)
+        with patch("requests.request", return_value=self._make_response()) as mock_req:
+            client._request("get", "https://example.com/data")
+            _, kwargs = mock_req.call_args
+            self.assertEqual(kwargs["timeout"], 30.0)
+
+    def test_explicit_timeout_kwarg_takes_precedence(self):
+        """If timeout is already in kwargs it is passed through unchanged."""
+        client = _HttpClient(retries=1, timeout=30.0)
+        with patch("requests.request", return_value=self._make_response()) as mock_req:
+            client._request("get", "https://example.com/data", timeout=5)
+            _, kwargs = mock_req.call_args
+            self.assertEqual(kwargs["timeout"], 5)
+
+
+class TestHttpClientRequester(unittest.TestCase):
+    """Tests for session vs direct requests.request routing."""
+
+    def _make_response(self):
+        resp = MagicMock(spec=requests.Response)
+        resp.status_code = 200
+        return resp
+
+    def test_uses_direct_request_without_session(self):
+        """Without a session, _request uses requests.request directly."""
+        client = _HttpClient(retries=1)
+        with patch("requests.request", return_value=self._make_response()) as mock_req:
+            client._request("get", "https://example.com/data")
+            mock_req.assert_called_once()
+
+    def test_uses_session_request_when_session_provided(self):
+        """With a session, _request uses session.request instead of requests.request."""
+        mock_session = MagicMock(spec=requests.Session)
+        mock_session.request.return_value = self._make_response()
+        client = _HttpClient(retries=1, session=mock_session)
+        with patch("requests.request") as mock_req:
+            client._request("get", "https://example.com/data")
+            mock_session.request.assert_called_once()
+            mock_req.assert_not_called()
+
+
+class TestHttpClientRetry(unittest.TestCase):
+    """Tests for retry behavior on RequestException."""
+
+    def test_retries_on_request_exception_and_succeeds(self):
+        """Retries after a RequestException and returns response on second attempt."""
+        resp = MagicMock(spec=requests.Response)
+        resp.status_code = 200
+        client = _HttpClient(retries=2, backoff=0)
+        with patch("requests.request", side_effect=[requests.exceptions.ConnectionError(), resp]) as mock_req:
+            with patch("time.sleep"):
+                result = client._request("get", "https://example.com/data")
+        self.assertEqual(mock_req.call_count, 2)
+        self.assertIs(result, resp)
+
+    def test_raises_after_all_retries_exhausted(self):
+        """Raises RequestException after all retry attempts fail."""
+        client = _HttpClient(retries=3, backoff=0)
+        with patch("requests.request", side_effect=requests.exceptions.ConnectionError("timeout")):
+            with patch("time.sleep"):
+                with self.assertRaises(requests.exceptions.RequestException):
+                    client._request("get", "https://example.com/data")
+
+    def test_backoff_delay_between_retries(self):
+        """Sleeps with exponential backoff between retry attempts."""
+        resp = MagicMock(spec=requests.Response)
+        resp.status_code = 200
+        client = _HttpClient(retries=3, backoff=1.0)
+        side_effects = [
+            requests.exceptions.ConnectionError(),
+            requests.exceptions.ConnectionError(),
+            resp,
+        ]
+        with patch("requests.request", side_effect=side_effects):
+            with patch("time.sleep") as mock_sleep:
+                client._request("get", "https://example.com/data")
+        # First retry: delay = 1.0 * 2^0 = 1.0, second retry: 1.0 * 2^1 = 2.0
+        mock_sleep.assert_has_calls([call(1.0), call(2.0)])

tests/unit/core/test_http_errors.py

@@ -180,3 +180,40 @@ def test_correlation_id_shared_inside_call_scope():
     h1, h2 = recorder.recorded_headers
     assert h1["x-ms-client-request-id"] != h2["x-ms-client-request-id"]
     assert h1["x-ms-correlation-id"] == h2["x-ms-correlation-id"]
+
+
+def test_validation_error_instantiates():
+    """ValidationError can be raised and carries the correct code."""
+    from PowerPlatform.Dataverse.core.errors import ValidationError
+
+    err = ValidationError("bad input", subcode="missing_field", details={"field": "name"})
+    assert err.code == "validation_error"
+    assert err.subcode == "missing_field"
+    assert err.details["field"] == "name"
+    assert err.source == "client"
+
+
+def test_sql_parse_error_instantiates():
+    """SQLParseError can be raised and carries the correct code."""
+    from PowerPlatform.Dataverse.core.errors import SQLParseError
+
+    err = SQLParseError("unexpected token", subcode="syntax_error")
+    assert err.code == "sql_parse_error"
+    assert err.subcode == "syntax_error"
+    assert err.source == "client"
+
+
+def test_http_error_optional_diagnostic_fields():
+    """HttpError stores correlation_id, service_request_id, and traceparent in details."""
+    from PowerPlatform.Dataverse.core.errors import HttpError
+
+    err = HttpError(
+        "Server error",
+        status_code=500,
+        correlation_id="corr-123",
+        service_request_id="svc-456",
+        traceparent="00-abc-def-01",
+    )
+    assert err.details["correlation_id"] == "corr-123"
+    assert err.details["service_request_id"] == "svc-456"
+    assert err.details["traceparent"] == "00-abc-def-01"