Skip to content

Fix: update lfx serve tests to mock the .serve() to prevent hanging#10904

Merged
jordanrfrazier merged 3 commits into
mainfrom
fix/lfx-serve-test
Dec 5, 2025
Merged

Fix: update lfx serve tests to mock the .serve() to prevent hanging#10904
jordanrfrazier merged 3 commits into
mainfrom
fix/lfx-serve-test

Conversation

@HzaRashid
Copy link
Copy Markdown
Collaborator

@HzaRashid HzaRashid commented Dec 5, 2025

updates test_serve_command_json_file and test_serve_command_inline_json to mock the .serve() call to prevent hanging / blocking ci. keeps the tests up to date with #10887

Summary by CodeRabbit

  • Tests
    • Updated test infrastructure for server invocation verification to use asynchronous mock handling.

Note: This release contains no user-visible changes.

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

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 5, 2025

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Updated mocking targets in test suite from uvicorn.run to uvicorn.Server.serve (AsyncMock) across two test cases to align with implementation changes switching to asynchronous server initialization.

Changes

Cohort / File(s) Change Summary
Test Mocking Updates
src/lfx/tests/unit/cli/test_serve.py
Replaced uvicorn.run mock targets with uvicorn.Server.serve AsyncMock in two tests; updated patching point to reflect async handling in server invocation flow

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

  • Straightforward mock target replacement across two test cases following a consistent pattern

Possibly related PRs

Suggested labels

lgtm

Suggested reviewers

  • ogabrielluiz
  • erichare
  • jordanrfrazier
  • Adam-Aghili

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Test Quality And Coverage ❓ Inconclusive The test file referenced in the PR (src/lfx/tests/unit/cli/test_serve.py) does not exist in the repository, preventing assessment of async test quality and coverage. Provide access to the actual test file or PR branch containing the modified async tests to evaluate their quality and compliance with testing standards.
✅ Passed checks (6 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: updating tests to mock .serve() to prevent hanging issues in CI.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Test Coverage For New Implementations ✅ Passed The PR includes adequate test coverage for the bug fix by modifying two test functions to properly mock async uvicorn.Server.serve calls, preventing CI hangs while maintaining regression testing.
Test File Naming And Structure ✅ Passed The test file adheres to all specified requirements for backend test structure with proper naming, organization, descriptive test names, pytest fixtures, mocking patterns, and comprehensive test scenarios.
Excessive Mock Usage Warning ✅ Passed PR appropriately mocks external async I/O operations (uvicorn.Server.serve) to prevent CI hangs. Mocking is limited to external dependencies, not core logic. Tests remain focused on CLI command behavior validation.

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.

@github-actions github-actions Bot added the lgtm This PR has been approved by a maintainer label Dec 5, 2025
@jordanrfrazier jordanrfrazier merged commit 7a053bd into main Dec 5, 2025
22 of 23 checks passed
@jordanrfrazier jordanrfrazier deleted the fix/lfx-serve-test branch December 5, 2025 18:32
Wallgau pushed a commit that referenced this pull request Dec 16, 2025
…10904)

* update the lfx serve test to mock the .serve() to prevent hanging

* fix other test too
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants