mirror of
https://github.com/langbot-app/LangBot.git
synced 2026-08-11 13:20:58 +00:00
test(quality): fix fake tests and add missing coverage
P0 fixes: - telemetry: rewrite fake tests with real behavior verification (25 tests) - config: delete copied-source tests, use proper imports (2 deleted) - persistence: fix try-except pass to verify specific errors P1 fixes: - pipeline: add real FixedWindowAlgo tests instead of mocks (12 tests) - provider: add SessionManager and ToolManager tests (25 tests) - storage: add S3StorageProvider tests with moto mock (16 tests) - plugin: add handler action tests for setting inheritance (15 tests) - rag: add file storage and ZIP processing tests (21 tests) - vector: add VDB filter conversion tests (30 tests) P2 fixes: - pipeline/msgtrun: strengthen assertions for exact message count - api: add response structure validation in integration tests New test files: - provider/test_session_manager.py - provider/test_tool_manager.py - storage/test_s3storage.py - plugin/test_handler_actions.py - rag/test_file_storage.py - vector/test_vdb_filter_conversion.py Source code bugs documented: - provider: TokenManager.next_token() ZeroDivisionError - telemetry: send_tasks class variable shared state - command: empty command IndexError, unused parameters - utils: funcschema KeyError - entity: vector.py independent declarative_base Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -133,7 +133,15 @@ class TestRoundTruncatorProcess:
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_truncate_exceeds_limit(self):
|
||||
"""Messages exceeding max-round should be truncated."""
|
||||
"""Messages exceeding max-round should be truncated precisely.
|
||||
|
||||
Algorithm: traverse backwards, collect while current_round < max_round, count user messages as rounds.
|
||||
For max_round=2 with 7 messages (u1, a1, u2, a2, u3, a3, u_current):
|
||||
- Iterate: u_current(r=0<2, collect, r=1), a3(r=1<2, collect), u3(r=1<2, collect, r=2)
|
||||
- a2: r=2 not < 2 → break
|
||||
- Collected reverse: [u_current, a3, u3]
|
||||
- Reversed: [u3, a3, u_current] = 3 messages
|
||||
"""
|
||||
msgtrun = get_msgtrun_module()
|
||||
entities = get_entities_module()
|
||||
|
||||
@@ -145,6 +153,7 @@ class TestRoundTruncatorProcess:
|
||||
await stage.initialize(pipeline_config)
|
||||
|
||||
# Create query with many messages exceeding limit
|
||||
# 7 messages = 3 full rounds + 1 current user
|
||||
query = text_query("current message")
|
||||
query.pipeline_config = pipeline_config
|
||||
query.messages = [
|
||||
@@ -160,9 +169,17 @@ class TestRoundTruncatorProcess:
|
||||
result = await stage.process(query, 'ConversationMessageTruncator')
|
||||
|
||||
assert result.result_type == entities.ResultType.CONTINUE
|
||||
# Should only keep last 2 rounds (2 user messages)
|
||||
# Each round = user + assistant, so 2 rounds = 4 messages + current = 5
|
||||
assert len(result.new_query.messages) <= 5
|
||||
# Should keep exactly 3 messages: message3, response3, current message
|
||||
messages = result.new_query.messages
|
||||
assert len(messages) == 3
|
||||
|
||||
# Verify exact message content
|
||||
assert messages[0].role == 'user'
|
||||
assert messages[0].content == 'message 3'
|
||||
assert messages[1].role == 'assistant'
|
||||
assert messages[1].content == 'response 3'
|
||||
assert messages[2].role == 'user'
|
||||
assert messages[2].content == 'current message'
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_truncate_empty_messages(self):
|
||||
|
||||
@@ -5,6 +5,8 @@ Tests the actual RateLimit implementation from pkg.pipeline.ratelimit
|
||||
"""
|
||||
|
||||
import pytest
|
||||
import asyncio
|
||||
import time
|
||||
from unittest.mock import AsyncMock, Mock, patch
|
||||
from importlib import import_module
|
||||
import langbot_plugin.api.entities.builtin.provider.session as provider_session
|
||||
@@ -19,6 +21,284 @@ def get_modules():
|
||||
return ratelimit, entities, algo_module
|
||||
|
||||
|
||||
def get_fixedwin_module():
|
||||
"""Lazy import of FixedWindowAlgo"""
|
||||
return import_module('langbot.pkg.pipeline.ratelimit.algos.fixedwin')
|
||||
|
||||
|
||||
class TestFixedWindowAlgo:
|
||||
"""Tests for the actual FixedWindowAlgo implementation.
|
||||
|
||||
IMPORTANT: These tests verify the real algorithm logic, not mocks.
|
||||
"""
|
||||
|
||||
@pytest.fixture
|
||||
def mock_app_for_algo(self):
|
||||
"""Create mock app for algorithm initialization."""
|
||||
mock_app = Mock()
|
||||
mock_app.logger = Mock()
|
||||
return mock_app
|
||||
|
||||
@pytest.fixture
|
||||
def sample_query_with_rate_limit(self, sample_query):
|
||||
"""Create query with rate limit configuration."""
|
||||
sample_query.pipeline_config = {
|
||||
'safety': {
|
||||
'rate-limit': {
|
||||
'window-length': 60, # 60 seconds window
|
||||
'limitation': 10, # 10 requests per window
|
||||
'strategy': 'drop',
|
||||
}
|
||||
}
|
||||
}
|
||||
return sample_query
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fixedwin_algo_initialization(self, mock_app_for_algo):
|
||||
"""Test that FixedWindowAlgo initializes correctly."""
|
||||
fixedwin = get_fixedwin_module()
|
||||
|
||||
algo = fixedwin.FixedWindowAlgo(mock_app_for_algo)
|
||||
await algo.initialize()
|
||||
|
||||
assert algo.containers_lock is not None
|
||||
assert algo.containers == {}
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fixedwin_within_limit_returns_true(self, mock_app_for_algo, sample_query_with_rate_limit):
|
||||
"""Test that requests within limit are allowed."""
|
||||
fixedwin = get_fixedwin_module()
|
||||
|
||||
algo = fixedwin.FixedWindowAlgo(mock_app_for_algo)
|
||||
await algo.initialize()
|
||||
|
||||
# Make requests within limit
|
||||
for i in range(10):
|
||||
result = await algo.require_access(
|
||||
sample_query_with_rate_limit,
|
||||
provider_session.LauncherTypes.PERSON,
|
||||
'12345'
|
||||
)
|
||||
assert result is True, f"Request {i+1} should be allowed"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fixedwin_exceeds_limit_drop_strategy(self, mock_app_for_algo, sample_query_with_rate_limit):
|
||||
"""Test that exceeding limit with 'drop' strategy returns False."""
|
||||
fixedwin = get_fixedwin_module()
|
||||
|
||||
algo = fixedwin.FixedWindowAlgo(mock_app_for_algo)
|
||||
await algo.initialize()
|
||||
|
||||
# Exhaust the limit
|
||||
for i in range(10):
|
||||
await algo.require_access(
|
||||
sample_query_with_rate_limit,
|
||||
provider_session.LauncherTypes.PERSON,
|
||||
'12345'
|
||||
)
|
||||
|
||||
# Next request should be denied
|
||||
result = await algo.require_access(
|
||||
sample_query_with_rate_limit,
|
||||
provider_session.LauncherTypes.PERSON,
|
||||
'12345'
|
||||
)
|
||||
|
||||
assert result is False, "Request exceeding limit should be denied"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fixedwin_different_sessions_isolated(self, mock_app_for_algo, sample_query_with_rate_limit):
|
||||
"""Test that different sessions have independent rate limits."""
|
||||
fixedwin = get_fixedwin_module()
|
||||
|
||||
algo = fixedwin.FixedWindowAlgo(mock_app_for_algo)
|
||||
await algo.initialize()
|
||||
|
||||
# Exhaust limit for session 1
|
||||
for i in range(10):
|
||||
await algo.require_access(
|
||||
sample_query_with_rate_limit,
|
||||
provider_session.LauncherTypes.PERSON,
|
||||
'session1'
|
||||
)
|
||||
|
||||
# Session 2 should still have its own limit
|
||||
result = await algo.require_access(
|
||||
sample_query_with_rate_limit,
|
||||
provider_session.LauncherTypes.PERSON,
|
||||
'session2'
|
||||
)
|
||||
|
||||
assert result is True, "Different session should have independent limit"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fixedwin_limit_one_request(self, mock_app_for_algo, sample_query):
|
||||
"""Test with limitation=1 allows only one request."""
|
||||
fixedwin = get_fixedwin_module()
|
||||
|
||||
sample_query.pipeline_config = {
|
||||
'safety': {
|
||||
'rate-limit': {
|
||||
'window-length': 60,
|
||||
'limitation': 1, # Only 1 request allowed
|
||||
'strategy': 'drop',
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
algo = fixedwin.FixedWindowAlgo(mock_app_for_algo)
|
||||
await algo.initialize()
|
||||
|
||||
# First request allowed
|
||||
result1 = await algo.require_access(
|
||||
sample_query,
|
||||
provider_session.LauncherTypes.PERSON,
|
||||
'12345'
|
||||
)
|
||||
assert result1 is True
|
||||
|
||||
# Second request denied
|
||||
result2 = await algo.require_access(
|
||||
sample_query,
|
||||
provider_session.LauncherTypes.PERSON,
|
||||
'12345'
|
||||
)
|
||||
assert result2 is False
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fixedwin_container_persists(self, mock_app_for_algo, sample_query_with_rate_limit):
|
||||
"""Test that container is created and persists across requests."""
|
||||
fixedwin = get_fixedwin_module()
|
||||
|
||||
algo = fixedwin.FixedWindowAlgo(mock_app_for_algo)
|
||||
await algo.initialize()
|
||||
|
||||
# First request creates container
|
||||
await algo.require_access(
|
||||
sample_query_with_rate_limit,
|
||||
provider_session.LauncherTypes.PERSON,
|
||||
'12345'
|
||||
)
|
||||
|
||||
# Key format: 'LauncherTypes.PERSON_12345' (enum string representation)
|
||||
expected_key = 'LauncherTypes.PERSON_12345'
|
||||
assert expected_key in algo.containers
|
||||
container = algo.containers[expected_key]
|
||||
|
||||
# Container should have records
|
||||
assert len(container.records) > 0
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fixedwin_new_window_clears_records(self, mock_app_for_algo, sample_query):
|
||||
"""Test that a new time window starts fresh records.
|
||||
|
||||
This test verifies the window calculation logic:
|
||||
- Records are keyed by window start timestamp
|
||||
- When window advances, new key is created
|
||||
"""
|
||||
fixedwin = get_fixedwin_module()
|
||||
|
||||
# Use a very short window for testing
|
||||
sample_query.pipeline_config = {
|
||||
'safety': {
|
||||
'rate-limit': {
|
||||
'window-length': 1, # 1 second window for fast test
|
||||
'limitation': 5,
|
||||
'strategy': 'drop',
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
algo = fixedwin.FixedWindowAlgo(mock_app_for_algo)
|
||||
await algo.initialize()
|
||||
|
||||
# Make requests in current window
|
||||
now = int(time.time())
|
||||
window_start = now - now % 1
|
||||
|
||||
for i in range(5):
|
||||
await algo.require_access(sample_query, provider_session.LauncherTypes.PERSON, 'test')
|
||||
|
||||
# Key format: 'LauncherTypes.PERSON_test'
|
||||
expected_key = 'LauncherTypes.PERSON_test'
|
||||
container = algo.containers[expected_key]
|
||||
assert window_start in container.records
|
||||
assert container.records[window_start] == 5
|
||||
|
||||
# Wait for next window (1 second)
|
||||
await asyncio.sleep(1.1)
|
||||
|
||||
# New request should be allowed (new window)
|
||||
result = await algo.require_access(sample_query, provider_session.LauncherTypes.PERSON, 'test')
|
||||
assert result is True, "New window should allow new requests"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fixedwin_wait_strategy_blocks_until_next_window(self, mock_app_for_algo, sample_query):
|
||||
"""Test that 'wait' strategy blocks until next window.
|
||||
|
||||
NOTE: This test is timing-sensitive and may take ~1 second.
|
||||
"""
|
||||
fixedwin = get_fixedwin_module()
|
||||
|
||||
# Use 1-second window for testability
|
||||
sample_query.pipeline_config = {
|
||||
'safety': {
|
||||
'rate-limit': {
|
||||
'window-length': 1,
|
||||
'limitation': 1, # Only 1 request per second
|
||||
'strategy': 'wait',
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
algo = fixedwin.FixedWindowAlgo(mock_app_for_algo)
|
||||
await algo.initialize()
|
||||
|
||||
# First request allowed
|
||||
start_time = time.time()
|
||||
result1 = await algo.require_access(
|
||||
sample_query,
|
||||
provider_session.LauncherTypes.PERSON,
|
||||
'wait_test'
|
||||
)
|
||||
assert result1 is True
|
||||
|
||||
# Exhaust limit
|
||||
await algo.require_access(sample_query, provider_session.LauncherTypes.PERSON, 'wait_test')
|
||||
|
||||
# Third request should wait and then succeed
|
||||
result3 = await algo.require_access(
|
||||
sample_query,
|
||||
provider_session.LauncherTypes.PERSON,
|
||||
'wait_test'
|
||||
)
|
||||
elapsed = time.time() - start_time
|
||||
|
||||
assert result3 is True, "After wait, request should succeed"
|
||||
# Should have waited approximately until next window
|
||||
# With 1-second window, elapsed should be > 1 second
|
||||
assert elapsed >= 1.0, f"Should have waited for next window, elapsed={elapsed:.2f}s"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_fixedwin_release_access(self, mock_app_for_algo, sample_query_with_rate_limit):
|
||||
"""Test that release_access does nothing (current implementation)."""
|
||||
fixedwin = get_fixedwin_module()
|
||||
|
||||
algo = fixedwin.FixedWindowAlgo(mock_app_for_algo)
|
||||
await algo.initialize()
|
||||
|
||||
# release_access is empty in current implementation
|
||||
await algo.release_access(
|
||||
sample_query_with_rate_limit,
|
||||
provider_session.LauncherTypes.PERSON,
|
||||
'12345'
|
||||
)
|
||||
|
||||
# Should not raise or change state
|
||||
assert 'person_12345' not in algo.containers
|
||||
|
||||
|
||||
# Original mock-based tests for RateLimit stage integration
|
||||
@pytest.mark.asyncio
|
||||
async def test_require_access_allowed(mock_app, sample_query):
|
||||
"""Test RequireRateLimitOccupancy allows access when rate limit is not exceeded"""
|
||||
|
||||
Reference in New Issue
Block a user