refactor: address architect review findings (6 items)
R1: Extend @safe to catch ValueError->400, simplify routes_backtest
(eliminated 4 copies of duplicated try/except)
R2: Consolidate PROVIDER constant into obb_utils.py (single source)
R3: Add days_ago() helper to obb_utils.py, replace 8+ duplications
R4: Extract Reddit/ApeWisdom into reddit_service.py from finnhub_service
R5: Fix missing top-level import asyncio in finnhub_service
R6: (deferred - sentiment logic extraction is a larger change)
All 561 tests passing.
This commit is contained in:
@@ -5,6 +5,7 @@ from unittest.mock import patch, AsyncMock, MagicMock
|
||||
import pytest
|
||||
|
||||
import finnhub_service
|
||||
import reddit_service
|
||||
|
||||
|
||||
# --- get_social_sentiment ---
|
||||
@@ -178,7 +179,7 @@ def test_summarize_social_missing_fields():
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("finnhub_service.httpx.AsyncClient")
|
||||
@patch("reddit_service.httpx.AsyncClient")
|
||||
async def test_reddit_sentiment_symbol_found(mock_client_cls):
|
||||
mock_resp = MagicMock()
|
||||
mock_resp.raise_for_status = MagicMock()
|
||||
@@ -195,7 +196,7 @@ async def test_reddit_sentiment_symbol_found(mock_client_cls):
|
||||
mock_client.__aexit__ = AsyncMock(return_value=False)
|
||||
mock_client_cls.return_value = mock_client
|
||||
|
||||
result = await finnhub_service.get_reddit_sentiment("AAPL")
|
||||
result = await reddit_service.get_reddit_sentiment("AAPL")
|
||||
assert result["found"] is True
|
||||
assert result["symbol"] == "AAPL"
|
||||
assert result["rank"] == 3
|
||||
@@ -205,7 +206,7 @@ async def test_reddit_sentiment_symbol_found(mock_client_cls):
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("finnhub_service.httpx.AsyncClient")
|
||||
@patch("reddit_service.httpx.AsyncClient")
|
||||
async def test_reddit_sentiment_symbol_not_found(mock_client_cls):
|
||||
mock_resp = MagicMock()
|
||||
mock_resp.raise_for_status = MagicMock()
|
||||
@@ -221,14 +222,14 @@ async def test_reddit_sentiment_symbol_not_found(mock_client_cls):
|
||||
mock_client.__aexit__ = AsyncMock(return_value=False)
|
||||
mock_client_cls.return_value = mock_client
|
||||
|
||||
result = await finnhub_service.get_reddit_sentiment("AAPL")
|
||||
result = await reddit_service.get_reddit_sentiment("AAPL")
|
||||
assert result["found"] is False
|
||||
assert result["symbol"] == "AAPL"
|
||||
assert "not in Reddit" in result["message"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("finnhub_service.httpx.AsyncClient")
|
||||
@patch("reddit_service.httpx.AsyncClient")
|
||||
async def test_reddit_sentiment_zero_mentions_prev(mock_client_cls):
|
||||
mock_resp = MagicMock()
|
||||
mock_resp.raise_for_status = MagicMock()
|
||||
@@ -244,13 +245,13 @@ async def test_reddit_sentiment_zero_mentions_prev(mock_client_cls):
|
||||
mock_client.__aexit__ = AsyncMock(return_value=False)
|
||||
mock_client_cls.return_value = mock_client
|
||||
|
||||
result = await finnhub_service.get_reddit_sentiment("AAPL")
|
||||
result = await reddit_service.get_reddit_sentiment("AAPL")
|
||||
assert result["found"] is True
|
||||
assert result["mentions_change_pct"] is None # division by zero handled
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("finnhub_service.httpx.AsyncClient")
|
||||
@patch("reddit_service.httpx.AsyncClient")
|
||||
async def test_reddit_sentiment_api_failure(mock_client_cls):
|
||||
mock_client = AsyncMock()
|
||||
mock_client.get.side_effect = Exception("Connection error")
|
||||
@@ -258,13 +259,13 @@ async def test_reddit_sentiment_api_failure(mock_client_cls):
|
||||
mock_client.__aexit__ = AsyncMock(return_value=False)
|
||||
mock_client_cls.return_value = mock_client
|
||||
|
||||
result = await finnhub_service.get_reddit_sentiment("AAPL")
|
||||
result = await reddit_service.get_reddit_sentiment("AAPL")
|
||||
assert result["symbol"] == "AAPL"
|
||||
assert "error" in result
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("finnhub_service.httpx.AsyncClient")
|
||||
@patch("reddit_service.httpx.AsyncClient")
|
||||
async def test_reddit_sentiment_case_insensitive(mock_client_cls):
|
||||
mock_resp = MagicMock()
|
||||
mock_resp.raise_for_status = MagicMock()
|
||||
@@ -280,7 +281,7 @@ async def test_reddit_sentiment_case_insensitive(mock_client_cls):
|
||||
mock_client.__aexit__ = AsyncMock(return_value=False)
|
||||
mock_client_cls.return_value = mock_client
|
||||
|
||||
result = await finnhub_service.get_reddit_sentiment("AAPL")
|
||||
result = await reddit_service.get_reddit_sentiment("AAPL")
|
||||
assert result["found"] is True
|
||||
|
||||
|
||||
@@ -288,7 +289,7 @@ async def test_reddit_sentiment_case_insensitive(mock_client_cls):
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("finnhub_service.httpx.AsyncClient")
|
||||
@patch("reddit_service.httpx.AsyncClient")
|
||||
async def test_reddit_trending_happy_path(mock_client_cls):
|
||||
mock_resp = MagicMock()
|
||||
mock_resp.raise_for_status = MagicMock()
|
||||
@@ -306,7 +307,7 @@ async def test_reddit_trending_happy_path(mock_client_cls):
|
||||
mock_client.__aexit__ = AsyncMock(return_value=False)
|
||||
mock_client_cls.return_value = mock_client
|
||||
|
||||
result = await finnhub_service.get_reddit_trending()
|
||||
result = await reddit_service.get_reddit_trending()
|
||||
assert len(result) == 3
|
||||
assert result[0]["symbol"] == "TSLA"
|
||||
assert result[0]["rank"] == 1
|
||||
@@ -316,7 +317,7 @@ async def test_reddit_trending_happy_path(mock_client_cls):
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("finnhub_service.httpx.AsyncClient")
|
||||
@patch("reddit_service.httpx.AsyncClient")
|
||||
async def test_reddit_trending_limits_to_25(mock_client_cls):
|
||||
mock_resp = MagicMock()
|
||||
mock_resp.raise_for_status = MagicMock()
|
||||
@@ -333,12 +334,12 @@ async def test_reddit_trending_limits_to_25(mock_client_cls):
|
||||
mock_client.__aexit__ = AsyncMock(return_value=False)
|
||||
mock_client_cls.return_value = mock_client
|
||||
|
||||
result = await finnhub_service.get_reddit_trending()
|
||||
result = await reddit_service.get_reddit_trending()
|
||||
assert len(result) == 25
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("finnhub_service.httpx.AsyncClient")
|
||||
@patch("reddit_service.httpx.AsyncClient")
|
||||
async def test_reddit_trending_empty_results(mock_client_cls):
|
||||
mock_resp = MagicMock()
|
||||
mock_resp.raise_for_status = MagicMock()
|
||||
@@ -350,12 +351,12 @@ async def test_reddit_trending_empty_results(mock_client_cls):
|
||||
mock_client.__aexit__ = AsyncMock(return_value=False)
|
||||
mock_client_cls.return_value = mock_client
|
||||
|
||||
result = await finnhub_service.get_reddit_trending()
|
||||
result = await reddit_service.get_reddit_trending()
|
||||
assert result == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("finnhub_service.httpx.AsyncClient")
|
||||
@patch("reddit_service.httpx.AsyncClient")
|
||||
async def test_reddit_trending_api_failure(mock_client_cls):
|
||||
mock_client = AsyncMock()
|
||||
mock_client.get.side_effect = Exception("ApeWisdom down")
|
||||
@@ -363,5 +364,5 @@ async def test_reddit_trending_api_failure(mock_client_cls):
|
||||
mock_client.__aexit__ = AsyncMock(return_value=False)
|
||||
mock_client_cls.return_value = mock_client
|
||||
|
||||
result = await finnhub_service.get_reddit_trending()
|
||||
result = await reddit_service.get_reddit_trending()
|
||||
assert result == []
|
||||
|
||||
@@ -16,7 +16,7 @@ async def client():
|
||||
@pytest.mark.asyncio
|
||||
@patch("routes_sentiment.finnhub_service.get_recommendation_trends", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.openbb_service.get_upgrades_downgrades", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.finnhub_service.get_reddit_sentiment", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.reddit_service.get_reddit_sentiment", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.finnhub_service.get_sentiment_summary", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.alphavantage_service.get_news_sentiment", new_callable=AsyncMock)
|
||||
async def test_stock_sentiment(mock_av, mock_sentiment, mock_reddit, mock_upgrades, mock_recs, client):
|
||||
|
||||
@@ -81,7 +81,7 @@ async def test_stock_social_sentiment_invalid_symbol(client):
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("routes_sentiment.finnhub_service.get_reddit_sentiment", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.reddit_service.get_reddit_sentiment", new_callable=AsyncMock)
|
||||
async def test_stock_reddit_sentiment_found(mock_fn, client):
|
||||
mock_fn.return_value = {
|
||||
"symbol": "AAPL",
|
||||
@@ -104,7 +104,7 @@ async def test_stock_reddit_sentiment_found(mock_fn, client):
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("routes_sentiment.finnhub_service.get_reddit_sentiment", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.reddit_service.get_reddit_sentiment", new_callable=AsyncMock)
|
||||
async def test_stock_reddit_sentiment_not_found(mock_fn, client):
|
||||
mock_fn.return_value = {
|
||||
"symbol": "OBSCURE",
|
||||
@@ -119,7 +119,7 @@ async def test_stock_reddit_sentiment_not_found(mock_fn, client):
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("routes_sentiment.finnhub_service.get_reddit_sentiment", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.reddit_service.get_reddit_sentiment", new_callable=AsyncMock)
|
||||
async def test_stock_reddit_sentiment_service_error_returns_502(mock_fn, client):
|
||||
mock_fn.side_effect = RuntimeError("ApeWisdom down")
|
||||
resp = await client.get("/api/v1/stock/AAPL/reddit-sentiment")
|
||||
@@ -136,7 +136,7 @@ async def test_stock_reddit_sentiment_invalid_symbol(client):
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("routes_sentiment.finnhub_service.get_reddit_trending", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.reddit_service.get_reddit_trending", new_callable=AsyncMock)
|
||||
async def test_reddit_trending_happy_path(mock_fn, client):
|
||||
mock_fn.return_value = [
|
||||
{"rank": 1, "symbol": "TSLA", "name": "Tesla", "mentions_24h": 500, "upvotes": 1200, "rank_24h_ago": 2, "mentions_24h_ago": 400},
|
||||
@@ -154,7 +154,7 @@ async def test_reddit_trending_happy_path(mock_fn, client):
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("routes_sentiment.finnhub_service.get_reddit_trending", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.reddit_service.get_reddit_trending", new_callable=AsyncMock)
|
||||
async def test_reddit_trending_empty(mock_fn, client):
|
||||
mock_fn.return_value = []
|
||||
resp = await client.get("/api/v1/discover/reddit-trending")
|
||||
@@ -163,7 +163,7 @@ async def test_reddit_trending_empty(mock_fn, client):
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("routes_sentiment.finnhub_service.get_reddit_trending", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.reddit_service.get_reddit_trending", new_callable=AsyncMock)
|
||||
async def test_reddit_trending_service_error_returns_502(mock_fn, client):
|
||||
mock_fn.side_effect = RuntimeError("ApeWisdom unavailable")
|
||||
resp = await client.get("/api/v1/discover/reddit-trending")
|
||||
@@ -176,7 +176,7 @@ async def test_reddit_trending_service_error_returns_502(mock_fn, client):
|
||||
@pytest.mark.asyncio
|
||||
@patch("routes_sentiment.finnhub_service.get_recommendation_trends", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.openbb_service.get_upgrades_downgrades", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.finnhub_service.get_reddit_sentiment", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.reddit_service.get_reddit_sentiment", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.finnhub_service.get_sentiment_summary", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.alphavantage_service.get_news_sentiment", new_callable=AsyncMock)
|
||||
async def test_composite_sentiment_all_sources(mock_av, mock_fh, mock_reddit, mock_upgrades, mock_recs, client):
|
||||
@@ -229,7 +229,7 @@ async def test_composite_sentiment_all_sources(mock_av, mock_fh, mock_reddit, mo
|
||||
@pytest.mark.asyncio
|
||||
@patch("routes_sentiment.finnhub_service.get_recommendation_trends", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.openbb_service.get_upgrades_downgrades", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.finnhub_service.get_reddit_sentiment", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.reddit_service.get_reddit_sentiment", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.finnhub_service.get_sentiment_summary", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.alphavantage_service.get_news_sentiment", new_callable=AsyncMock)
|
||||
async def test_composite_sentiment_no_data_returns_unknown(mock_av, mock_fh, mock_reddit, mock_upgrades, mock_recs, client):
|
||||
@@ -250,7 +250,7 @@ async def test_composite_sentiment_no_data_returns_unknown(mock_av, mock_fh, moc
|
||||
@pytest.mark.asyncio
|
||||
@patch("routes_sentiment.finnhub_service.get_recommendation_trends", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.openbb_service.get_upgrades_downgrades", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.finnhub_service.get_reddit_sentiment", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.reddit_service.get_reddit_sentiment", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.finnhub_service.get_sentiment_summary", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.alphavantage_service.get_news_sentiment", new_callable=AsyncMock)
|
||||
async def test_composite_sentiment_strong_bullish_label(mock_av, mock_fh, mock_reddit, mock_upgrades, mock_recs, client):
|
||||
@@ -271,7 +271,7 @@ async def test_composite_sentiment_strong_bullish_label(mock_av, mock_fh, mock_r
|
||||
@pytest.mark.asyncio
|
||||
@patch("routes_sentiment.finnhub_service.get_recommendation_trends", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.openbb_service.get_upgrades_downgrades", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.finnhub_service.get_reddit_sentiment", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.reddit_service.get_reddit_sentiment", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.finnhub_service.get_sentiment_summary", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.alphavantage_service.get_news_sentiment", new_callable=AsyncMock)
|
||||
async def test_composite_sentiment_bearish_label(mock_av, mock_fh, mock_reddit, mock_upgrades, mock_recs, client):
|
||||
@@ -291,7 +291,7 @@ async def test_composite_sentiment_bearish_label(mock_av, mock_fh, mock_reddit,
|
||||
@pytest.mark.asyncio
|
||||
@patch("routes_sentiment.finnhub_service.get_recommendation_trends", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.openbb_service.get_upgrades_downgrades", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.finnhub_service.get_reddit_sentiment", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.reddit_service.get_reddit_sentiment", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.finnhub_service.get_sentiment_summary", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.alphavantage_service.get_news_sentiment", new_callable=AsyncMock)
|
||||
async def test_composite_sentiment_one_source_failing_is_graceful(mock_av, mock_fh, mock_reddit, mock_upgrades, mock_recs, client):
|
||||
@@ -318,7 +318,7 @@ async def test_composite_sentiment_invalid_symbol(client):
|
||||
@pytest.mark.asyncio
|
||||
@patch("routes_sentiment.finnhub_service.get_recommendation_trends", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.openbb_service.get_upgrades_downgrades", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.finnhub_service.get_reddit_sentiment", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.reddit_service.get_reddit_sentiment", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.finnhub_service.get_sentiment_summary", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.alphavantage_service.get_news_sentiment", new_callable=AsyncMock)
|
||||
async def test_composite_sentiment_reddit_low_mentions_excluded(mock_av, mock_fh, mock_reddit, mock_upgrades, mock_recs, client):
|
||||
@@ -338,7 +338,7 @@ async def test_composite_sentiment_reddit_low_mentions_excluded(mock_av, mock_fh
|
||||
@pytest.mark.asyncio
|
||||
@patch("routes_sentiment.finnhub_service.get_recommendation_trends", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.openbb_service.get_upgrades_downgrades", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.finnhub_service.get_reddit_sentiment", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.reddit_service.get_reddit_sentiment", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.finnhub_service.get_sentiment_summary", new_callable=AsyncMock)
|
||||
@patch("routes_sentiment.alphavantage_service.get_news_sentiment", new_callable=AsyncMock)
|
||||
async def test_composite_sentiment_details_structure(mock_av, mock_fh, mock_reddit, mock_upgrades, mock_recs, client):
|
||||
|
||||
Reference in New Issue
Block a user