LCORE-641: split unit tests for HTTP requests models

[tisnik]

Description

LCORE-641: split unit tests for HTTP requests models

Type of change

• Refactor
• New feature
• Bug fix
• CVE fix
• Optimization
• Documentation Update
• Configuration Update
• Bump-up service version
• Bump-up dependent library
• Bump-up library or tool used for development (does not change the final image)
• CI configuration change
• Konflux configuration change
• Unit tests improvement
• Integration tests improvement
• End to end tests improvement

Related Tickets & Documents

• Related Issue #LCORE-641

Summary by CodeRabbit

• Tests
  • Added unit tests for attachments validating constructor behavior and permissive handling of unknown content types.
  • Added comprehensive tests for query requests covering defaults, optional fields, attachment-to-document conversion, provider/model validation, and media-type logging.
  • Simplified feedback request tests to focus solely on FeedbackRequest scenarios, removing unrelated cases.
  • Improves test coverage and confidence in request handling without user-facing changes.

[coderabbitai]

Walkthrough

Adds new unit tests for Attachment and QueryRequest models and simplifies FeedbackRequest tests to focus only on FeedbackRequest. The changes introduce constructor, attachment handling, provider/model validation, get_documents and media_type logging tests while removing unrelated suites from the FeedbackRequest file.

Changes

Cohort / File(s)	Summary
Attachment tests tests/unit/models/requests/test_attachment.py	New test module with TestAttachment: tests Attachment constructor with standard and unknown content types; asserts fields reflect inputs.
FeedbackRequest tests simplification tests/unit/models/requests/test_feedback_request.py	Reduced to only FeedbackRequest tests; removed prior Attachment and QueryRequest test suites; imports trimmed and placeholder TestFeedbackRequest added.
QueryRequest tests tests/unit/models/requests/test_query_request.py	New comprehensive QueryRequest tests: constructor defaults, optional fields, attachments handling, get_documents behavior, validate_provider_and_model error cases, and media_type warning logged via mocked logger.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• LCORE-641: stub allowing splitting unit tests for models #508 — Adds/tests or package initializer under tests/unit/models/requests/ that this change depends on.

Poem

I twitch my whiskers at tests so bright,
New cases hop in, assertions tight.
Attachments snug, queries roam,
Feedback trimmed to call home.
I thump with joy — the test suite’s right! 🐇✨

✨ Finishing Touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

tests/unit/models/requests/test_attachment.py (2)

1-4: De-duplicate via parametrization and fix misnamed case

Both tests validate the same constructor behavior; parametrize on content_type to reduce duplication and implicitly fix the “unknown_attachment_type” misnomer (it’s actually testing content_type).

Apply:

@@
-"""Unit tests for Attachment model."""
+"""Unit tests for Attachment model."""
 
-from models.requests import Attachment
+from models.requests import Attachment
+import pytest
@@
-    def test_constructor(self) -> None:
-        """Test the Attachment with custom values."""
-        a = Attachment(
-            attachment_type="configuration",
-            content_type="application/yaml",
-            content="kind: Pod\n metadata:\n name:    private-reg",
-        )
-        assert a.attachment_type == "configuration"
-        assert a.content_type == "application/yaml"
-        assert a.content == "kind: Pod\n metadata:\n name:    private-reg"
+    @pytest.mark.parametrize("content_type", ["application/yaml", "unknown/type"])
+    def test_constructor(self, content_type: str) -> None:
+        """Attachment accepts any content_type; fields are preserved."""
+        a = Attachment(
+            attachment_type="configuration",
+            content_type=content_type,
+            content="kind: Pod\n metadata:\n name:    private-reg",
+        )
+        assert a.attachment_type == "configuration"
+        assert a.content_type == content_type
+        assert a.content == "kind: Pod\n metadata:\n name:    private-reg"
@@
-    def test_constructor_unknown_attachment_type(self) -> None:
-        """Test the Attachment with custom values."""
-        # for now we allow any content type
-        a = Attachment(
-            attachment_type="configuration",
-            content_type="unknown/type",
-            content="kind: Pod\n metadata:\n name:    private-reg",
-        )
-        assert a.attachment_type == "configuration"
-        assert a.content_type == "unknown/type"
-        assert a.content == "kind: Pod\n metadata:\n name:    private-reg"

Also applies to: 9-30

---

1-4: Add a negative test for forbidden extra fields (pydantic model_config extra='forbid')

Covers an important contract of Attachment: unexpected fields should raise ValidationError.

Apply:

@@
-from models.requests import Attachment
+from models.requests import Attachment
+from pydantic import ValidationError
@@
 class TestAttachment:
@@
+    def test_extra_fields_forbidden(self) -> None:
+        with pytest.raises(ValidationError):
+            Attachment(
+                attachment_type="log",
+                content_type="text/plain",
+                content="payload",
+                unexpected="boom",
+            )

Also applies to: 31-31

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 92c0ef5 and 6f3e0ab.

📒 Files selected for processing (3)

• tests/unit/models/requests/test_attachment.py (1 hunks)
• tests/unit/models/requests/test_feedback_request.py (1 hunks)
• tests/unit/models/requests/test_query_request.py (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

• tests/unit/models/requests/test_query_request.py
• tests/unit/models/requests/test_feedback_request.py

🧰 Additional context used

🧬 Code graph analysis (1)

tests/unit/models/requests/test_attachment.py (1)

src/models/requests.py (1)

• Attachment (15-69)

⏰ 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 (1)

tests/unit/models/requests/test_attachment.py (1)

9-18: Solid constructor happy-path coverage

Asserts match the model fields and preserve exact content. Good baseline.