Test/missing image processing tests (#36)

* Add ImageProcessingResponse test

* Add ImageProcessing tests

* Add service_url fixture where convenient

* Add Given/When/Then comments to all tests where missing

* Add a new test to greater_or_equal validator

* Ignore MyPy protocol classes from coverage

* Update Github CI pipeline

Author: ebram96

.github/workflows/ci.yml

@@ -1,4 +1,4 @@
-name: Unit Tests
+name: Unit Tests, Test Coverage, and Pre-Commit Hooks
 
 on:
   pull_request:
@@ -30,8 +30,7 @@ jobs:
         run: coverage run -m pytest -s tests
 
       - name: Test Coverage
-        # TODO: Change this after finishing tests.
-        run: coverage report --fail-under=94
+        run: coverage report --fail-under=96
 
       - name: Run Pre-Commit Hooks
         run: pre-commit run --all-files

src/abstract_api/core/mixins/nested_entities_mixin.py

@@ -3,7 +3,7 @@
 
 class _ResponseFieldProtocol(Protocol):
     def _init_response_field(self, field: str, value: Any) -> None:
-        ...
+        ...  # pragma: no cover
 
 
 if TYPE_CHECKING:

src/abstract_api/image_processing/image_processing.py

@@ -2,7 +2,6 @@
 from typing import Any, BinaryIO, ClassVar
 
 from ..core.bases import BaseService
-from ..core.exceptions import ClientRequestError
 from ..core.validators import numerical
 from .image_processing_response import ImageProcessingResponse
 from .strategies import BaseStrategy
@@ -107,7 +106,7 @@ def _process(
     ) -> ImageProcessingResponse:
 
         if image is None and url is None:
-            raise ClientRequestError("Image or URL must be passed")
+            raise ValueError("Image or URL must be passed")
 
         numerical.between("quality", quality, 0, 100)
 

src/abstract_api/image_processing/strategies/_mixins/_json_representable_protocol.py

@@ -4,4 +4,4 @@
 class JSONRepresentableProtocol(Protocol):
     """MyPy protocol to indicate a class having .json() method."""
     def json(self) -> dict[str, int | str]:  # noqa: D102
-        ...
+        ...  # pragma: no cover

tests/avatars/test_avatars.py

@@ -19,33 +19,48 @@ def service(self) -> Avatars:
     def test__validate_params(
         self, service, image_size, font_size, char_limit, image_format, mocker
     ):
+        # Given
         mocked__validate_params = mocker.patch.object(
             service,
             "_validate_params",
             wraps=service._validate_params
         )
+
+        # Then
         with pytest.raises(ClientRequestError):
+            # When
             service.create("", image_size=image_size)
         assert mocked__validate_params.call_count == 1
+
+        # Then
         with pytest.raises(ClientRequestError):
+            # When
             service.create("", font_size=font_size)
         assert mocked__validate_params.call_count == 2
+
+        # Then
         with pytest.raises(ClientRequestError):
+            # When
             service.create("", char_limit=char_limit)
         assert mocked__validate_params.call_count == 3
+
+        # Then
         with pytest.raises(ClientRequestError):
+            # When
             service.create("", image_format=image_format)
         assert mocked__validate_params.call_count == 4
 
     def test_create(self, service, base_url, requests_mock):
+        # Given
         url = base_url.format(subdomain=Avatars._subdomain)
         content = b'some-john-doe-avatar-content'
         name = "John Doe"
-
         requests_mock.get(url, content=content)
 
+        # When
         response = service.create(name=name)
 
+        # Then
         assert isinstance(response, AvatarsResponse)
         assert response.content == content
         assert response.meta.body == content

tests/company_enrichment/test_company_enrichment.py

@@ -10,22 +10,28 @@ class TestCompanyEnrichment:
     def service(self) -> CompanyEnrichment:
         return CompanyEnrichment(api_key="no-api-key")
 
+    @pytest.fixture
+    def service_url(self, base_url, service):
+        return base_url.format(subdomain=service._subdomain)
+
     def test_check(
         self,
         service,
+        service_url,
         company_enrichment_sample,
-        base_url,
         requests_mock,
         mocker
     ):
-        url = base_url.format(subdomain=CompanyEnrichment._subdomain)
-        requests_mock.get(url, json=company_enrichment_sample)
+        # Given
+        requests_mock.get(service_url, json=company_enrichment_sample)
         mocked__service_request = mocker.patch.object(
             service, "_service_request", wraps=service._service_request
         )
 
+        # When
         response = service.check(domain=company_enrichment_sample["domain"])
 
+        # Then
         assert isinstance(response, CompanyEnrichmentResponse)
         mocked__service_request.assert_called_once_with(
             _response_class=CompanyEnrichmentResponse,
@@ -46,27 +52,29 @@ def test_check(
     def test_check_with_response_fields(
         self,
         service,
+        service_url,
         company_enrichment_sample,
-        base_url,
         requests_mock,
         mocker
     ):
-        url = base_url.format(subdomain=CompanyEnrichment._subdomain)
+        # Given
         sample_for_fields = {
             "name": "Google",
             "domain": "google.com"
         }
         selected_fields = service._prepare_selected_fields(sample_for_fields.keys())
-        requests_mock.get(url, json=sample_for_fields)
+        requests_mock.get(service_url, json=sample_for_fields)
         mocked__service_request = mocker.patch.object(
             service, "_service_request", wraps=service._service_request
         )
 
+        # When
         response = service.check(
             domain=company_enrichment_sample["domain"],
             fields=selected_fields
         )
 
+        # Then
         assert isinstance(response, CompanyEnrichmentResponse)
         assert response.name == sample_for_fields["name"]
         assert response.domain == sample_for_fields["domain"]

tests/company_enrichment/test_company_enrichment_response.py

@@ -6,10 +6,13 @@
 
 class TestCompanyEnrichmentResponse:
     def test_instance(self, company_enrichment_sample, mocker):
+        # Given
         response = generate_response(company_enrichment_sample)
 
+        # When
         instance = CompanyEnrichmentResponse(response, RESPONSE_FIELDS)
 
+        # Then
         assert_response_fields(
             instance, RESPONSE_FIELDS, company_enrichment_sample, mocker
         )

tests/core/bases/test__base_response.py

@@ -6,15 +6,23 @@
 
 class TestBaseResponseMeta:
     def test_instance(self, content_response):
+        # When
         meta = BaseResponseMeta(content_response)
+
+        # Then
         assert meta.http_status == content_response.status_code
         assert meta.body == content_response.content
 
 
 class TestBaseResponse:
     def test_instance(self, blank_response, mocker):
+        # Given
         mocker.patch.object(
             BaseResponse, "_meta_class", BaseResponseMeta, create=True
         )
+
+        # When
         base_response = BaseResponse(blank_response)
+
+        # Then
         assert isinstance(base_response.meta, BaseResponseMeta)

tests/core/bases/test_base_service.py

@@ -35,36 +35,53 @@ def child_service_url(self, child_service):
         return child_service._BaseService__service_url()
 
     def test__read_api_key_from_env(self, child_service, mocker):
+        # Given
         env_key = "ABSTRACTAPI_{service_name}_API_KEY".format(
             service_name=child_service._service_name_env_var
         )
         value = "some-api-key"
         mocker.patch.dict(os.environ, {env_key: value})
-        assert child_service._read_api_key_from_env() == value
+
+        # When
+        api_key = child_service._read_api_key_from_env()
+
+        # Then
+        assert api_key == value
 
     def test_init_without_api_key(self, mocker):
+        # Given
         mocked_read = mocker.patch.object(
             _ChildService,
             "_read_api_key_from_env",
             wraps=_ChildService._read_api_key_from_env
         )
 
+        # Then
         with pytest.raises(ValueError):
-            _ChildService()
+            _ChildService()  # When
         mocked_read.assert_called_once()
 
     def test___service_url(self, base_url, mocker):
+        # Given
         service = "test"
         action = "testing"
         mocker.patch.object(BaseService, "_subdomain", service, create=True)
-        assert BaseService._BaseService__service_url(self=BaseService) == base_url.format(subdomain=service)
-        assert BaseService._BaseService__service_url(self=BaseService, action=action) == base_url.format(subdomain=service) + action
+
+        # When
+        service_url = BaseService._BaseService__service_url(self=BaseService)
+        service_url_with_action = BaseService._BaseService__service_url(self=BaseService, action=action)
+
+        # Then
+        assert service_url == base_url.format(subdomain=service)
+        assert service_url_with_action == base_url.format(subdomain=service) + action
 
     @pytest.mark.parametrize("method", ["PUT", "PATCH", "DELETE"])
     def test__service_request_with_invalid_method(
         self, method, child_service,
     ):
+        # Then
         with pytest.raises(ClientRequestError):
+            # When
             child_service._service_request(
                 _response_class=_ChildServiceResponse,
                 _method=method
@@ -77,16 +94,19 @@ def test__service_request_get(
         requests_mock,
         mocker
     ):
+        # Given
         service_params = {"param1": "value", "param2": None}
         requests_mock.get(child_service_url, status_code=requests.codes.OK)
         mocked_request = mocker.patch.object(
             requests, "request", wraps=requests.request
         )
 
+        # When
         child_service._service_request(
             _ChildServiceResponse, **service_params
         )
 
+        # Then
         mocked_request.assert_called_once_with(
             method="GET",
             url=child_service_url,
@@ -113,6 +133,7 @@ def test__service_request_post(
         requests_mock,
         mocker
     ):
+        # Given
         requests_mock.post(child_service_url, content=b"")
         mocked_request = mocker.patch.object(
             requests, "request", wraps=requests.request
@@ -126,13 +147,17 @@ def test__service_request_post(
             request_kwargs["_files"] = files
         if body is not None:
             request_kwargs["_body"] = body
-        child_service._service_request(**request_kwargs)
 
         call_kwargs = {"method": "POST", "url": child_service_url}
         if files is not None:
             call_kwargs["files"] = files
         if body is not None:
             call_kwargs["json"] = body
+
+        # When
+        child_service._service_request(**request_kwargs)
+
+        # Then
         mocked_request.assert_called_once_with(**call_kwargs)
 
     @pytest.mark.parametrize(
@@ -145,10 +170,14 @@ def test__service_request_unaccepted_status_code(
         child_service_url,
         requests_mock
     ):
+        # Given
         requests_mock.get(
             child_service_url, status_code=status_code
         )
+
+        # Then
         with pytest.raises(APIRequestError):
+            # When
             child_service._service_request(_ChildServiceResponse)
 
     def test__service_request_parsing_error(
@@ -157,10 +186,13 @@ def test__service_request_parsing_error(
         blank_response,
         mocker
     ):
+        # Given
         mocked_request = mocker.patch.object(requests, "request")
         mocked_request.return_value = blank_response
 
+        # Then
         with pytest.raises(ResponseParseError):
+            # When
             child_service._service_request(
                 _ChildServiceResponse,
                 _response_class_kwargs={"key": "unexpected"}

tests/core/bases/test_file_response.py

@@ -4,7 +4,9 @@
 
 class TestFileResponse:
     def test_initialization(self, content_response):
+        # When
         instance = FileResponse(content_response)
 
+        # Then
         assert instance.content == content_response.content
         assert isinstance(instance.meta, FileResponseMeta)