LCORE-641: split unit tests for HTTP responses models #509
Conversation
WalkthroughAdds four new unit test modules under tests/unit/models/responses for individual response models and removes the prior consolidated test file tests/unit/models/test_responses.py. Tests cover constructors and validation behavior for AuthorizedResponse, QueryResponse, StatusResponse, and UnauthorizedResponse. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
tests/unit/models/responses/test_unauthorized_response.py (1)
20-23: Strengthen the ValidationError assertion and prefer tool-agnostic ignore.Capture the exception and assert the missing field to avoid false positives; also use a cross-checker-friendly ignore comment.
- def test_constructor_fields_required(self) -> None: - """Test the UnauthorizedResponse constructor.""" - with pytest.raises(ValidationError): - _ = UnauthorizedResponse() # pyright: ignore + def test_constructor_fields_required(self) -> None: + """Test the UnauthorizedResponse constructor.""" + with pytest.raises(ValidationError) as excinfo: + _ = UnauthorizedResponse() # type: ignore[call-arg] + errors = excinfo.value.errors() + assert any("detail" in e.get("loc", []) for e in errors)tests/unit/models/responses/test_authorized_response.py (2)
22-33: DRY up missing-field tests with parametrization and assert the exact missing field.Reduces duplication and tightens validation.
- def test_constructor_fields_required(self) -> None: - """Test the AuthorizedResponse constructor.""" - - with pytest.raises(ValidationError): - # missing user_id parameter - _ = AuthorizedResponse(username="testuser") # pyright: ignore - - with pytest.raises(ValidationError): - # missing username parameter - _ = AuthorizedResponse( - user_id="123e4567-e89b-12d3-a456-426614174000" - ) # pyright: ignore + @pytest.mark.parametrize( + ("kwargs", "missing"), + [ + ({"username": "testuser"}, "user_id"), + ({"user_id": "123e4567-e89b-12d3-a456-426614174000"}, "username"), + ], + ) + def test_constructor_fields_required(self, kwargs, missing) -> None: + """Test the AuthorizedResponse constructor.""" + with pytest.raises(ValidationError) as excinfo: + _ = AuthorizedResponse(**kwargs) # type: ignore[call-arg] + assert any(missing in e.get("loc", []) for e in excinfo.value.errors())
15-21: Optional: validate UUID format explicitly.If user_id is typed as UUID, add a negative test (e.g., user_id="not-a-uuid") to assert a ValidationError with loc containing "user_id".
tests/unit/models/responses/test_query_response.py (1)
18-22: Add a negative test for required ‘response’.Ensures we fail fast when the core payload is missing.
+import pytest +from pydantic import ValidationError @@ class TestQueryResponse: @@ def test_optional_conversation_id(self) -> None: """Test the QueryResponse with default conversation ID.""" qr = QueryResponse(response="LLM answer") assert qr.conversation_id is None assert qr.response == "LLM answer" + + def test_constructor_requires_response(self) -> None: + """Ensure `response` is required.""" + with pytest.raises(ValidationError): + _ = QueryResponse() # type: ignore[call-arg]tests/unit/models/responses/test_status_response.py (1)
9-19: Parametrize enabled/disabled to reduce duplication.Functionally equivalent but more concise and scalable for future cases.
- def test_constructor_feedback_enabled(self) -> None: - """Test the StatusResponse constructor.""" - sr = StatusResponse(functionality="feedback", status={"enabled": True}) - assert sr.functionality == "feedback" - assert sr.status == {"enabled": True} - - def test_constructor_feedback_disabled(self) -> None: - """Test the StatusResponse constructor.""" - sr = StatusResponse(functionality="feedback", status={"enabled": False}) - assert sr.functionality == "feedback" - assert sr.status == {"enabled": False} + @pytest.mark.parametrize("enabled", [True, False]) + def test_constructor_feedback_enabled_flag(self, enabled: bool) -> None: + """Test the StatusResponse constructor.""" + sr = StatusResponse(functionality="feedback", status={"enabled": enabled}) + assert sr.functionality == "feedback" + assert sr.status == {"enabled": enabled}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
tests/unit/models/responses/test_authorized_response.py(1 hunks)tests/unit/models/responses/test_query_response.py(1 hunks)tests/unit/models/responses/test_status_response.py(1 hunks)tests/unit/models/responses/test_unauthorized_response.py(1 hunks)tests/unit/models/test_responses.py(0 hunks)
💤 Files with no reviewable changes (1)
- tests/unit/models/test_responses.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-pr
🔇 Additional comments (3)
tests/unit/models/responses/test_unauthorized_response.py (1)
13-19: LGTM: constructor happy-path is clear and readable.tests/unit/models/responses/test_authorized_response.py (1)
13-21: LGTM: positive-path assertions look good.tests/unit/models/responses/test_query_response.py (1)
9-17: LGTM: covers primary fields and values.
1 participant
Description
LCORE-641: split unit tests for HTTP responses models
Type of change
Related Tickets & Documents
Summary by CodeRabbit
Tests
Chores
Notes