LCORE-974: updated unit tests

[tisnik]

Description

LCORE-974: updated 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

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

Related Tickets & Documents

• Related Issue #LCORE-974

Summary by CodeRabbit

• Tests
  • Updated configurations with new timeout parameter support.
  • Added support for transcript-related configuration fields in user data structures.
  • Enhanced type checking annotations across test suites to improve type safety.
  • Refactored mock implementations for improved test flexibility.

[coderabbitai]

Walkthrough

This PR updates test files to accommodate two new parameters in core data models: a timeout field added to LlamaStackConfiguration and transcripts_enabled/transcripts_storage fields added to UserDataCollection. Changes include parameter additions to test instantiations, PyRight type-checking suppressions, mock parameter binding adjustments, and minor formatting updates across 13+ test files. No functional logic changes; purely test infrastructure updates.

Changes

Cohort / File(s)	Summary
LlamaStackConfiguration timeout parameter tests/unit/models/config/test_authentication_configuration.py, tests/unit/models/config/test_llama_stack_configuration.py, tests/unit/models/responses/test_successful_responses.py, tests/unit/test_client.py, tests/unit/utils/test_common.py	Updated test instantiations of LlamaStackConfiguration to include the new timeout=60 parameter across configuration setup paths.
UserDataCollection transcript fields tests/unit/app/endpoints/test_feedback.py	Constructor calls and assertions updated to accommodate new transcripts_enabled and transcripts_storage fields; added type: ignore annotations to detail checks.
PyRight type-checking suppressions tests/unit/app/endpoints/test_rags.py, tests/unit/authentication/test_api_key_token.py, tests/unit/authentication/test_jwk_token.py, tests/unit/cache/test_cache_factory.py, tests/unit/cache/test_postgres_cache.py, tests/unit/models/rlsapi/test_requests.py, tests/unit/models/rlsapi/test_responses.py	Added inline pyright: ignore comments to suppress type-checking warnings on constructor calls and assertions; no behavioral changes.
Mock parameter binding tests/unit/app/endpoints/test_tools.py	Authorization mock patches updated to use throwaway parameter (_) instead of named parameter (action) for increased parameter acceptance flexibility.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• [feat] Add configurable timeout for Llama Stack requests #1087: Directly adds the timeout field to LlamaStackConfiguration in the source model.
• LCORE-1280: docs update #1092: Also introduces timeout field to LlamaStackConfiguration configuration model.
• OLS-1946: Change double negative in user data collection config #271: Modifies UserDataCollection with new feedback and transcript-related flags and fields.

Suggested labels

ok-to-test

Suggested reviewers

• manstis
• ldjebran
• maorfr

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'LCORE-974: updated unit tests' is vague and generic, using non-descriptive terms that don't convey meaningful information about the specific changes made.	Consider using a more specific title that highlights the primary change, such as 'Add timeout parameter to LlamaStackConfiguration and update type annotations' or 'Update unit tests for configuration and type-checking updates'.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

tests/unit/authentication/test_jwk_token.py (1)

213-219: ⚠️ Potential issue | 🟡 Minor

Remove the unnecessary pyright suppression.

All fields in JwtConfiguration have default values (user_id_claim defaults to constants.DEFAULT_JWT_UID_CLAIM, username_claim defaults to constants.DEFAULT_JWT_USER_NAME_CLAIM, and role_rules has default_factory=list). Calling JwtConfiguration() with no arguments is valid and does not produce a reportCallIssue. The # pyright: ignore[reportCallIssue] comment on line 219 should be removed.

🧹 Nitpick comments (2)

tests/unit/authentication/test_api_key_token.py (1)

81-83: Type suppression pattern is acceptable but could be cleaner.

The isinstance(detail, dict) assertions on lines 82 and 107 should narrow the type, but the type checker doesn't always recognize this in assertion contexts. The # type: ignore[index] comments work but can mask real issues if the type of detail changes.

A slightly more robust pattern would be to assign after the type guard:

♻️ Optional: Cleaner type narrowing pattern

     assert exc_info.value.status_code == 401
     detail = exc_info.value.detail
     assert isinstance(detail, dict)
-    assert detail["cause"] == "No Authorization header found"  # type: ignore[index]
+    assert detail["cause"] == "No Authorization header found"

If the type checker still complains, consider using an intermediate variable:

assert isinstance(detail, dict)
detail_dict: dict[str, str] = detail
assert detail_dict["cause"] == "No Authorization header found"

That said, the current approach is pragmatic and doesn't affect test correctness.

Also applies to: 106-108

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

56-59: Type ignore comments are acceptable, but consider a cleaner pattern.

The # type: ignore[index] comments are necessary because Pyright doesn't narrow HTTPException.detail (typed as str | dict[str, Any] | None) after the isinstance(detail, dict) assertion. This same pattern appears at lines 59, 161, and 189.

A slightly cleaner alternative would be to assign to a typed variable after the isinstance check, which avoids scattering type ignores:

♻️ Optional refactor

     detail = e.value.detail
     assert isinstance(detail, dict)
-    assert "response" in detail
-    assert "Unable to connect to Llama Stack" in detail["response"]  # type: ignore[index]
+    response = detail.get("response")
+    assert response is not None
+    assert "Unable to connect to Llama Stack" in response