Added The test suite #30
Conversation
WalkthroughA comprehensive test infrastructure and code quality toolchain has been added to the backend. This includes configuration files for Flake8, Black, and isort, a Makefile for common development tasks, a development requirements file, and multiple scripts for test setup and execution (Windows-compatible). Extensive test suites for models, services, agents, APIs, and utilities have been introduced, along with supporting fixtures and helper functions. The Changes
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (12)
backend/tests/utils/test_helpers.py (2)
9-30: Improve file handling with context manager.The function correctly creates WAV files for testing, but the temporary file handling could be improved.
Apply this diff to use a context manager approach:
def create_test_wav_file(duration_seconds=1, sample_rate=16000): """ Create a temporary WAV file for testing audio processing. Returns: str: Path to the created temporary file """ - temp_file = tempfile.NamedTemporaryFile(suffix='.wav', delete=False) - temp_file.close() + fd, temp_path = tempfile.mkstemp(suffix='.wav') + os.close(fd) # Close the file descriptor # Create a simple sine wave - with wave.open(temp_file.name, 'wb') as wf: + with wave.open(temp_path, 'wb') as wf: wf.setnchannels(1) # Mono wf.setsampwidth(2) # 16-bit wf.setframerate(sample_rate) # Generate simple audio data (silence for testing) frames = sample_rate * duration_seconds for _ in range(frames): wf.writeframesraw(b'\x00\x00') - return temp_file.name + return temp_path
33-38: Simplify exception handling.The cleanup function works correctly but can be simplified using
contextlib.suppress.Apply this diff to use contextlib.suppress:
+from contextlib import suppress + def cleanup_test_file(file_path): """Clean up a temporary test file.""" - try: + with suppress(FileNotFoundError): os.unlink(file_path) - except FileNotFoundError: - passbackend/tests/test models/test_agent.py (2)
4-4: Remove unused import.The pytest import is not used in this test file.
-import pytest from src.models.agent import Agent
8-41: Consider adding more comprehensive test coverage.While the basic tests are good, consider adding tests for:
- Default values (especially voice_id default)
- Field validation and constraints
- Edge cases (empty/null values)
- Model relationships if any exist
backend/Makefile (1)
1-1: Add missing targets to .PHONY declaration.The
test-watchandcleantargets should be included in the .PHONY declaration since they don't create files.-.PHONY: test test-unit test-integration test-coverage lint format install-dev +.PHONY: test test-unit test-integration test-coverage lint format install-dev test-watch cleanbackend/tests/test_api/test_agent.py (1)
4-4: Remove unused import.The
pytestimport is not used in this test file.-import pytest from src.models.agent import Agentbackend/tests/test_agents/test_appointment_setter.py (1)
4-4: Remove unused import.The
pytestimport is not used in this test file.-import pytest from unittest.mock import Mockbackend/instructions.md (1)
50-50: Fix markdown formatting for embedded script.The shell script should be properly formatted as a code block rather than appearing as a malformed heading.
-#!/bin/bash +```bash +#!/bin/bashAnd add a closing code block at the end of the script.
backend/tests/test_api/test_calls.py (1)
4-7: Remove unused imports.Static analysis correctly identifies that
pytestandtempfileare imported but never used in this file.-import pytest from unittest.mock import patch, Mock -import tempfile import osbackend/tests/test services/test_llm_service.py (1)
4-4: Remove unused import.Static analysis correctly identifies that
pytestis imported but never used directly in this file.-import pytest from unittest.mock import Mock, patchbackend/tests/conftest.py (2)
6-6: Remove unused imports.Static analysis correctly identifies unused imports that should be removed for cleaner code.
import tempfile -import os from sqlalchemy import create_enginefrom src.models.agent import Agent -from src.models.call import CallLogAlso applies to: 16-16
72-96: Clarify the purpose of duplicate STT service fixtures.Having both
mock_stt_serviceandmock_stt_service_fixedcreates confusion. The "fixed" version appears more comprehensive, but consider consolidating them or clearly documenting when to use each.If the "fixed" version is the preferred approach, consider removing the original and renaming the fixed version:
-@pytest.fixture -def mock_stt_service(): - """Mock STT service to avoid loading Whisper model.""" - with patch('src.services.stt_service.STTService') as mock: - mock_instance = Mock() - mock_instance.transcribe.return_value = "This is test transcribed text." - mock.return_value = mock_instance - yield mock_instance - - -# Fix the config reference in the STT service file @pytest.fixture -def mock_stt_service_fixed(): - """Mock STT service with correct config references.""" +def mock_stt_service(): + """Mock STT service with WhisperModel and settings mocked."""
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
backend/.flake8(1 hunks)backend/Makefile(1 hunks)backend/instructions.md(1 hunks)backend/project.toml(1 hunks)backend/requirements-dev.txt(1 hunks)backend/setup_tests.py(1 hunks)backend/tests/conftest.py(1 hunks)backend/tests/test models/test_agent.py(1 hunks)backend/tests/test services/test_llm_service.py(1 hunks)backend/tests/test_agents/test_appointment_setter.py(1 hunks)backend/tests/test_api/test_agent.py(1 hunks)backend/tests/test_api/test_calls.py(1 hunks)backend/tests/utils/test_helpers.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
backend/tests/test models/test_agent.py (2)
backend/tests/conftest.py (1)
test_db(32-42)backend/src/models/agent.py (1)
Agent(5-11)
backend/tests/test_api/test_agent.py (2)
backend/tests/conftest.py (3)
client(46-57)sample_agent(132-138)test_db(32-42)backend/src/schemas/agent.py (2)
Agent(12-16)AgentCreate(9-10)
backend/tests/test_agents/test_appointment_setter.py (4)
backend/src/agents/appointment_setter/logic.py (4)
AppointmentSetterAgent(8-30)AppointmentSetterAgent(8-43)__init__(11-20)process_response(30-43)backend/tests/conftest.py (1)
mock_llm_service(62-68)backend/src/services/llm_service.py (1)
get_response(49-62)backend/src/agents/base_agent.py (1)
BaseAgent(5-25)
backend/tests/test services/test_llm_service.py (1)
backend/src/services/llm_service.py (5)
LLMService(5-62)get_response(49-62)LLMService(6-34)__init__(9-20)get_response(22-34)
backend/tests/test_api/test_calls.py (2)
backend/tests/conftest.py (5)
client(46-57)sample_agent(132-138)mock_llm_service(62-68)mock_tts_service(100-106)temp_audio_dir(110-117)backend/src/api/routes/calls.py (3)
call_webhook(26-69)originate_call(79-108)OriginateCallRequest(74-76)
backend/tests/conftest.py (4)
backend/src/core/database.py (1)
get_db(22-31)backend/src/services/llm_service.py (1)
get_response(49-62)backend/src/services/stt_service.py (1)
transcribe(29-42)backend/src/services/tts_service.py (1)
synthesize(25-32)
🪛 Ruff (0.12.2)
backend/tests/utils/test_helpers.py
16-16: Use a context manager for opening files
(SIM115)
35-38: Use contextlib.suppress(FileNotFoundError) instead of try-except-pass
Replace with contextlib.suppress(FileNotFoundError)
(SIM105)
backend/tests/test models/test_agent.py
4-4: pytest imported but unused
Remove unused import: pytest
(F401)
backend/tests/test_api/test_agent.py
4-4: pytest imported but unused
Remove unused import: pytest
(F401)
backend/tests/test_agents/test_appointment_setter.py
4-4: pytest imported but unused
Remove unused import: pytest
(F401)
backend/tests/test services/test_llm_service.py
4-4: pytest imported but unused
Remove unused import: pytest
(F401)
backend/tests/test_api/test_calls.py
4-4: pytest imported but unused
Remove unused import: pytest
(F401)
6-6: tempfile imported but unused
Remove unused import: tempfile
(F401)
backend/tests/conftest.py
6-6: os imported but unused
Remove unused import: os
(F401)
16-16: src.models.call.CallLog imported but unused
Remove unused import: src.models.call.CallLog
(F401)
🪛 checkmake (0.2.2)
backend/Makefile
[warning] 1-1: Missing required phony target "all"
(minphony)
[warning] 1-1: Missing required phony target "clean"
(minphony)
🪛 LanguageTool
backend/instructions.md
[style] ~94-~94: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 1906 characters long)
Context: ...der=80 echo "" echo "✅ All tests passed!" echo "📊 Coverage report generated in htmlcov/" echo "🎉 Voice Marketing Agents is ready for deployment!"
(EN_EXCESSIVE_EXCLAMATION)
🪛 markdownlint-cli2 (0.17.2)
backend/instructions.md
50-50: No space after hash on atx style heading
(MD018, no-missing-space-atx)
🔇 Additional comments (19)
backend/.flake8 (1)
1-14: LGTM! Well-configured flake8 setup.The configuration follows best practices with appropriate exclusions and ignored error codes that align with Black formatter and modern Python conventions.
backend/requirements-dev.txt (1)
1-18: Excellent comprehensive development dependencies.The requirements file includes all necessary tools for a robust testing and development environment. The version constraints are appropriate and the selection covers testing frameworks, data generation, code quality, and database isolation.
backend/tests/test models/test_agent.py (1)
11-27: Good basic CRUD test coverage.The test properly verifies agent creation, database persistence, and field values. The test data and assertions are appropriate.
backend/Makefile (1)
3-44: Excellent development workflow setup.The Makefile provides a well-structured set of targets covering the full development lifecycle: dependency management, testing (unit/integration/coverage), code quality (linting/formatting), and cleanup. The use of pytest markers for test categorization is particularly good practice.
backend/tests/test_api/test_agent.py (1)
8-82: Excellent API test coverage.This test suite provides comprehensive coverage of the agents API endpoints with both positive and negative test cases. The tests are well-structured with clear docstrings, appropriate assertions, and proper use of fixtures. The coverage includes creation, retrieval, listing, validation errors, and not-found scenarios.
backend/tests/test_agents/test_appointment_setter.py (1)
9-72: Comprehensive agent logic testing.Excellent test coverage for the AppointmentSetterAgent class. The tests properly verify initialization, greeting generation, and response processing with both empty and populated conversation history. The detailed verification of message structure passed to the LLM service (lines 46-50, 69-72) demonstrates thorough testing practices.
backend/instructions.md (1)
1-49: Excellent testing documentation.The testing guide is comprehensive and well-structured, covering setup, execution, test organization, and troubleshooting. The coverage requirements and best practices are clearly documented, providing excellent guidance for developers.
backend/setup_tests.py (1)
9-86: Well-structured test environment setup script.Excellent implementation with proper error handling, clear user feedback, and comprehensive environment validation. The script creates a proper Python package structure for tests and handles dependency installation gracefully. The separation of concerns between environment checking, directory creation, and dependency installation makes the code maintainable and easy to understand.
backend/tests/test_api/test_calls.py (5)
13-34: LGTM! Well-structured test for initial webhook call.The test properly mocks dependencies, uses appropriate fixtures, and validates both HTTP response characteristics and TwiML XML structure.
36-60: LGTM! Comprehensive test for speech processing flow.The test effectively validates the webhook's handling of user speech input and correctly verifies the agent interaction with proper parameter passing.
62-70: LGTM! Effective error handling test.The test properly validates the 404 error response for non-existent agents with clear assertions.
72-103: LGTM! Comprehensive test for call origination with proper mocking.The test effectively mocks Twilio client interactions and environment variables, validates response data, and verifies correct API calls. The mock setup is thorough and realistic.
105-117: LGTM! Effective test for missing configuration handling.The test properly validates error response when Twilio environment variables are missing, using appropriate mocking techniques.
backend/tests/conftest.py (6)
20-28: LGTM! Standard test database engine setup.The fixture properly creates an in-memory SQLite engine with appropriate session scoping and table creation.
45-57: LGTM! Proper FastAPI test client setup with dependency override.The fixture correctly overrides the database dependency and ensures cleanup, following FastAPI testing best practices.
61-68: LGTM! Well-designed mock service fixture.The fixture properly mocks the LLMService with predictable responses, avoiding external dependencies during testing.
99-106: LGTM! Appropriate TTS service mock.The fixture properly mocks the TTSService with realistic return values, avoiding external model dependencies.
109-117: LGTM! Excellent temporary directory fixture with proper cleanup.The fixture properly manages temporary audio directory settings with correct restoration, ideal for testing file operations.
121-138: LGTM! Well-structured test data fixtures.The agent data factory and database creation fixtures follow good patterns with proper session management and data persistence.
|
@Adez017 Also attach a demo to show the work of this pr, rest code i will review in a bit |
Hiteshydv001
added
GSSoc'25
Level 3
|
ok |
|
this is the file |
|
okay add a demo i will review again in some time |
Hi @Hiteshydv001 , i think i had added the markdown file and also there is |
|
That all good just add a demo that can verify that tests are working |
You mean any video or commands |
|
Demo video |
ok , i will update it tommorow as i had exams going on |
|
oh no issues do it as per your convinience |
|
here is the video : https://drive.google.com/file/d/1_fOt7P0gePn27XfhzxFM__echJF3LY3x/view?usp=sharing also i would like to share that i am runiing commands like this because there is a conflict in my system for urtc64 python and python 3.13 , you can simple run the files , also the test are failing as i had not installed all the modules . |
|
Also, it is worth mentioning that you may need to update the requirements file according to system compatibility. As of now, on my system, I have made the related changes based on the latest Python version, which is 3.13. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
backend/run_tests_simple.py (2)
53-57: Remove unused variable assignment.The
resultvariable is assigned but never used, as flagged by static analysis.- result = subprocess.run( + subprocess.run( [sys.executable, "-m", "pytest", "-v", "--tb=short"], env=test_env, check=True )
36-47: Consider consolidating directory creation logic.There's duplication between this script and
setup_tests_windows.pyfor creating test directories. Both scripts create similar directory structures.Consider extracting the directory creation logic into a shared utility function to reduce code duplication between
run_tests_simple.pyandsetup_tests_windows.py.backend/setup_tests_windows.py (1)
33-39: Consider consolidating with run_tests_simple.py.The test directory structure creation is duplicated between this script and
run_tests_simple.py(lines 36-47). Both create similar directory structures with slight variations.Consider extracting the common directory creation logic into a shared utility function to reduce code duplication and ensure consistency between the setup and runner scripts.
backend/tests/test_basic.py (1)
4-4: Remove unused pytest import.As flagged by static analysis, the
pytestimport is not used in this module.-import pytestbackend/tests/test_api/test_agents.py (1)
4-4: Remove unused pytest import.As flagged by static analysis, the
pytestimport is not used in this module.-import pytestbackend/tests/conftest.py (3)
12-12: Remove unused imports.Static analysis identified unused imports that should be cleaned up.
-from unittest.mock import Mock, patch +from unittest.mock import Mock- from src.models.call import CallLogAlso applies to: 25-25
54-56: Consider optimizing database cleanup approach.The current approach of dropping and recreating all tables for each test works but may be slower than needed. Consider deleting table contents instead of recreating the schema.
- # Clean up all tables after each test - Base.metadata.drop_all(bind=test_engine) - Base.metadata.create_all(bind=test_engine) + # Clean up all tables after each test + with test_engine.connect() as connection: + for table in reversed(Base.metadata.sorted_tables): + connection.execute(table.delete()) + connection.commit()
59-71: Clean up unnecessary finally block.The empty
finally: passblock in the database override function is unnecessary.def override_get_db(): try: yield test_db - finally: - pass + except Exception: + test_db.rollback() + raiseAlternatively, if no cleanup is needed:
def override_get_db(): - try: - yield test_db - finally: - pass + yield test_db
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
backend/requirements-dev.txt(1 hunks)backend/run_tests_simple.py(1 hunks)backend/setup_tests_windows.py(1 hunks)backend/src/services/stt_service.py(2 hunks)backend/tests/__init__.py(1 hunks)backend/tests/conftest.py(1 hunks)backend/tests/test_api/__init__.py(1 hunks)backend/tests/test_api/test_agents.py(1 hunks)backend/tests/test_basic.py(1 hunks)backend/tests/test_models/__init__.py(1 hunks)backend/tests/test_services/__init__.py(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- backend/tests/init.py
- backend/tests/test_api/init.py
- backend/tests/test_models/init.py
- backend/tests/test_services/init.py
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/requirements-dev.txt
🧰 Additional context used
🧬 Code Graph Analysis (3)
backend/run_tests_simple.py (1)
backend/setup_tests_windows.py (1)
main(50-72)
backend/setup_tests_windows.py (1)
backend/run_tests_simple.py (1)
main(32-62)
backend/tests/test_api/test_agents.py (4)
backend/tests/conftest.py (1)
client(60-71)backend/src/schemas/agent.py (2)
Agent(12-16)AgentCreate(9-10)backend/src/models/agent.py (1)
Agent(5-11)backend/src/api/routes/agents.py (1)
create_agent(14-26)
🪛 Ruff (0.12.2)
backend/run_tests_simple.py
53-53: Local variable result is assigned to but never used
Remove assignment to unused variable result
(F841)
backend/tests/test_basic.py
4-4: pytest imported but unused
Remove unused import: pytest
(F401)
backend/tests/test_api/test_agents.py
4-4: pytest imported but unused
Remove unused import: pytest
(F401)
backend/tests/conftest.py
12-12: unittest.mock.patch imported but unused
Remove unused import: unittest.mock.patch
(F401)
25-25: src.models.call.CallLog imported but unused; consider using importlib.util.find_spec to test for availability
(F401)
🔇 Additional comments (15)
backend/src/services/stt_service.py (3)
9-11: LGTM! Configuration key fix is essential.The correction from
FASTER_WHISPER_MODEL_SIZEtoWHISPER_MODEL_SIZEprevents a potentialKeyErrorwhen accessing settings. The hardcodedcompute_type = "int8"provides consistency for testing environments.
30-36: Good code clarity improvements.The added comments clearly explain the transcription process and segment concatenation, making the code more maintainable.
42-42: Clean error handling simplification.The simplified return statement maintains the same behavior while being more concise.
backend/run_tests_simple.py (1)
8-30: Well-structured environment setup.The function properly configures the Python path and test environment variables. The test configuration values are appropriate for isolated testing.
backend/setup_tests_windows.py (2)
8-27: Comprehensive dependency installation with good error handling.The dependency versions are appropriate and the error handling provides clear feedback to users on installation failures.
29-48: Efficient test structure creation.The directory creation logic is well-implemented with proper init.py file generation for Python package recognition.
backend/tests/test_basic.py (2)
10-19: Excellent foundational test coverage.The basic math and import tests provide good smoke test coverage to ensure the testing environment is properly configured.
21-36: Thorough database fixture validation.The test properly validates the database fixture by creating, persisting, and verifying an Agent instance with all expected properties.
backend/tests/test_api/test_agents.py (3)
10-25: Comprehensive agent creation test.The test properly validates all aspects of agent creation including request data, response status, and returned fields. The test data structure aligns well with the API contract.
27-33: Good edge case coverage for empty list.Testing the empty list scenario ensures the API behaves correctly when no agents exist.
35-41: Proper error handling validation.The 404 test case ensures proper error responses for non-existent resources, which is crucial for API reliability.
backend/tests/conftest.py (4)
32-42: LGTM! Well-designed test database setup.The in-memory SQLite configuration with appropriate connection args is ideal for fast, isolated testing.
74-96: LGTM! Well-designed mock service fixtures.The mock services provide predictable responses while avoiding external dependencies, which is ideal for unit testing.
99-103: LGTM! Proper temporary directory handling.Using
tempfile.TemporaryDirectory()with automatic cleanup is the correct approach for test file management.
106-124: LGTM! Well-structured test data fixtures.The separation between data and persistence, along with proper database session usage, follows testing best practices.
try to create virtual env and we are using docker for everything |
as of now my system is not capable of handling the tasks , as i got it formatted from scratch beacuse of some issue , if its fine for you , could you give it a try |
|
Hi @Adez017 , please look into it |
|
@Adez017 this is also not working use docker to run the application then test it using the logs of the container |
|
How much more time is required @Adez017 ?? |
i need a little of your help to finish this |
2 participants
Hi @Hiteshydv001 , i had added the complete test suite for backend process includeing , api calls , models , configuration , etc .
please take a look and if you had any suggestion please let me know .
closes #7
Summary by CodeRabbit
New Features
Tests
Chores