Skip to content

Commit aa34664

Browse files
costajohntclaude
costajohnt
and
claude
authored
Improve tests: deterministic time, concurrent SQLite, reduce brittle mocks (#99)
* fix: address 3 testing issues (#75, #76, #91) - #75: Make time-dependent tests deterministic by replacing datetime.now() and date.today() with frozen reference times (FROZEN_NOW/FROZEN_DATE) and patching datetime modules in test_cooldown, test_agent_tools, test_shadow_evaluator, test_dashboard_data, test_daily_metrics_extended, and test_equity_tracker_extended. - #76: Add 5 concurrent SQLite access tests using threading in test_state_db: concurrent writes (20 threads), concurrent read/write, same-symbol contention, mixed table operations, and transaction rollback under contention. All verify WAL mode handles concurrent access safely. - #91: Reduce brittle string-based mock patches by adding ScriptPatchContext helper and build_screener_patches/build_momentum_patches factories to conftest.py. Refactored test_screener_trade.py and test_momentum_trade.py to use the new helpers, eliminating ~50-line patch dicts and deeply nested with-blocks. Added shared make_account, make_insider_signal, make_scorer_result factories. All 1134 tests pass. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address 3 issues from PR review - Remove unnecessary mock_dt.strftime assignments in test_dashboard_data (datetime.now() returns a real datetime; .strftime() is called on it, not on the mock class) - Add missing log_trade patch to build_momentum_patches for safety - Simplify TestRunDailyEvaluation: replace 5x try/finally with ExitStack context manager in _frozen_time() Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 64d461f commit aa34664

10 files changed

Lines changed: 737 additions & 487 deletions

‎tests/conftest.py‎

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import sys
44
from pathlib import Path
55
from types import SimpleNamespace
6+
from unittest.mock import MagicMock, patch
67

78
import pytest
89

@@ -46,6 +47,194 @@ def make_position(symbol: str, market_value: float = 1000.0) -> SimpleNamespace:
4647
return SimpleNamespace(symbol=symbol, market_value=str(market_value))
4748

4849

50+
def make_account(equity: float = 100_000, daytrade_count: int = 0) -> SimpleNamespace:
51+
"""Return a lightweight mock that quacks like an Alpaca TradeAccount."""
52+
return SimpleNamespace(equity=str(equity), daytrade_count=daytrade_count)
53+
54+
55+
def make_insider_signal(
56+
rec=None, earnings_in_days=None, reasoning=""
57+
) -> SimpleNamespace:
58+
"""Return a lightweight mock insider signal."""
59+
from strategies.enums import InsiderRecommendation
60+
if rec is None:
61+
rec = InsiderRecommendation.NEUTRAL
62+
return SimpleNamespace(recommendation=rec, earnings_in_days=earnings_in_days, reasoning=reasoning)
63+
64+
65+
def make_sentiment_signal(
66+
score: float = 0.5, confidence: float = 0.8, reasoning: str = "Positive outlook"
67+
) -> SimpleNamespace:
68+
"""Return a lightweight mock sentiment signal."""
69+
return SimpleNamespace(score=score, confidence=confidence, reasoning=reasoning)
70+
71+
72+
def make_scorer_result(
73+
is_meaningful: bool = False, conviction: float = 0.5, bucket: str = "", sample_size: int = 0
74+
) -> SimpleNamespace:
75+
"""Return a lightweight mock signal scorer result."""
76+
return SimpleNamespace(is_meaningful=is_meaningful, conviction=conviction, bucket=bucket, sample_size=sample_size)
77+
78+
79+
# ---------------------------------------------------------------------------
80+
# Script test patch helpers — reduces string-based patch boilerplate
81+
# ---------------------------------------------------------------------------
82+
83+
class ScriptPatchContext:
84+
"""Manages a dict of string-based module patches for script integration tests.
85+
86+
Usage::
87+
88+
ctx = ScriptPatchContext("scripts.screener_trade")
89+
ctx.set("acquire_process_lock", return_value=True)
90+
ctx.set("is_strategy_paused", return_value=False)
91+
with ctx:
92+
from scripts.screener_trade import main
93+
main()
94+
ctx.mock("acquire_process_lock").assert_called_once()
95+
"""
96+
97+
def __init__(self, module_path: str):
98+
self._module = module_path
99+
self._patches: dict[str, MagicMock] = {}
100+
self._active: dict[str, object] = {} # context managers
101+
102+
def set(self, name: str, mock_obj: MagicMock | None = None, **kwargs) -> MagicMock:
103+
"""Register a patch for ``module_path.name``.
104+
105+
If *mock_obj* is given it is used directly, otherwise a new
106+
``MagicMock`` is created from *kwargs* (passed to the MagicMock
107+
constructor).
108+
"""
109+
if mock_obj is None:
110+
mock_obj = MagicMock(**kwargs)
111+
self._patches[name] = mock_obj
112+
return mock_obj
113+
114+
def mock(self, name: str) -> MagicMock:
115+
"""Retrieve the mock registered for *name*."""
116+
return self._patches[name]
117+
118+
def __enter__(self):
119+
for name, mock_obj in self._patches.items():
120+
cm = patch(f"{self._module}.{name}", mock_obj)
121+
self._active[name] = cm
122+
cm.start()
123+
return self
124+
125+
def __exit__(self, *exc_info):
126+
for cm in self._active.values():
127+
cm.stop()
128+
self._active.clear()
129+
130+
131+
def build_screener_patches(
132+
positions=None,
133+
account=None,
134+
candidates=None,
135+
regime=("normal", {}),
136+
cooldowns=None,
137+
paused=False,
138+
healthy=(True, ""),
139+
extra=None,
140+
):
141+
"""Build a ScriptPatchContext for screener_trade.main() with sensible defaults.
142+
143+
Returns a ScriptPatchContext pre-populated with all the standard patches.
144+
Override individual patches via *extra* dict (name -> MagicMock or kwargs).
145+
"""
146+
if positions is None:
147+
positions = []
148+
if account is None:
149+
account = make_account()
150+
if candidates is None:
151+
candidates = []
152+
if cooldowns is None:
153+
cooldowns = {}
154+
155+
mock_tc = MagicMock()
156+
mock_tc.get_all_positions.return_value = positions
157+
mock_tc.get_account.return_value = account
158+
159+
ctx = ScriptPatchContext("scripts.screener_trade")
160+
ctx.set("acquire_process_lock", return_value=True)
161+
ctx.set("is_strategy_paused", return_value=paused)
162+
ctx.set("check_portfolio_health", return_value=healthy)
163+
ctx.set("get_trading_client", return_value=mock_tc)
164+
ctx.set("get_data_client")
165+
ctx.set("get_market_regime", return_value=regime)
166+
symbols = [c.symbol for c in candidates] if candidates else ["TEST"]
167+
ctx.set("get_liquid_symbols", return_value=symbols)
168+
ctx.set("screen_for_dips", return_value=candidates)
169+
ctx.set("load_cooldowns", return_value=cooldowns)
170+
ctx.set("is_on_cooldown", return_value=False)
171+
ctx.set("load_scoring_table", return_value=None)
172+
ctx.set("log_signal")
173+
ctx.set("log_trade")
174+
ctx.set("log_audit_entry")
175+
ctx.set("send_alert")
176+
177+
if extra:
178+
for name, value in extra.items():
179+
if isinstance(value, MagicMock):
180+
ctx.set(name, mock_obj=value)
181+
else:
182+
ctx.set(name, **value) if isinstance(value, dict) else ctx.set(name, return_value=value)
183+
184+
return ctx
185+
186+
187+
def build_momentum_patches(
188+
positions=None,
189+
account=None,
190+
candidates=None,
191+
regime=("normal", {}),
192+
cooldowns=None,
193+
paused=False,
194+
healthy=(True, ""),
195+
extra=None,
196+
):
197+
"""Build a ScriptPatchContext for momentum_trade.main() with sensible defaults."""
198+
if positions is None:
199+
positions = []
200+
if account is None:
201+
account = make_account()
202+
if candidates is None:
203+
candidates = []
204+
if cooldowns is None:
205+
cooldowns = {}
206+
207+
mock_tc = MagicMock()
208+
mock_tc.get_all_positions.return_value = positions
209+
mock_tc.get_account.return_value = account
210+
211+
ctx = ScriptPatchContext("scripts.momentum_trade")
212+
ctx.set("acquire_process_lock", return_value=True)
213+
ctx.set("is_strategy_paused", return_value=paused)
214+
ctx.set("check_portfolio_health", return_value=healthy)
215+
ctx.set("get_trading_client", return_value=mock_tc)
216+
ctx.set("get_data_client")
217+
ctx.set("get_market_regime", return_value=regime)
218+
symbols = [c.symbol for c in candidates] if candidates else ["TEST"]
219+
ctx.set("get_liquid_symbols", return_value=symbols)
220+
ctx.set("screen_for_momentum", return_value=candidates)
221+
ctx.set("load_cooldowns", return_value=cooldowns)
222+
ctx.set("is_on_cooldown", return_value=False)
223+
ctx.set("log_signal")
224+
ctx.set("log_trade")
225+
ctx.set("log_audit_entry")
226+
ctx.set("send_alert")
227+
228+
if extra:
229+
for name, value in extra.items():
230+
if isinstance(value, MagicMock):
231+
ctx.set(name, mock_obj=value)
232+
else:
233+
ctx.set(name, **value) if isinstance(value, dict) else ctx.set(name, return_value=value)
234+
235+
return ctx
236+
237+
49238
@pytest.fixture
50239
def single_tech_position():
51240
"""One Technology position (AAPL)."""

‎tests/test_agent_tools.py‎

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77

88
import pytest
99

10+
# Fixed reference time so tests are deterministic regardless of when they run.
11+
FROZEN_NOW = datetime(2026, 6, 15, 12, 0, 0)
12+
1013
from scripts.agent_tools import (
1114
_HARDCODED_CHANGE_BUDGET,
1215
_HARDCODED_COOLING_PERIOD_DAYS,
@@ -77,7 +80,7 @@ def sample_changelog(agent_dir):
7780
changelog = {
7881
"changes": [{
7982
"id": "chg-test-001",
80-
"timestamp": datetime.now().isoformat(),
83+
"timestamp": FROZEN_NOW.isoformat(),
8184
"type": "parameter_adjustment",
8285
"status": "deployed",
8386
"commit_sha": "abc123",
@@ -231,13 +234,17 @@ def test_read_code_blocks_absolute_path(self, agent_dir):
231234
class TestDeployChangeConstraints:
232235
"""deploy_change enforces budget, samples, and safety invariants."""
233236

234-
def test_rejects_when_budget_exhausted(self, agent_dir):
237+
@patch("scripts.agent_writes._get_week_key")
238+
def test_rejects_when_budget_exhausted(self, mock_week_key, agent_dir):
239+
# Use the ISO week key for FROZEN_NOW so the deployed entries match
240+
frozen_week = f"{FROZEN_NOW.isocalendar()[0]}-W{FROZEN_NOW.isocalendar()[1]:02d}"
241+
mock_week_key.return_value = frozen_week
235242
# Create changelog with _HARDCODED_CHANGE_BUDGET deploys this week
236243
changelog = {"changes": []}
237244
for i in range(_HARDCODED_CHANGE_BUDGET):
238245
changelog["changes"].append({
239246
"id": f"chg-{i}",
240-
"timestamp": datetime.now().isoformat(),
247+
"timestamp": FROZEN_NOW.isoformat(),
241248
"status": "deployed",
242249
"commit_sha": f"sha{i}",
243250
})
@@ -279,7 +286,7 @@ def test_refuses_non_agent_commit(self, agent_dir):
279286
def test_refuses_already_rolled_back(self, agent_dir):
280287
changelog = {"changes": [{
281288
"id": "chg-1",
282-
"timestamp": datetime.now().isoformat(),
289+
"timestamp": FROZEN_NOW.isoformat(),
283290
"status": "rolled_back",
284291
"commit_sha": "abc123",
285292
}]}
@@ -380,14 +387,17 @@ def test_pause_invalid_strategy(self, agent_dir):
380387
assert result["success"] is False
381388
assert "Unknown strategy" in result["error"]
382389

383-
def test_pause_expires(self, agent_dir):
390+
@patch("scripts.agent_writes.datetime")
391+
def test_pause_expires(self, mock_dt, agent_dir):
392+
mock_dt.now.return_value = FROZEN_NOW
393+
mock_dt.fromisoformat = datetime.fromisoformat
384394
# Write an already-expired pause
385395
pause_file = agent_dir / "pause-screener.json"
386396
pause_file.write_text(json.dumps({
387397
"strategy": "screener",
388398
"reason": "test",
389-
"paused_at": (datetime.now() - timedelta(days=4)).isoformat(),
390-
"expires_at": (datetime.now() - timedelta(days=1)).isoformat(),
399+
"paused_at": (FROZEN_NOW - timedelta(days=4)).isoformat(),
400+
"expires_at": (FROZEN_NOW - timedelta(days=1)).isoformat(),
391401
}))
392402
assert is_strategy_paused("screener") is False
393403

@@ -936,13 +946,13 @@ def test_failed_deploys_dont_count_toward_budget(self, agent_dir):
936946
changelog = {"changes": [
937947
{
938948
"id": "chg-failed",
939-
"timestamp": datetime.now().isoformat(),
949+
"timestamp": FROZEN_NOW.isoformat(),
940950
"status": "failed",
941951
"commit_sha": None,
942952
},
943953
{
944954
"id": "chg-pending",
945-
"timestamp": datetime.now().isoformat(),
955+
"timestamp": FROZEN_NOW.isoformat(),
946956
"status": "pending",
947957
"commit_sha": None,
948958
},
@@ -962,7 +972,7 @@ def test_rollback_uses_no_commit(self, agent_dir):
962972
"""Rollback passes --no-commit flag to git revert."""
963973
changelog = {"changes": [{
964974
"id": "chg-1",
965-
"timestamp": datetime.now().isoformat(),
975+
"timestamp": FROZEN_NOW.isoformat(),
966976
"status": "deployed",
967977
"commit_sha": "abc123",
968978
"files_modified": ["strategies/constants.py"],
@@ -987,14 +997,14 @@ def test_rollback_single_commit_for_multiple_reverts(self, agent_dir):
987997
changelog = {"changes": [
988998
{
989999
"id": "chg-1",
990-
"timestamp": (datetime.now() - timedelta(hours=2)).isoformat(),
1000+
"timestamp": (FROZEN_NOW - timedelta(hours=2)).isoformat(),
9911001
"status": "deployed",
9921002
"commit_sha": "abc123",
9931003
"files_modified": ["strategies/constants.py"],
9941004
},
9951005
{
9961006
"id": "chg-2",
997-
"timestamp": datetime.now().isoformat(),
1007+
"timestamp": FROZEN_NOW.isoformat(),
9981008
"status": "deployed",
9991009
"commit_sha": "def456",
10001010
"files_modified": ["strategies/constants.py"],
@@ -1019,7 +1029,7 @@ def test_rollback_aborts_cleanly_on_revert_failure(self, agent_dir):
10191029
"""If a revert fails, all staged changes are reset."""
10201030
changelog = {"changes": [{
10211031
"id": "chg-1",
1022-
"timestamp": datetime.now().isoformat(),
1032+
"timestamp": FROZEN_NOW.isoformat(),
10231033
"status": "deployed",
10241034
"commit_sha": "abc123",
10251035
"files_modified": ["strategies/constants.py"],

0 commit comments

Comments
 (0)