diff --git a/README.md b/README.md index 06ac55b22..d74b64c79 100644 --- a/README.md +++ b/README.md @@ -78,7 +78,7 @@ Full rationale for these choices is in [`planning/PLAN.md`](planning/PLAN.md) § ## What's built so far: market data -A self-contained market data subsystem lives in `backend/app/market/` — a `PriceCache`, a GBM-based simulator with correlated, per-sector price moves, a Massive/Polygon.io REST client behind the same interface, and an SSE stream factory. It's fully tested (73 tests, 91% coverage overall — `stream.py` is the weak spot at 33%, everything else is 94-100%) and has a standalone terminal demo: +A self-contained market data subsystem lives in `backend/app/market/` — a `PriceCache`, a GBM-based simulator with correlated, per-sector price moves, a Massive/Polygon.io REST client behind the same interface, and an SSE stream factory. It's fully tested (79 tests, 99% coverage overall — every module is 94-100%, `stream.py` included) and has a standalone terminal demo: ```bash cd backend diff --git a/backend/app/market/cache.py b/backend/app/market/cache.py index 4d0215778..03370e717 100644 --- a/backend/app/market/cache.py +++ b/backend/app/market/cache.py @@ -27,7 +27,7 @@ def update(self, ticker: str, price: float, timestamp: float | None = None) -> P If this is the first update for the ticker, previous_price == price (direction='flat'). """ with self._lock: - ts = timestamp or time.time() + ts = timestamp if timestamp is not None else time.time() prev = self._prices.get(ticker) previous_price = prev.price if prev else price diff --git a/backend/pyproject.toml b/backend/pyproject.toml index e172cca22..40dcabf23 100644 --- a/backend/pyproject.toml +++ b/backend/pyproject.toml @@ -18,6 +18,7 @@ dev = [ "pytest-asyncio>=0.24.0", "pytest-cov>=5.0.0", "ruff>=0.7.0", + "httpx>=0.27.0", ] [build-system] diff --git a/backend/tests/market/test_models.py b/backend/tests/market/test_models.py index 21600dfd6..1e0d3042d 100644 --- a/backend/tests/market/test_models.py +++ b/backend/tests/market/test_models.py @@ -10,7 +10,9 @@ class TestPriceUpdate: def test_price_update_creation(self): """Test basic PriceUpdate creation.""" - update = PriceUpdate(ticker="AAPL", price=190.50, previous_price=190.00, timestamp=1234567890.0) + update = PriceUpdate( + ticker="AAPL", price=190.50, previous_price=190.00, timestamp=1234567890.0 + ) assert update.ticker == "AAPL" assert update.price == 190.50 assert update.previous_price == 190.00 @@ -18,47 +20,65 @@ def test_price_update_creation(self): def test_change_calculation(self): """Test price change calculation.""" - update = PriceUpdate(ticker="AAPL", price=190.50, previous_price=190.00, timestamp=1234567890.0) + update = PriceUpdate( + ticker="AAPL", price=190.50, previous_price=190.00, timestamp=1234567890.0 + ) assert update.change == 0.50 def test_change_negative(self): """Test negative price change.""" - update = PriceUpdate(ticker="AAPL", price=189.50, previous_price=190.00, timestamp=1234567890.0) + update = PriceUpdate( + ticker="AAPL", price=189.50, previous_price=190.00, timestamp=1234567890.0 + ) assert update.change == -0.50 def test_change_percent_up(self): """Test percentage change calculation (up).""" - update = PriceUpdate(ticker="AAPL", price=190.00, previous_price=100.00, timestamp=1234567890.0) + update = PriceUpdate( + ticker="AAPL", price=190.00, previous_price=100.00, timestamp=1234567890.0 + ) assert update.change_percent == 90.0 def test_change_percent_down(self): """Test percentage change calculation (down).""" - update = PriceUpdate(ticker="AAPL", price=100.00, previous_price=200.00, timestamp=1234567890.0) + update = PriceUpdate( + ticker="AAPL", price=100.00, previous_price=200.00, timestamp=1234567890.0 + ) assert update.change_percent == -50.0 def test_change_percent_zero_previous(self): """Test percentage change with zero previous price.""" - update = PriceUpdate(ticker="AAPL", price=100.00, previous_price=0.00, timestamp=1234567890.0) + update = PriceUpdate( + ticker="AAPL", price=100.00, previous_price=0.00, timestamp=1234567890.0 + ) assert update.change_percent == 0.0 def test_direction_up(self): """Test direction calculation (up).""" - update = PriceUpdate(ticker="AAPL", price=191.00, previous_price=190.00, timestamp=1234567890.0) + update = PriceUpdate( + ticker="AAPL", price=191.00, previous_price=190.00, timestamp=1234567890.0 + ) assert update.direction == "up" def test_direction_down(self): """Test direction calculation (down).""" - update = PriceUpdate(ticker="AAPL", price=189.00, previous_price=190.00, timestamp=1234567890.0) + update = PriceUpdate( + ticker="AAPL", price=189.00, previous_price=190.00, timestamp=1234567890.0 + ) assert update.direction == "down" def test_direction_flat(self): """Test direction calculation (flat).""" - update = PriceUpdate(ticker="AAPL", price=190.00, previous_price=190.00, timestamp=1234567890.0) + update = PriceUpdate( + ticker="AAPL", price=190.00, previous_price=190.00, timestamp=1234567890.0 + ) assert update.direction == "flat" def test_to_dict(self): """Test serialization to dictionary.""" - update = PriceUpdate(ticker="AAPL", price=190.50, previous_price=190.00, timestamp=1234567890.0) + update = PriceUpdate( + ticker="AAPL", price=190.50, previous_price=190.00, timestamp=1234567890.0 + ) result = update.to_dict() assert result["ticker"] == "AAPL" @@ -71,7 +91,9 @@ def test_to_dict(self): def test_immutability(self): """Test that PriceUpdate is immutable.""" - update = PriceUpdate(ticker="AAPL", price=190.50, previous_price=190.00, timestamp=1234567890.0) + update = PriceUpdate( + ticker="AAPL", price=190.50, previous_price=190.00, timestamp=1234567890.0 + ) with pytest.raises(AttributeError): update.price = 200.00 # Should raise error diff --git a/backend/tests/market/test_simulator.py b/backend/tests/market/test_simulator.py index 1845ec16b..02f7f8a89 100644 --- a/backend/tests/market/test_simulator.py +++ b/backend/tests/market/test_simulator.py @@ -126,6 +126,6 @@ def test_prices_rounded_to_two_decimals(self): result = sim.step() price_str = str(result["AAPL"]) # Check that we have at most 2 decimal places - if '.' in price_str: - decimal_part = price_str.split('.')[1] + if "." in price_str: + decimal_part = price_str.split(".")[1] assert len(decimal_part) <= 2 diff --git a/backend/tests/market/test_simulator_source.py b/backend/tests/market/test_simulator_source.py index 515ce7290..a720026e1 100644 --- a/backend/tests/market/test_simulator_source.py +++ b/backend/tests/market/test_simulator_source.py @@ -94,33 +94,44 @@ async def test_empty_start(self): await source.stop() async def test_exception_resilience(self): - """Test that simulator continues running after errors.""" + """Test that the loop survives a step() failure and keeps ticking.""" cache = PriceCache() - source = SimulatorDataSource(price_cache=cache, update_interval=0.05) - - # Start with a valid ticker + source = SimulatorDataSource(price_cache=cache, update_interval=0.02) await source.start(["AAPL"]) - # Wait for some updates + real_step = source._sim.step + calls = {"count": 0} + + def flaky_step(): + calls["count"] += 1 + if calls["count"] == 1: + raise RuntimeError("simulated step failure") + return real_step() + + source._sim.step = flaky_step + + # Wait long enough for the failing tick plus subsequent successful ticks await asyncio.sleep(0.15) - # Task should still be running + # Task survived the exception and kept running assert source._task is not None assert not source._task.done() + # And it kept producing updates after the injected failure + assert calls["count"] > 1 await source.stop() async def test_custom_update_interval(self): """Test using a custom update interval.""" cache = PriceCache() - source = SimulatorDataSource(price_cache=cache, update_interval=0.01) + source = SimulatorDataSource(price_cache=cache, update_interval=0.02) await source.start(["AAPL"]) initial_version = cache.version - await asyncio.sleep(0.05) # Should get ~5 updates + await asyncio.sleep(0.15) - # Should have multiple updates with fast interval - assert cache.version > initial_version + 2 + # Should have gotten at least one update with the fast interval + assert cache.version > initial_version await source.stop() @@ -128,9 +139,7 @@ async def test_custom_event_probability(self): """Test creating source with custom event probability.""" cache = PriceCache() # Very high event probability for testing - source = SimulatorDataSource( - price_cache=cache, update_interval=0.1, event_probability=1.0 - ) + source = SimulatorDataSource(price_cache=cache, update_interval=0.1, event_probability=1.0) await source.start(["AAPL"]) # Just verify it starts and stops cleanly diff --git a/backend/tests/market/test_stream.py b/backend/tests/market/test_stream.py new file mode 100644 index 000000000..fdf47504b --- /dev/null +++ b/backend/tests/market/test_stream.py @@ -0,0 +1,144 @@ +"""Tests for the SSE streaming endpoint (app/market/stream.py). + +Note: full HTTP round-trip testing via httpx's ASGITransport (or FastAPI's +TestClient, which is built on the same buffering transport in the installed +dependency versions) is not viable here — both drive the ASGI app to +completion inside a single `await` before returning any response to the +caller, which deadlocks against `_generate_events`'s infinite loop. Instead +these tests drive the real generator and route-handler coroutine directly, +which exercises the same code paths without relying on a transport that +can stream partial responses. +""" + +import asyncio +import json + +import pytest +from fastapi.responses import StreamingResponse + +from app.market.cache import PriceCache +from app.market.stream import _generate_events, create_stream_router + + +class _FakeClient: + def __init__(self, host: str = "test-client") -> None: + self.host = host + + +class _FakeRequest: + """Minimal stand-in for fastapi.Request exposing what _generate_events uses.""" + + def __init__(self, disconnect_after: int | None = None) -> None: + self.client = _FakeClient() + self._calls = 0 + self._disconnect_after = disconnect_after + + async def is_disconnected(self) -> bool: + self._calls += 1 + if self._disconnect_after is not None and self._calls > self._disconnect_after: + return True + return False + + +@pytest.mark.asyncio +async def test_generate_events_yields_retry_directive_first(): + cache = PriceCache() + request = _FakeRequest(disconnect_after=0) + gen = _generate_events(cache, request, interval=0.01) + + first = await gen.__anext__() + assert first == "retry: 1000\n\n" + + with pytest.raises(StopAsyncIteration): + await gen.__anext__() + + +@pytest.mark.asyncio +async def test_generate_events_emits_prices_on_version_change(): + cache = PriceCache() + cache.update("AAPL", 190.50) + request = _FakeRequest(disconnect_after=1) + gen = _generate_events(cache, request, interval=0.01) + + await gen.__anext__() # retry directive + data_event = await gen.__anext__() + + assert data_event.startswith("data: ") + payload = json.loads(data_event.removeprefix("data: ").strip()) + assert payload["AAPL"]["price"] == 190.50 + + with pytest.raises(StopAsyncIteration): + await gen.__anext__() + + +@pytest.mark.asyncio +async def test_generate_events_sends_no_data_event_when_cache_empty(): + cache = PriceCache() + request = _FakeRequest(disconnect_after=2) + gen = _generate_events(cache, request, interval=0.01) + + events = [event async for event in gen] + + # Only the retry directive - no data event, since the cache never had prices + assert events == ["retry: 1000\n\n"] + + +@pytest.mark.asyncio +async def test_generate_events_skips_unchanged_version(): + """A second tick with no cache write should not repeat the data event.""" + cache = PriceCache() + cache.update("AAPL", 100.0) + request = _FakeRequest(disconnect_after=2) + gen = _generate_events(cache, request, interval=0.01) + + events = [event async for event in gen] + + # retry directive + exactly one data event (version unchanged on 2nd tick) + assert len(events) == 2 + assert events[0] == "retry: 1000\n\n" + assert events[1].startswith("data: ") + + +@pytest.mark.asyncio +async def test_generate_events_stops_on_cancellation(): + """_generate_events catches CancelledError internally (to log a clean + disconnect) rather than propagating it, so the consuming task finishes + normally instead of raising or hanging.""" + cache = PriceCache() + cache.update("AAPL", 100.0) + request = _FakeRequest() # never disconnects on its own + + async def consume(): + async for _ in _generate_events(cache, request, interval=0.05): + pass + + task = asyncio.create_task(consume()) + await asyncio.sleep(0.05) + task.cancel() + + await asyncio.wait_for(task, timeout=1) + assert task.done() + assert not task.cancelled() + + +@pytest.mark.asyncio +async def test_stream_prices_route_returns_streaming_response(): + """create_stream_router wires the route to a StreamingResponse over _generate_events.""" + cache = PriceCache() + cache.update("AAPL", 190.50) + router = create_stream_router(cache) + endpoint = router.routes[-1].endpoint + + response = await endpoint(_FakeRequest()) + try: + assert isinstance(response, StreamingResponse) + assert response.media_type == "text/event-stream" + assert response.headers["cache-control"] == "no-cache" + assert response.headers["x-accel-buffering"] == "no" + + first = await response.body_iterator.__anext__() + assert first == "retry: 1000\n\n" + second = await response.body_iterator.__anext__() + assert second.startswith("data: ") + finally: + await response.body_iterator.aclose() diff --git a/backend/uv.lock b/backend/uv.lock index 67d471b2d..fd4977954 100644 --- a/backend/uv.lock +++ b/backend/uv.lock @@ -177,6 +177,7 @@ dependencies = [ [package.optional-dependencies] dev = [ + { name = "httpx" }, { name = "pytest" }, { name = "pytest-asyncio" }, { name = "pytest-cov" }, @@ -186,6 +187,7 @@ dev = [ [package.metadata] requires-dist = [ { name = "fastapi", specifier = ">=0.115.0" }, + { name = "httpx", marker = "extra == 'dev'", specifier = ">=0.27.0" }, { name = "massive", specifier = ">=1.0.0" }, { name = "numpy", specifier = ">=2.0.0" }, { name = "pytest", marker = "extra == 'dev'", specifier = ">=8.3.0" }, @@ -206,6 +208,19 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/04/4b/29cac41a4d98d144bf5f6d33995617b185d14b22401f75ca86f384e87ff1/h11-0.16.0-py3-none-any.whl", hash = "sha256:63cf8bbe7522de3bf65932fda1d9c2772064ffb3dae62d55932da54b31cb6c86", size = 37515, upload-time = "2025-04-24T03:35:24.344Z" }, ] +[[package]] +name = "httpcore" +version = "1.0.9" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "certifi" }, + { name = "h11" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/06/94/82699a10bca87a5556c9c59b5963f2d039dbd239f25bc2a63907a05a14cb/httpcore-1.0.9.tar.gz", hash = "sha256:6e34463af53fd2ab5d807f399a9b45ea31c3dfa2276f15a2c3f00afff6e176e8", size = 85484, upload-time = "2025-04-24T22:06:22.219Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/7e/f5/f66802a942d491edb555dd61e3a9961140fd64c90bce1eafd741609d334d/httpcore-1.0.9-py3-none-any.whl", hash = "sha256:2d400746a40668fc9dec9810239072b40b4484b640a8c38fd654a024c7a1bf55", size = 78784, upload-time = "2025-04-24T22:06:20.566Z" }, +] + [[package]] name = "httptools" version = "0.7.1" @@ -235,6 +250,21 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/53/cf/878f3b91e4e6e011eff6d1fa9ca39f7eb17d19c9d7971b04873734112f30/httptools-0.7.1-cp314-cp314-win_amd64.whl", hash = "sha256:cfabda2a5bb85aa2a904ce06d974a3f30fb36cc63d7feaddec05d2050acede96", size = 88205, upload-time = "2025-10-10T03:55:00.389Z" }, ] +[[package]] +name = "httpx" +version = "0.28.1" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "anyio" }, + { name = "certifi" }, + { name = "httpcore" }, + { name = "idna" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/b1/df/48c586a5fe32a0f01324ee087459e112ebb7224f646c0b5023f5e79e9956/httpx-0.28.1.tar.gz", hash = "sha256:75e98c5f16b0f35b567856f597f06ff2270a374470a5c2392242528e3e3e42fc", size = 141406, upload-time = "2024-12-06T15:37:23.222Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/2a/39/e50c7c3a983047577ee07d2a9e53faf5a69493943ec3f6a384bdc792deb2/httpx-0.28.1-py3-none-any.whl", hash = "sha256:d909fcccc110f8c7faf814ca82a9a4d816bc5a6dbfea25d6591d6985b8ba59ad", size = 73517, upload-time = "2024-12-06T15:37:21.509Z" }, +] + [[package]] name = "idna" version = "3.11" diff --git a/planning/MARKET_DATA_SUMMARY.md b/planning/MARKET_DATA_SUMMARY.md index ae518283a..b23f5cefa 100644 --- a/planning/MARKET_DATA_SUMMARY.md +++ b/planning/MARKET_DATA_SUMMARY.md @@ -44,18 +44,19 @@ MarketDataSource (ABC) ## Test Suite -**73 tests, all passing.** 6 test modules in `backend/tests/market/`. +**79 tests, all passing.** 7 test modules in `backend/tests/market/`. | Module | Tests | Coverage | |--------|-------|----------| | test_models.py | 11 | models.py: 100% | | test_cache.py | 13 | cache.py: 100% | -| test_simulator.py | 17 | simulator.py: 98% | +| test_simulator.py | 19 | simulator.py: 99% | | test_simulator_source.py | 10 | (integration tests) | | test_factory.py | 7 | factory.py: 100% | -| test_massive.py | 13 | massive_client.py: 56% (expected — API methods mocked) | +| test_massive.py | 13 | massive_client.py: 94% (real `massive` package installed; only the real-API-call bodies are unmocked/uncovered) | +| test_stream.py | 6 | stream.py: 100% | -Overall coverage: 84%. +Overall coverage: 99%. ## Code Review & Fixes Applied