LCORE-1071: better comments in unit tests

[tisnik]

Description

LCORE-1071: better comments in 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

Tools used to create PR

Identify any AI code assistants used in this PR (for transparency and review context)

• Assisted-by: CodeRabbitAI
• Generated by: N/A

Related Tickets & Documents

• Related Issue #LCORE-1071

Summary by CodeRabbit

• Tests
  • Enhanced test mocks with additional attributes for more comprehensive validation.
  • Improved test documentation with detailed docstrings for better clarity and maintainability.
  • Made exception handling in tests more explicit for better error tracing.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Warning

Rate limit exceeded

@tisnik has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 17 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between cee6883 and 838b9ac.

📒 Files selected for processing (4)

• tests/unit/app/endpoints/test_query_v2.py (1 hunks)
• tests/unit/app/endpoints/test_rags.py (1 hunks)
• tests/unit/authorization/test_middleware.py (2 hunks)
• tests/unit/utils/test_llama_stack_version.py (1 hunks)

Walkthrough

All changes are confined to test files. Updates include adding comprehensive docstrings to helper functions and expanding test mock initialization with additional attributes to cover more test assertions without altering runtime control flow.

Changes

Cohort / File(s)	Summary
Test documentation & error handling tests/unit/app/endpoints/test_query_v2.py	Adds detailed docstring to _raise helper; now explicitly constructs a Request object and raises APIConnectionError with it embedded, replacing implicit exception behavior
Test mock expansion tests/unit/app/endpoints/test_rags.py	Enriches RagInfo mock initialization with full attribute set (id, name, created_at, last_active_at, expires_at, object, status, usage_bytes) to support extended test assertions
Test helper documentation tests/unit/authorization/test_middleware.py	Adds detailed docstrings to two inner async endpoint functions within test helpers; no logic or control flow changes
Test utility documentation tests/unit/utils/test_llama_stack_version.py	Adds comprehensive docstring to _check_version_must_fail helper describing purpose, parameters, and exceptions; no logic changes

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

• All changes are test-only with no production code modifications
• Updates consist primarily of docstring additions and mock attribute expansion
• Changes follow consistent patterns across multiple test files with minimal logic density
• No control-flow alterations or complex interactions to validate

Possibly related PRs

• LCORE-740: minor fixes in endpoint unit tests #763: Modifies the same test file (test_query_v2.py) to add explicit Request object construction and APIConnectionError raising in the error helper, with related docstring and typing improvements.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: adding docstrings and improving comments across multiple unit test files.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

---

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/app/endpoints/test_query_v2.py (1)

477-488: Align docstring/annotation with always‑raising helper

The docstring nicely explains that this helper always raises APIConnectionError with a Request, matching how it’s used in the test. One small nit: the function is annotated as -> Exception but the docstring’s “Returns: None” section suggests a normal return path, even though it never returns.

Consider either:

• Dropping the “Returns” section and stating explicitly that the function always raises, or
• Changing the return annotation to -> None or -> NoReturn to match the behavior.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dcb4637 and cee6883.

📒 Files selected for processing (4)

• tests/unit/app/endpoints/test_query_v2.py (1 hunks)
• tests/unit/app/endpoints/test_rags.py (1 hunks)
• tests/unit/authorization/test_middleware.py (2 hunks)
• tests/unit/utils/test_llama_stack_version.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

tests/{unit,integration}/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/{unit,integration}/**/*.py: Use pytest for all unit and integration tests; do not use unittest framework
Unit tests must achieve 60% code coverage; integration tests must achieve 10% coverage

Files:

• tests/unit/utils/test_llama_stack_version.py
• tests/unit/authorization/test_middleware.py
• tests/unit/app/endpoints/test_query_v2.py
• tests/unit/app/endpoints/test_rags.py

tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Use pytest-mock with AsyncMock objects for mocking in tests

Files:

• tests/unit/utils/test_llama_stack_version.py
• tests/unit/authorization/test_middleware.py
• tests/unit/app/endpoints/test_query_v2.py
• tests/unit/app/endpoints/test_rags.py

🧠 Learnings (1)

📚 Learning: 2025-11-24T16:58:04.410Z

Learnt from: CR
Repo: lightspeed-core/lightspeed-stack PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T16:58:04.410Z
Learning: Applies to src/**/{client,app/endpoints/**}.py : Handle `APIConnectionError` from Llama Stack in integration code

Applied to files:

• tests/unit/app/endpoints/test_query_v2.py

🪛 GitHub Actions: Python linter

tests/unit/utils/test_llama_stack_version.py

[error] 85-85: pylint failed during 'uv run pylint src tests': Line too long (116/100) (C0301).

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

• GitHub Check: build-pr
• GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
• GitHub Check: E2E: library mode / ci
• GitHub Check: E2E: library mode / azure
• GitHub Check: E2E: server mode / ci
• GitHub Check: E2E: server mode / azure

🔇 Additional comments (3)

tests/unit/authorization/test_middleware.py (2)

279-285: Inner endpoint docstring is clear and consistent

The added docstring succinctly documents the mock endpoint’s purpose, arguments, and return value; no changes needed.

---

302-308: Second mock endpoint docstring matches behavior and usage

This docstring mirrors the pattern from the success case and accurately describes the mocked failure endpoint. Looks good as-is.

tests/unit/app/endpoints/test_rags.py (1)

193-203: Expanded RagInfo mock docstring accurately documents fields

The added __init__ docstring clearly describes all predefined attributes and matches the assignments below, improving test readability without affecting behavior.