Chore/add tests #46

mnbf9rca · 2024-07-12T14:44:13Z

Summary by Sourcery

This pull request refactors the Client class to improve request handling and deserialization, updates the ApiError model for better timestamp parsing, and adds comprehensive tests for various functionalities of the Client class.

Enhancements:
- Refactored multiple methods in the Client class to use a new _send_request_and_deserialize method for sending requests and deserializing responses.
- Updated the _get_result_expiry method to handle cases where the 'date' header is missing in the response.
- Modified the ApiError model to use parsedate_to_datetime for parsing the timestamp.
Tests:
- Added comprehensive tests for the Client class, including tests for model creation, client initialization, model loading, cache control header parsing, deserialization, and error handling.

sourcery-ai · 2024-07-12T14:44:20Z

Reviewer's Guide by Sourcery

This pull request primarily focuses on adding tests for the Client class in the pydantic_tfl_api module. It also includes some refactoring and minor changes to the client.py and models/api_error.py files to improve code quality and functionality. The new tests cover various methods and edge cases, ensuring better code coverage and reliability.

File-Level Changes

Files	Changes
`pydantic_tfl_api/client.py` `pydantic_tfl_api/models/api_error.py`	Refactored methods for better error handling and code quality, including changes to timestamp parsing and request handling.
`tests/test_client.py`	Added a comprehensive test suite for the `Client` class, covering various methods and edge cases to ensure better code coverage and reliability.
`example.py`	Added an example script demonstrating the usage of the `Client` class with various methods.

Tips

Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
Continue your discussion with Sourcery by replying directly to review comments.
You can change your review settings at any time by accessing your dashboard:
- Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
- Change the review language;
You can always contact us if you have any questions or feedback.

sourcery-ai

Hey @mnbf9rca - I've reviewed your changes and they look great!

Here's what I looked at during the review

🟡 General issues: 4 issues found
🟢 Security: all looks good
🟡 Testing: 1 issue found
🟢 Complexity: all looks good
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.}

sourcery-ai · 2024-07-12T14:45:26Z

pydantic_tfl_api/client.py


    def _get_result_expiry(self, response: Response) -> datetime | None:
        s_maxage = self._get_s_maxage_from_cache_control_header(response)
-        request_datetime = parsedate_to_datetime(response.headers.get("date"))
+        request_datetime = parsedate_to_datetime(response.headers.get("date")) if "date" in response.headers else None


issue: Handling of missing 'date' header

Returning None when the 'date' header is missing might lead to issues downstream if request_datetime is expected to be a datetime object. Consider raising an exception or handling this case more explicitly.

sourcery-ai · 2024-07-12T14:45:26Z

pydantic_tfl_api/client.py

@@ -90,7 +90,7 @@ def _get_model(self, model_name: str) -> BaseModel:

    def _create_model_instance(
        self, Model: BaseModel, response_json: Any, result_expiry: datetime | None
-    ):
+    ) -> BaseModel | List[BaseModel]:


suggestion: Return type annotation

The return type annotation BaseModel | List[BaseModel] might be too broad. If possible, consider specifying more precise types to improve type safety and readability.

Suggested change

) -> BaseModel | List[BaseModel]:

) -> Union[Model, List[Model]]:

sourcery-ai · 2024-07-12T14:45:26Z

pydantic_tfl_api/client.py

+        return self._send_request_and_deserialize(
+            endpoints["stopPointsByLineId"].format(line_id), "StopPoint"
        )
-        if response.status_code != 200:
-            return self._deserialize_error(response)
-        return self._deserialize("StopPoint", response)

    def get_line_meta_modes(self) -> models.Mode | models.ApiError:
-        response = self.client.send_request(endpoints["lineMetaModes"])
-        if response.status_code != 200:
-            return self._deserialize_error(response)
-        return self._deserialize("Mode", response)
+        return self._send_request_and_deserialize(endpoints["lineMetaModes"], "Mode")

    def get_lines(
        self, line_id: str | None = None, mode: str | None = None
    ) -> models.Line | List[models.Line] | models.ApiError:


issue: Refactoring to use _send_request_and_deserialize

The refactoring to use _send_request_and_deserialize improves code reuse and readability. However, ensure that the new method handles all edge cases and error conditions that were previously managed in the individual methods.

sourcery-ai · 2024-07-12T14:45:27Z

pydantic_tfl_api/models/api_error.py

@@ -11,6 +12,7 @@ class ApiError(BaseModel):

    @field_validator('timestamp_utc', mode='before')
    def parse_timestamp(cls, v):
-        return datetime.strptime(v, '%a, %d %b %Y %H:%M:%S %Z')
+        return v if isinstance(v, datetime) else parsedate_to_datetime(v)


suggestion: Field validator logic

The field validator now handles both datetime objects and string dates. Ensure that parsedate_to_datetime correctly handles all expected date formats and edge cases.

Suggested change

return v if isinstance(v, datetime) else parsedate_to_datetime(v)

if isinstance(v, datetime):

return v

try:

return parsedate_to_datetime(v)

except (TypeError, ValueError):

raise ValueError(f"Invalid date format: {v}")

sourcery-ai · 2024-07-12T14:45:27Z

tests/test_client.py

+@pytest.mark.parametrize(
+    "Model, response_json, result_expiry, expected_name, expected_age, expected_expiry",
+    [
+        # Happy path tests
+        (
+            PydanticTestModel,
+            {"name": "Alice", "age": 30},
+            datetime(2023, 12, 31),
+            "Alice",
+            30,


suggestion (testing): Missing test for invalid model name in test_create_model_with_expiry

Consider adding a test case for an invalid model name to ensure that the function handles it gracefully.

sourcery-ai · 2024-07-12T14:45:27Z

tests/test_client.py

+    if expected_name is None:
+        with pytest.raises(ValidationError):
+            Client._create_model_with_expiry(None, Model, response_json, result_expiry)
+    else:
+        instance = Client._create_model_with_expiry(
+            None, Model, response_json, result_expiry
+        )
+
+        # Assert
+        assert instance.name == expected_name
+        assert instance.age == expected_age
+        assert instance.content_expires == expected_expiry


issue (code-quality): Avoid conditionals in tests. (no-conditionals-in-tests)

Explanation
Avoid complex code, like conditionals, in test functions.
Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:

loops

conditionals

Some ways to fix this:

Use parametrized tests to get rid of the loop.

Move the complex logic into helpers.

Move the complex part into pytest fixtures.

Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.

Software Engineering at Google / Don't Put Logic in Tests

sourcery-ai · 2024-07-12T14:45:27Z

tests/test_client.py

+    if exception:
+        with pytest.raises(exception):
+            Client._get_model(client, model_name)
+    else:
+        result = Client._get_model(client, model_name)
+        assert result == expected_result


issue (code-quality): Avoid conditionals in tests. (no-conditionals-in-tests)

Explanation
Avoid complex code, like conditionals, in test functions.
Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:

loops

conditionals

Some ways to fix this:

Use parametrized tests to get rid of the loop.

Move the complex logic into helpers.

Move the complex part into pytest fixtures.

Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.

Software Engineering at Google / Don't Put Logic in Tests

sourcery-ai · 2024-07-12T14:45:27Z

tests/test_client.py

+        if isinstance(response_json, dict):
+            mock_create_model_with_expiry.assert_called_with(
+                Model, response_json, result_expiry
+            )
+        else:
+            for item in response_json:
+                mock_create_model_with_expiry.assert_any_call(Model, item, result_expiry)


issue (code-quality): Avoid conditionals in tests. (no-conditionals-in-tests)

Explanation
Avoid complex code, like conditionals, in test functions.
Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:

loops

conditionals

Some ways to fix this:

Use parametrized tests to get rid of the loop.

Move the complex logic into helpers.

Move the complex part into pytest fixtures.

Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.

Software Engineering at Google / Don't Put Logic in Tests

sourcery-ai · 2024-07-12T14:45:27Z

tests/test_client.py

+            for item in response_json:
+                mock_create_model_with_expiry.assert_any_call(Model, item, result_expiry)


issue (code-quality): Avoid loops in tests. (no-loop-in-tests)

Explanation
Avoid complex code, like loops, in test functions.
Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:

loops

conditionals

Some ways to fix this:

Use parametrized tests to get rid of the loop.

Move the complex logic into helpers.

Move the complex part into pytest fixtures.

Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.

Software Engineering at Google / Don't Put Logic in Tests

…ethod

mnbf9rca added 11 commits July 12, 2024 11:45

refactor client with _send_request_and_deserialize()

53962a4

test _create_model_with_expiry

c01c638

test Client initialization

802e3f9

test client._load_models

557b7a6

chore: Add launch configuration for Python debugging

a1f555b

test client._get_s_maxage_from_cache_control_header

7d6bc16

test Client._deserialize

4fcf966

test _get_result_expiry

9226423

test _get_model

e6b8bfc

create tests for _create_model_instance

0037a7c

test _deserialize_error

b571b15

sourcery-ai bot reviewed Jul 12, 2024

View reviewed changes

mnbf9rca added 3 commits July 15, 2024 11:15

chore: Add optional service types parameter to get_route_by_mode method

f46f376

chore: Update RestClient to include request headers in send_request m…

401a301

…ethod

chore: Add test files for TFL API responses

305094a

mnbf9rca merged commit 0504bed into main Jul 15, 2024
4 checks passed

mnbf9rca temporarily deployed to bump-version July 15, 2024 13:30 — with GitHub Actions Inactive

mnbf9rca deleted the chore/add-tests branch July 15, 2024 13:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Chore/add tests #46

Chore/add tests #46

mnbf9rca commented Jul 12, 2024 •

edited by sourcery-ai bot

Loading

sourcery-ai bot commented Jul 12, 2024 •

edited

Loading

sourcery-ai bot left a comment

sourcery-ai bot Jul 12, 2024

sourcery-ai bot Jul 12, 2024

sourcery-ai bot Jul 12, 2024

sourcery-ai bot Jul 12, 2024

sourcery-ai bot Jul 12, 2024

sourcery-ai bot Jul 12, 2024

sourcery-ai bot Jul 12, 2024

sourcery-ai bot Jul 12, 2024

sourcery-ai bot Jul 12, 2024

	) -> BaseModel \| List[BaseModel]:
	) -> Union[Model, List[Model]]:

-        return v if isinstance(v, datetime) else parsedate_to_datetime(v)
+        if isinstance(v, datetime):
+            return v
+        try:
+            return parsedate_to_datetime(v)
+        except (TypeError, ValueError):
+            raise ValueError(f"Invalid date format: {v}")

		for item in response_json:
		mock_create_model_with_expiry.assert_any_call(Model, item, result_expiry)

Chore/add tests #46

Chore/add tests #46

Conversation

mnbf9rca commented Jul 12, 2024 • edited by sourcery-ai bot Loading

Summary by Sourcery

sourcery-ai bot commented Jul 12, 2024 • edited Loading

Reviewer's Guide by Sourcery

File-Level Changes

sourcery-ai bot left a comment

Choose a reason for hiding this comment

sourcery-ai bot Jul 12, 2024

Choose a reason for hiding this comment

sourcery-ai bot Jul 12, 2024

Choose a reason for hiding this comment

sourcery-ai bot Jul 12, 2024

Choose a reason for hiding this comment

sourcery-ai bot Jul 12, 2024

Choose a reason for hiding this comment

sourcery-ai bot Jul 12, 2024

Choose a reason for hiding this comment

sourcery-ai bot Jul 12, 2024

Choose a reason for hiding this comment

sourcery-ai bot Jul 12, 2024

Choose a reason for hiding this comment

sourcery-ai bot Jul 12, 2024

Choose a reason for hiding this comment

sourcery-ai bot Jul 12, 2024

Choose a reason for hiding this comment

mnbf9rca commented Jul 12, 2024 •

edited by sourcery-ai bot

Loading

sourcery-ai bot commented Jul 12, 2024 •

edited

Loading