test(hip-3-pusher): add comprehensive unit tests for price state and listeners #3242
Conversation
…listeners Add new tests covering: - PriceUpdate and PriceSourceState classes - OracleUpdate dataclass - ConstantSourceConfig for constant price values - OracleMidAverageConfig for oracle-mid average calculations - Mark and external price configurations - Multiple symbols handling - Pair source edge cases (missing/stale base/quote) - SEDA price source - Exponent conversion - Invalid source config error handling - Hermes, Lazer, Hyperliquid, and SEDA listener message parsing - Subscribe request format validation Increases test count from 4 to 53 tests. Co-Authored-By: Mike Rolish <[email protected]>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| "invalid_key": "invalid_value" | ||
| } | ||
|
|
||
| listener.parse_hermes_message(message) |
There was a problem hiding this comment.
parse_hermes_message swallows all exceptions, so i'm not sure these tests are testing much
There was a problem hiding this comment.
Good point! I've updated the docstring to clarify that since parse_hermes_message swallows exceptions internally, this test verifies the observable behavior (no state mutation on bad input) rather than exception handling. The test still documents the expected behavior even if it can't verify the internal exception path.
| assert update8 is not None | ||
| assert update8.price == "99000000" | ||
|
|
||
| def test_parse_lazer_message_missing_price(self): |
There was a problem hiding this comment.
I would also test for the price: None scenario since we recently changed lazer behavior to emit null prices if no agg is available.
There was a problem hiding this comment.
Added test_parse_lazer_message_null_price to test the explicit price: None scenario. The test verifies that feeds with null prices are skipped (not stored in state) while other valid feeds in the same message are still processed correctly.
| class MockSedaListener: | ||
| def __init__(self): | ||
| self.seda_state = seda_state | ||
|
|
||
| listener = MockSedaListener() |
There was a problem hiding this comment.
There's a bunch of repetition in these tests, pls extract the mocks and setups into fixtures. The fixture factory pattern is also useful for creating mock data generators if you need slightly varied mock data based on some parameters.
There was a problem hiding this comment.
Done! Extracted MockSedaListener class and seda_listener_fixture pytest fixture to reduce repetition in the SEDA tests.
| expected_timestamp = datetime.datetime.fromisoformat("2024-01-15T12:00:00.000Z").timestamp() | ||
| assert update.timestamp == expected_timestamp | ||
|
|
||
| def test_parse_seda_message_different_timestamp_format(self): |
There was a problem hiding this comment.
This is a curious test :) is this something we've dealt with from SEDA before?
There was a problem hiding this comment.
Not based on a specific SEDA issue - I added it to verify that Python's fromisoformat() handles various ISO 8601 formats including timestamps with microseconds. Updated the docstring to clarify this is testing the parser's flexibility with different precision levels that SEDA may emit.
| source_type = "invalid" | ||
|
|
||
| with pytest.raises(ValueError): | ||
| price_state.get_price(InvalidConfig(), OracleUpdate({}, {}, {})) |
There was a problem hiding this comment.
Can we enforce this at config load time? It wouldn't be great to discover a fallback source has a bad config at runtime when we actually need it lol
There was a problem hiding this comment.
Good suggestion! Enforcing invalid source config at config load time would be cleaner than raising at runtime. That's outside the scope of this test PR, but it would be a good follow-up improvement to the config validation logic.
There was a problem hiding this comment.
Verifying the source type is in an enumeration of what we've set up is trivial enough. Checking that that specific symbol is being listened for is a bit more work. Let me see what I can add.
- Update Hermes malformed test docstring to clarify it tests observable behavior - Add test for Lazer explicit null price scenario - Extract SEDA test setup into pytest fixture to reduce repetition - Update SEDA timestamp format test docstring to explain fromisoformat behavior Co-Authored-By: Mike Rolish <[email protected]>
Summary
Adds comprehensive unit tests for the hip-3-pusher Python app, expanding test coverage from 4 tests to 54 tests. This includes:
test_listeners.pycovering message parsing for Hermes, Lazer, Hyperliquid, and SEDA listenerstest_price_state.pywith tests for PriceUpdate, PriceSourceState, OracleUpdate classes, ConstantSourceConfig, OracleMidAverageConfig, mark/external prices, multiple symbols, pair source edge cases, SEDA source, exponent conversion, and error handlingRationale
The existing test file only had 4 basic tests covering the price fallback waterfall. This PR adds coverage for the core data classes, all price source configuration types, listener message parsing, and edge cases like missing/stale prices.
How has this been tested?
All 54 tests pass locally with
uv run pytest tests/ -v.Important notes for reviewers
API usage fix: The existing tests used
oracle_px, _, _ = price_state.get_all_prices(DEX)but the actualget_all_prices()method takes no arguments and returns anOracleUpdateobject. The tests have been updated to use the correct API.SEDA listener tests: These use a MockSedaListener class to test
_parse_seda_messagedirectly since the actual listener requires file paths for API keys.Updates since last revision
Addressed reviewer feedback:
test_parse_lazer_message_null_priceto test explicitprice: Nonescenario (Lazer emits null prices when no aggregation is available)seda_listener_fixture) to reduce repetitionparse_hermes_messageswallows exceptions internallyfromisoformat()flexibility with various ISO 8601 formatsLink to Devin run: https://app.devin.ai/sessions/ce563d74b2c841948fa71abc84492932
Requested by: Mike Rolish ([email protected]) (@merolish)