Skip to content

Celery: Update testcases mock#721

Merged
AkhileshNegi merged 1 commit intomainfrom
hotfix/celery-testcases-update
Mar 26, 2026
Merged

Celery: Update testcases mock#721
AkhileshNegi merged 1 commit intomainfrom
hotfix/celery-testcases-update

Conversation

@AkhileshNegi
Copy link
Collaborator

@AkhileshNegi AkhileshNegi commented Mar 26, 2026

Summary

Fixing Failing testcases in main due to celery changes

Checklist

Before submitting a pull request, please ensure that you mark these task.

  • Ran fastapi run --reload app/main.py or docker compose up in the repository root and test.
  • If you've fixed a bug or added code that is tested and has test cases.

Summary by CodeRabbit

Tests

  • Updated unit tests across multiple job scheduling modules to patch more specific job-starting functions instead of generic scheduler implementations.
  • Removed assertions validating internal function path arguments from scheduled job calls.
  • Enhanced test specificity and coverage precision for various job types including LLM processing, STT/TTS evaluation, collections management, document transformation, and response generation tasks.

@coderabbitai
Copy link

coderabbitai bot commented Mar 26, 2026

📝 Walkthrough

Walkthrough

The PR updates test mocks across multiple backend services to patch more specific job-scheduling functions (e.g., start_llm_job, start_stt_batch_submission, start_tts_batch_submission, etc.) instead of generic start_low_priority_job and start_high_priority_job. It also removes assertions validating the function_path parameter in scheduler call assertions.

Changes

Cohort / File(s) Summary
LLM Tests
backend/app/tests/api/routes/test_llm.py, backend/app/tests/services/llm/test_jobs.py
Replaced mocking start_high_priority_job with start_llm_job (or start_llm_chain_job for chain tests). Removed function_path assertions validating specific executor paths.
Evaluation Batch Tests
backend/app/tests/api/routes/test_stt_evaluation.py, backend/app/tests/api/routes/test_tts_evaluation.py
Replaced mocking start_low_priority_job with domain-specific functions (start_stt_batch_submission, start_tts_batch_submission). Removed function_path assertions from TTS tests.
Collection Management Tests
backend/app/tests/services/collections/test_create_collection.py, backend/app/tests/services/collections/test_delete_collection.py
Replaced mocking start_low_priority_job with job-specific schedulers (start_create_collection_job, start_delete_collection_job). Removed function_path kwargs assertions.
Document Transformation Tests
backend/app/tests/services/doctransformer/test_job/test_integration.py, backend/app/tests/services/doctransformer/test_job/test_start_job.py
Replaced mocking start_low_priority_job with start_doctransform_job. Removed function_path assertions validating executor path.
Response Generation Tests
backend/app/tests/services/response/test_jobs_response.py
Replaced mocking start_high_priority_job with start_response_job. Removed function_path assertion from success test path.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • Feature/celery new execute job #714 — Introduced the new domain-specific job-scheduling functions (e.g., start_llm_job, start_stt_batch_submission) that these test updates now mock.
  • Hotfix/celery import issues #705 — Modified Celery job-scheduling implementation and entrypoints that correspond to the functions being mocked in these updated tests.

Suggested reviewers

  • kartpop
  • Prajna1999

Poem

🐰 Hops through mocks with glee,
Generic jobs fade away,
Specific paths now shine so bright,
Tests refactored, all is right!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Celery: Update testcases mock' is vague and uses non-descriptive phrasing that doesn't clearly convey what specific change was made. Use a more descriptive title that specifies which mocks were updated and why, e.g., 'Update test mocks to use job-specific functions instead of generic priority-based functions' or 'Replace generic job start functions with specific job-type mocks in tests'.
✅ 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 82.35% 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
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch hotfix/celery-testcases-update

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.

@AkhileshNegi AkhileshNegi self-assigned this Mar 26, 2026
@AkhileshNegi AkhileshNegi added the bug Something isn't working label Mar 26, 2026
@AkhileshNegi AkhileshNegi marked this pull request as ready for review March 26, 2026 15:11
@AkhileshNegi AkhileshNegi changed the title fixing testcase by moving to new mock targets Celery: Update testcases mock Mar 26, 2026
Copy link

@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.

🧹 Nitpick comments (1)
backend/app/tests/services/response/test_jobs_response.py (1)

13-13: Missing return type hint.

The function is missing a return type annotation.

Proposed fix
-def test_start_job(db: Session):
+def test_start_job(db: Session) -> None:

As per coding guidelines: "Always add type hints to all function parameters and return values in Python code"

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/app/tests/services/response/test_jobs_response.py` at line 13, The
test function test_start_job currently lacks a return type annotation; update
its signature to include an explicit return type (e.g., def test_start_job(db:
Session) -> None:) so it conforms to the project's typing guidelines and clearly
indicates it returns nothing; ensure the import for Session remains and no other
behavior is changed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@backend/app/tests/services/response/test_jobs_response.py`:
- Line 13: The test function test_start_job currently lacks a return type
annotation; update its signature to include an explicit return type (e.g., def
test_start_job(db: Session) -> None:) so it conforms to the project's typing
guidelines and clearly indicates it returns nothing; ensure the import for
Session remains and no other behavior is changed.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ce50eae9-f172-45b0-9d52-d84676a8caa7

📥 Commits

Reviewing files that changed from the base of the PR and between d94feab and 3a34dfa.

📒 Files selected for processing (9)
  • backend/app/tests/api/routes/test_llm.py
  • backend/app/tests/api/routes/test_stt_evaluation.py
  • backend/app/tests/api/routes/test_tts_evaluation.py
  • backend/app/tests/services/collections/test_create_collection.py
  • backend/app/tests/services/collections/test_delete_collection.py
  • backend/app/tests/services/doctransformer/test_job/test_integration.py
  • backend/app/tests/services/doctransformer/test_job/test_start_job.py
  • backend/app/tests/services/llm/test_jobs.py
  • backend/app/tests/services/response/test_jobs_response.py

@codecov
Copy link

codecov bot commented Mar 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@AkhileshNegi AkhileshNegi merged commit e577369 into main Mar 26, 2026
3 checks passed
@AkhileshNegi AkhileshNegi deleted the hotfix/celery-testcases-update branch March 26, 2026 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants