Skip to content

Conversation

@max-svistunov
Copy link
Contributor

@max-svistunov max-svistunov commented Oct 21, 2025

Description

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-417
  • Closes # LCORE-417

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  • Please provide detailed steps to perform tests related to this code change.
  • How were the fix/results from this change verified? Please provide relevant screenshots or results.

Summary by CodeRabbit

  • Tests
    • Standardized unit tests to use pytest-mock's mocker fixture for mocks and async mocks, updating test signatures and scaffolding for consistency.
    • Added and updated multiple unit tests, including expanded coverage around header extraction and related utilities.
  • Chores
    • Added Ruff linter configuration to project settings to enforce import/linting guidance.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 21, 2025

Walkthrough

Replace unittest.mock usage in tests with pytest-mock's mocker fixture: change Mock/AsyncMock instantiations to mocker.Mock/mocker.AsyncMock, convert patch context managers to mocker.patch, and add mocker: MockerFixture parameters to affected tests and fixtures. No production code changes.

Changes

Cohort / File(s) Summary
App endpoint tests
tests/unit/app/endpoints/test_conversations_v2.py, tests/unit/app/endpoints/test_health.py, tests/unit/app/endpoints/test_providers.py
Replaced unittest.mock.Mock/AsyncMock with mocker.Mock/mocker.AsyncMock, updated fixtures and test signatures to accept mocker: MockerFixture, and switched to mocker.patch where used.
Runner & request model tests
tests/unit/runners/test_uvicorn_runner.py, tests/unit/models/requests/test_query_request.py
Migrated tests from unittest.mock to pytest-mock: updated test signatures to accept mocker, replaced patch(...) usage with mocker.patch(...), and instantiated mocks via mocker.
Utility tests
tests/unit/utils/auth_helpers.py, tests/unit/utils/test_checks.py, tests/unit/utils/test_common.py, tests/unit/utils/test_mcp_headers.py, tests/unit/utils/test_types.py
Systematically replaced Mock/AsyncMock and patch usages with mocker.Mock/mocker.AsyncMock and mocker.patch; added mocker: MockerFixture parameters and adjusted imports to use pytest_mock.
Tooling config
pyproject.toml
Added tool.ruff configuration including a banned-api rule recommending pytest/pytest-mock over unittest/unittest.mock.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

Suggested reviewers

  • tisnik

Poem

🐇 I hopped through tests with tiny feet,

Swapped old mocks for something neat,
Mocker snug in every test,
Async hops now sleep more rest,
A carrot cheer for CI green sweet 🥕

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 PR title "LCORE-417 Convert unittest mocking to pytest mocking" directly and accurately describes the primary change across all modified test files. The title is concise, specific, and clearly communicates that the PR performs a systematic migration of mocking patterns from unittest.mock to pytest-mock throughout the test suite. A developer scanning the PR history would immediately understand the scope and intent of these changes without ambiguity.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3c8f7a2 and aa7b22b.

📒 Files selected for processing (1)
  • pyproject.toml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • pyproject.toml

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

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

@max-svistunov max-svistunov force-pushed the lcore-417-convert-to-pytest branch 2 times, most recently from dbefba8 to 6cb76d2 Compare October 21, 2025 20:02
@max-svistunov max-svistunov force-pushed the lcore-417-convert-to-pytest branch from 6cb76d2 to 34a8644 Compare October 21, 2025 20:09
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 34a8644 and 3c8f7a2.

📒 Files selected for processing (1)
  • pyproject.toml (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
pyproject.toml

📄 CodeRabbit inference engine (CLAUDE.md)

pyproject.toml: ALWAYS check pyproject.toml for existing dependencies before adding new ones
ALWAYS verify current library versions in pyproject.toml rather than assuming versions
Prefer reading supported Python versions and tool configs from pyproject.toml

Files:

  • pyproject.toml
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: lightspeed-core/lightspeed-stack#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-18T16:46:33.353Z
Learning: Applies to tests/**/*.py : Use pytest-mock to create AsyncMock objects for async interactions in tests
Learnt from: CR
PR: lightspeed-core/lightspeed-stack#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-18T16:46:33.353Z
Learning: Applies to tests/{unit,integration}/**/*.py : Do not use unittest in tests; pytest is the standard

@max-svistunov max-svistunov force-pushed the lcore-417-convert-to-pytest branch from 3c8f7a2 to aa7b22b Compare October 21, 2025 20:52
@max-svistunov
Copy link
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 22, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Contributor

@tisnik tisnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tisnik tisnik merged commit bd392a3 into lightspeed-core:main Oct 22, 2025
18 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants