test: improve test naming and organization for better clarity

- Rename test methods to follow 'should' pattern for better behavior description\n- Combine related test cases to reduce fragmentation\n- Group chat, database, and API tests logically\n- Improve test docstrings and comments for better documentation\n- Maintain same test coverage while reducing code duplication\n\nTest files affected:\n- test_app.py: Group API endpoint tests by functionality\n- test_chat.py: Clarify OpenAI client interaction tests\n- test_database.py: Better describe CRUD operation tests

Author: ksarang90

chatbot-api/.coverage

@@ -7,21 +7,24 @@
 # Load environment variables
 load_dotenv()
 
-# Initialize the client with error checking
-api_key = os.getenv('OPENAI_API_KEY')
-api_base = os.getenv('OPENAI_API_BASE')
+def get_openai_client():
+    # Initialize the client with error checking
+    api_key = os.getenv('OPENAI_API_KEY')
+    api_base = os.getenv('OPENAI_API_BASE')
 
-if not api_key:
-    raise ValueError("OPENAI_API_KEY environment variable is not set")
+    if not api_key:
+        raise ValueError("OPENAI_API_KEY environment variable is not set")
+
+    return OpenAI(
+        api_key=api_key,
+        base_url=api_base  # This is optional, only if you're using a custom API endpoint
+    )
 
 # Initialize the client
-client = OpenAI(
-    api_key=api_key,
-    base_url=api_base  # This is optional, only if you're using a custom API endpoint
-)
+client = get_openai_client()
 
-print(f"API Key present: {bool(api_key)}")
-print(f"API Base present: {bool(api_base)}")
+print(f"API Key present: {bool(os.getenv('OPENAI_API_KEY'))}")
+print(f"API Base present: {bool(os.getenv('OPENAI_API_BASE'))}")
 
 # Function to get bot response using GPT-4
 def get_bot_response(user_message):

chatbot-api/chat.py

@@ -1,4 +1,10 @@
-Flask==2.3.2
+Flask==2.0.1
 flask-cors==3.0.10
 openai>=1.0.0
-python-dotenv==1.0.0
+python-dotenv==1.0.0
+Werkzeug==2.0.1
+
+# Testing dependencies
+pytest==7.4.3
+pytest-cov==4.1.0
+pytest-mock==3.12.0

chatbot-api/requirements.txt

@@ -0,0 +1,21 @@
+import pytest
+import os
+from dotenv import load_dotenv
+
+@pytest.fixture(autouse=True)
+def setup_test_env():
+    """Automatically set up test environment variables before each test"""
+    # Load test environment variables
+    load_dotenv()
+    
+    # Set up test environment variables
+    os.environ['OPENAI_API_KEY'] = 'test-api-key'
+    os.environ['OPENAI_API_BASE'] = 'https://test-api-base.com'
+    
+    yield
+    
+    # Clean up after tests
+    if 'OPENAI_API_KEY' in os.environ:
+        del os.environ['OPENAI_API_KEY']
+    if 'OPENAI_API_BASE' in os.environ:
+        del os.environ['OPENAI_API_BASE'] 

chatbot-api/tests/conftest.py

@@ -0,0 +1,157 @@
+import pytest
+from flask import json
+from app import app
+from unittest.mock import patch, MagicMock
+from datetime import datetime, timezone
+
+@pytest.fixture
+def client():
+    """Create a test client for the Flask app"""
+    app.config['TESTING'] = True
+    with app.test_client() as client:
+        yield client
+
+@pytest.fixture
+def mock_services():
+    """Mock all external services (chat and database)"""
+    with patch('app.get_bot_response') as mock_chat, \
+         patch('app.generate_conversation_name') as mock_name_gen, \
+         patch('app.get_bot_response_stream') as mock_stream, \
+         patch('app.save_conversation') as mock_save, \
+         patch('app.get_conversations') as mock_get_all, \
+         patch('app.get_conversation') as mock_get_one:
+        yield {
+            'chat': mock_chat,
+            'name_gen': mock_name_gen,
+            'stream': mock_stream,
+            'save': mock_save,
+            'get_all': mock_get_all,
+            'get_one': mock_get_one
+        }
+
+def test_chat_api_should_handle_regular_and_streaming_responses(client, mock_services):
+    """Test chat API endpoints handle both regular and streaming responses correctly"""
+    # Test regular chat
+    mock_services['chat'].return_value = "Hello, I'm the assistant"
+    
+    response = client.post('/api/chat', json={
+        'conversation_id': 'test-conv',
+        'messages': [{'role': 'user', 'content': 'Hello'}]
+    })
+    assert response.status_code == 200
+    assert json.loads(response.data)['reply'] == "Hello, I'm the assistant"
+
+    # Test invalid chat requests
+    invalid_payloads = [
+        {'messages': [{'role': 'user', 'content': 'Hello'}]},  # Missing conversation_id
+        {'conversation_id': 'test-conv'},  # Missing messages
+        {'conversation_id': 'test-conv', 'messages': 'not-a-list'}  # Invalid messages format
+    ]
+    for payload in invalid_payloads:
+        response = client.post('/api/chat', json=payload)
+        assert response.status_code == 400
+
+    # Test streaming chat
+    mock_services['stream'].return_value = [
+        {'content': 'Hello'},
+        {'content': ' World'}
+    ]
+    
+    response = client.post('/api/stream_chat', json={
+        'conversation_id': 'test-conv',
+        'messages': [{'role': 'user', 'content': 'Hello'}],
+        'model': 'gpt-4o'
+    })
+    
+    assert response.status_code == 200
+    assert response.headers['Content-Type'] == 'text/event-stream'
+    
+    data = [json.loads(line.decode().split('data: ')[1]) 
+            for line in response.data.split(b'\n\n') 
+            if line.startswith(b'data: ')]
+    
+    assert len(data) == 2
+    assert data[0]['content'] == 'Hello'
+    assert data[1]['content'] == ' World'
+
+    # Test invalid streaming requests
+    invalid_stream_payloads = [
+        {'messages': [{'role': 'user', 'content': 'Hello'}]},  # Missing conversation_id
+        {'conversation_id': 'test-conv'}  # Missing messages
+    ]
+    for payload in invalid_stream_payloads:
+        response = client.post('/api/stream_chat', json=payload)
+        assert response.status_code == 400
+
+def test_conversations_api_should_support_crud_operations(client, mock_services):
+    """Test conversation API endpoints support all CRUD operations correctly"""
+    timestamp = datetime.now(timezone.utc).isoformat()
+    
+    # Test name generation
+    mock_services['name_gen'].return_value = "Test Conversation"
+    response = client.post('/api/generate_name', json={'message': 'Hello'})
+    assert response.status_code == 200
+    assert json.loads(response.data)['name'] == "Test Conversation"
+    
+    # Test invalid name generation request
+    response = client.post('/api/generate_name', json={})
+    assert response.status_code == 400
+
+    # Test saving conversation (Create)
+    test_data = {
+        'conversation_id': 'test-conv',
+        'conversation_name': 'Test Conv',
+        'messages': [{'role': 'user', 'content': 'Hello'}]
+    }
+    
+    response = client.post('/api/conversations', json=test_data)
+    assert response.status_code == 201
+    response_data = json.loads(response.data)
+    assert response_data['conversation_id'] == test_data['conversation_id']
+    assert response_data['conversation_name'] == test_data['conversation_name']
+    assert 'last_updated' in response_data
+
+    # Test invalid save request
+    response = client.post('/api/conversations', json={})
+    assert response.status_code == 400
+
+    # Test retrieving all conversations (Read - List)
+    mock_conversations = [
+        {
+            'conversation_id': 'conv-1',
+            'conversation_name': 'First Conv',
+            'last_updated': timestamp
+        },
+        {
+            'conversation_id': 'conv-2',
+            'conversation_name': 'Second Conv',
+            'last_updated': timestamp
+        }
+    ]
+    mock_services['get_all'].return_value = mock_conversations
+    
+    response = client.get('/api/conversations')
+    assert response.status_code == 200
+    data = json.loads(response.data)
+    assert len(data) == 2
+    assert data[0]['conversation_id'] == 'conv-1'
+    assert data[1]['conversation_id'] == 'conv-2'
+
+    # Test retrieving specific conversation (Read - Single)
+    mock_conversation = {
+        'conversation_id': 'test-conv',
+        'conversation_name': 'Test Conv',
+        'messages': json.dumps([{'role': 'user', 'content': 'Hello'}]),
+        'last_updated': timestamp
+    }
+    mock_services['get_one'].return_value = mock_conversation
+    
+    response = client.get('/api/conversations/test-conv')
+    assert response.status_code == 200
+    data = json.loads(response.data)
+    assert data['conversation_id'] == 'test-conv'
+
+    # Test retrieving non-existent conversation
+    mock_services['get_one'].return_value = None
+    response = client.get('/api/conversations/non-existent')
+    assert response.status_code == 404 

chatbot-api/tests/test_app.py

@@ -0,0 +1,85 @@
+import pytest
+from unittest.mock import patch, MagicMock
+import os
+from chat import get_bot_response, generate_conversation_name, get_bot_response_stream, get_openai_client
+
+@pytest.fixture
+def mock_openai():
+    """Mock OpenAI client for testing"""
+    with patch('chat.client') as mock_client:
+        yield mock_client
+
+def test_client_should_require_api_key():
+    """Test OpenAI client creation requires API key"""
+    with patch.dict(os.environ, {'OPENAI_API_KEY': ''}):
+        with pytest.raises(ValueError, match="OPENAI_API_KEY environment variable is not set"):
+            get_openai_client()
+
+def test_bot_response_should_handle_success_and_errors(mock_openai):
+    """Test bot response handles both successful and error cases"""
+    # Test successful response
+    mock_response = MagicMock()
+    mock_response.choices[0].message.content = "Hello, I'm the assistant"
+    mock_openai.chat.completions.create.return_value = mock_response
+
+    response = get_bot_response("Hello")
+    assert response == "Hello, I'm the assistant"
+    
+    # Verify API parameters
+    mock_openai.chat.completions.create.assert_called_with(
+        model="gpt-4o",
+        messages=[
+            {"role": "system", "content": "You are a helpful assistant."},
+            {"role": "user", "content": "Hello"}
+        ]
+    )
+
+    # Test error handling
+    mock_openai.chat.completions.create.side_effect = Exception("API Error")
+    response = get_bot_response("Hello")
+    assert response == "API Error"
+
+def test_conversation_name_should_be_generated_from_message(mock_openai):
+    """Test conversation name generation from user message"""
+    # Test successful name generation
+    mock_response = MagicMock()
+    mock_response.choices[0].message.content = "Test Conversation Title"
+    mock_openai.chat.completions.create.return_value = mock_response
+
+    name = generate_conversation_name("What is Python?")
+    assert name == "Test Conversation Title"
+    
+    # Verify API parameters
+    call_args = mock_openai.chat.completions.create.call_args[1]
+    assert call_args["model"] == "gpt-4o-mini"
+    assert call_args["max_tokens"] == 10
+    assert call_args["temperature"] == 0.7
+
+    # Test error handling
+    mock_openai.chat.completions.create.side_effect = Exception("API Error")
+    name = generate_conversation_name("What is Python?")
+    assert name == "API Error"
+
+def test_stream_response_should_handle_different_models(mock_openai):
+    """Test streaming responses for different model types and error cases"""
+    # Test streaming response
+    mock_chunk = MagicMock()
+    mock_chunk.choices[0].delta.content = "Hello"
+    mock_openai.chat.completions.create.return_value = [mock_chunk]
+
+    messages = [{"role": "user", "content": "Hi"}]
+    stream = list(get_bot_response_stream(messages, "gpt-4o"))
+    assert stream == [{"content": "Hello"}]
+
+    # Test o1 model (non-streaming)
+    mock_response = MagicMock()
+    mock_response.choices[0].message.content = "Hello from o1"
+    mock_openai.chat.completions.create.return_value = mock_response
+
+    stream = list(get_bot_response_stream(messages, "o1-preview"))
+    assert stream == [{"content": "Hello from o1"}]
+
+    # Test error handling
+    mock_openai.chat.completions.create.side_effect = Exception("Stream Error")
+    stream = list(get_bot_response_stream(messages, "gpt-4o"))
+    assert stream == [{"error": "Stream Error"}] 

chatbot-api/tests/test_chat.py

@@ -0,0 +1,76 @@
+import pytest
+import sqlite3
+import os
+from datetime import datetime
+from database import init_db, save_conversation, get_conversations, get_conversation
+
+# Test database file
+TEST_DB = 'test_conversations.db'
+
+@pytest.fixture(autouse=True)
+def setup_test_db(monkeypatch):
+    """Set up a test database before each test"""
+    monkeypatch.setattr('database.DB_NAME', TEST_DB)
+    init_db()
+    yield
+    if os.path.exists(TEST_DB):
+        os.remove(TEST_DB)
+
+def test_database_should_initialize_with_required_tables():
+    """Test database initialization creates necessary tables"""
+    assert os.path.exists(TEST_DB)
+    
+    conn = sqlite3.connect(TEST_DB)
+    cursor = conn.cursor()
+    cursor.execute("SELECT name FROM sqlite_master WHERE type='table' AND name='conversations'")
+    assert cursor.fetchone() is not None
+    conn.close()
+
+def test_conversation_should_support_create_read_update_operations():
+    """Test conversation storage supports basic CRUD operations (except Delete)"""
+    # Test saving new conversation (Create)
+    conv_id = "test-conv"
+    conv_name = "Test Conversation"
+    messages = '["Hello", "Hi there"]'
+    timestamp = datetime.utcnow().isoformat()
+    
+    save_conversation(conv_id, conv_name, messages, timestamp)
+    
+    # Test retrieving single conversation (Read)
+    result = get_conversation(conv_id)
+    assert result is not None
+    assert result['conversation_id'] == conv_id
+    assert result['conversation_name'] == conv_name
+    assert result['messages'] == messages
+    assert result['last_updated'] == timestamp
+    
+    # Test updating existing conversation (Update)
+    new_name = "Updated Name"
+    new_messages = '["Updated message"]'
+    new_timestamp = datetime.utcnow().isoformat()
+    save_conversation(conv_id, new_name, new_messages, new_timestamp)
+    
+    updated = get_conversation(conv_id)
+    assert updated['conversation_name'] == new_name
+    assert updated['messages'] == new_messages
+    assert updated['last_updated'] == new_timestamp
+    
+    # Test retrieving non-existent conversation
+    assert get_conversation("non-existent") is None
+
+def test_conversations_should_be_retrievable_as_list():
+    """Test retrieving multiple conversations as a list"""
+    # Save test conversations
+    conversations = [
+        ("conv-1", "First Conv", '["Message 1"]', datetime.utcnow().isoformat()),
+        ("conv-2", "Second Conv", '["Message 2"]', datetime.utcnow().isoformat())
+    ]
+    
+    for conv in conversations:
+        save_conversation(*conv)
+    
+    # Test retrieval
+    result = get_conversations()
+    assert len(result) == 2
+    assert result[0]['conversation_id'] == "conv-1"
+    assert result[1]['conversation_id'] == "conv-2"