LCORE-653: better code coverage for Requests unit tests

[tisnik]

Description

LCORE-653: better code coverage for Requests unit tests

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-653

Summary by CodeRabbit

• Tests
  • Expanded coverage for request validation, including boolean status handling in feedback status updates.
  • Added a test to ensure an invalid conversation ID produces a clear error.
  • Strengthened response validation by confirming all required fields are enforced and missing data raises a validation error.
  • These additions improve reliability, clarify error behavior, and help prevent regressions.

[coderabbitai]

Walkthrough

Adds new and expanded unit tests: introduces tests for FeedbackStatusUpdateRequest, adds a constructor error test for QueryRequest invalid conversation_id, and extends AuthorizedResponse tests to assert that constructing without required fields raises ValidationError.

Changes

Cohort / File(s)	Summary
Requests model tests tests/unit/models/requests/test_feedback_status_update_request.py, tests/unit/models/requests/test_query_request.py	New tests for FeedbackStatusUpdateRequest constructor and get_value; added QueryRequest test asserting ValueError for invalid conversation_id ("xyzzy").
Responses model tests tests/unit/models/responses/test_authorized_response.py	Added test to ensure constructing AuthorizedResponse with no parameters raises ValidationError; existing required-field checks retained.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• LCORE-641: split unit tests for HTTP requests models #510 — Adds/updates unit tests for QueryRequest validation, overlapping with the new invalid conversation_id test.
• LCORE-641: split unit tests for HTTP responses models #509 — Modifies tests for AuthorizedResponse required fields, matching the new no-args ValidationError check.
• LCORE-641: authorized response unit tests update #515 — Adjusts AuthorizedResponse tests around required fields and flags, intersecting with this test expansion.

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title concisely highlights the primary change by referencing the JIRA issue and clearly stating that the update improves code coverage for Request unit tests, which aligns directly with the modifications in the pull request. It is specific enough for a reviewer to understand the main purpose without delving into implementation details and avoids unnecessary noise. This phrasing effectively communicates the core intent of the changeset.
Description Check	✅ Passed	The description appropriately identifies the pull request’s goal of improving unit test coverage for Requests, correctly categorizes it under “Unit tests improvement,” and links to the related issue, demonstrating clear relevance to the changeset. It provides context for reviewers without veering off-topic or including unrelated information. The level of detail, though concise, sufficiently describes the PR’s intent and scope.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

Poem

A whisk of tests, a thump of feet,
I nose the code—so crisp and neat.
Flags flip true/false, IDs protest,
No fields? The forms will fail the test.
I hop, I stamp, CI lights glow—
Approved! Now watch these carrots grow. 🥕✨

✨ 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: 1

🧹 Nitpick comments (1)

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

1-23: Optionally extend coverage for defaults and forbidden extras

Consider asserting the default and extra="forbid" behavior.

Add:

import pytest
from pydantic import ValidationError

def test_default_and_extra_fields() -> None:
    fs = FeedbackStatusUpdateRequest()
    assert fs.status is False

    with pytest.raises(ValidationError):
        FeedbackStatusUpdateRequest(status=True, unexpected="x")

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2ba31bd and a63ba20.

📒 Files selected for processing (3)

• tests/unit/models/requests/test_feedback_status_update_request.py (1 hunks)
• tests/unit/models/requests/test_query_request.py (1 hunks)
• tests/unit/models/responses/test_authorized_response.py (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

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

src/models/requests.py (2)

• FeedbackStatusUpdateRequest (387-412)
• get_value (410-412)

tests/unit/models/responses/test_authorized_response.py (1)

src/models/responses.py (1)

• AuthorizedResponse (304-340)

⏰ 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). (3)

• GitHub Check: Pyright
• GitHub Check: build-pr
• GitHub Check: e2e_tests

🔇 Additional comments (2)

tests/unit/models/responses/test_authorized_response.py (1)

26-28: LGTM: Validates required fields via Pydantic correctly

Asserting ValidationError for missing params is appropriate here.

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

1-23: Solid coverage for constructor and accessor

Tests cover both boolean states and getter; no issues spotted.

[coderabbitai]

⚠️ Potential issue

Use ValidationError instead of ValueError for Pydantic field validation

check_uuid is a Pydantic field_validator; raising ValueError there surfaces as pydantic.ValidationError at model init. This test will otherwise fail.

Apply this diff:

-    def test_constructor_wrong_conversation_id(self) -> None:
+    def test_constructor_wrong_conversation_id(self) -> None:
         """Test the QueryRequest constructor with wrong conversation_id."""
-        with pytest.raises(ValueError, match="Improper conversation ID 'xyzzy'"):
+        with pytest.raises(ValidationError, match="Improper conversation ID 'xyzzy'"):
             _ = QueryRequest(query="Tell me about Kubernetes", conversation_id="xyzzy")

Also add the missing import near the other imports:

from pydantic import ValidationError

🤖 Prompt for AI Agents

In tests/unit/models/requests/test_query_request.py around lines 25 to 28, the
test currently expects a ValueError but the field validator in the Pydantic
model surfaces a pydantic.ValidationError at model initialization; change the
pytest.raises(ValueError, ...) to pytest.raises(ValidationError, ...) and ensure
you add the missing import from pydantic import ValidationError alongside the
other imports at the top of the file so the test asserts the correct exception
type.