fix: centralize tick price validation in calculate_spread_ratio and determine_order_limits (#52)
Replaces manual isinstance/<=0 checks in calculate_spread_ratio() and determine_order_limits() with _valid_tick_price(), ensuring NaN, inf, -inf, zero, negative, bool, and invalid-string tick values are consistently rejected across all trading helpers. Adds regression tests covering numeric-string acceptance and every invalid-value category for both functions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 4.6
parent
f1ada55bce
commit
1c57be5c44
+8
-11
@@ -782,18 +782,15 @@ def calculate_spread_ratio(client: Mt5TradingClient, symbol: str) -> float:
|
|||||||
"""Return ``(ask - bid) / ((ask + bid) / 2)`` for the latest tick.
|
"""Return ``(ask - bid) / ((ask + bid) / 2)`` for the latest tick.
|
||||||
|
|
||||||
Raises:
|
Raises:
|
||||||
Mt5TradingError: If bid or ask is unavailable or non-positive.
|
Mt5TradingError: If bid or ask is unavailable.
|
||||||
"""
|
"""
|
||||||
tick = get_tick_snapshot(client, symbol)
|
tick = get_tick_snapshot(client, symbol)
|
||||||
bid = tick.get("bid")
|
bid = _valid_tick_price(tick, "bid")
|
||||||
ask = tick.get("ask")
|
ask = _valid_tick_price(tick, "ask")
|
||||||
if not isinstance(bid, int | float) or not isinstance(ask, int | float):
|
if bid is None or ask is None:
|
||||||
msg = f"Tick bid/ask is unavailable for {symbol!r}."
|
msg = f"Tick bid/ask is unavailable for {symbol!r}."
|
||||||
raise Mt5TradingError(msg)
|
raise Mt5TradingError(msg)
|
||||||
if bid <= 0 or ask <= 0:
|
return (ask - bid) / ((ask + bid) / 2.0)
|
||||||
msg = f"Tick bid/ask must be positive for {symbol!r}."
|
|
||||||
raise Mt5TradingError(msg)
|
|
||||||
return (float(ask) - float(bid)) / ((float(ask) + float(bid)) / 2.0)
|
|
||||||
|
|
||||||
|
|
||||||
def calculate_new_position_margin_ratio(
|
def calculate_new_position_margin_ratio(
|
||||||
@@ -1003,11 +1000,11 @@ def determine_order_limits(
|
|||||||
_require_protective_ratio(take_profit_ratio, "take_profit_limit_ratio")
|
_require_protective_ratio(take_profit_ratio, "take_profit_limit_ratio")
|
||||||
normalized_side = _position_side_from_order_side(side)
|
normalized_side = _position_side_from_order_side(side)
|
||||||
tick = get_tick_snapshot(client, symbol)
|
tick = get_tick_snapshot(client, symbol)
|
||||||
entry_value = tick["ask"] if normalized_side == "long" else tick["bid"]
|
entry_key = "ask" if normalized_side == "long" else "bid"
|
||||||
if not isinstance(entry_value, int | float):
|
entry = _valid_tick_price(tick, entry_key)
|
||||||
|
if entry is None:
|
||||||
msg = f"Tick price is unavailable for {symbol!r}."
|
msg = f"Tick price is unavailable for {symbol!r}."
|
||||||
raise Mt5TradingError(msg)
|
raise Mt5TradingError(msg)
|
||||||
entry = float(entry_value)
|
|
||||||
try:
|
try:
|
||||||
symbol_info = get_symbol_snapshot(client, symbol)
|
symbol_info = get_symbol_snapshot(client, symbol)
|
||||||
except (AttributeError, KeyError, TypeError, ValueError):
|
except (AttributeError, KeyError, TypeError, ValueError):
|
||||||
|
|||||||
@@ -427,6 +427,55 @@ class TestDetermineOrderLimits:
|
|||||||
with pytest.raises(Mt5TradingError, match="Tick price is unavailable"):
|
with pytest.raises(Mt5TradingError, match="Tick price is unavailable"):
|
||||||
determine_order_limits(client, "EURUSD", "long")
|
determine_order_limits(client, "EURUSD", "long")
|
||||||
|
|
||||||
|
def test_accepts_numeric_string_entry(self) -> None:
|
||||||
|
"""Test numeric string ask/bid values are accepted as entry prices."""
|
||||||
|
client = MagicMock()
|
||||||
|
client.symbol_info_tick_as_dict.return_value = {
|
||||||
|
"ask": "1.1010",
|
||||||
|
"bid": "1.1000",
|
||||||
|
}
|
||||||
|
client.symbol_info_as_dict.return_value = {"digits": 4}
|
||||||
|
|
||||||
|
result = determine_order_limits(client, "EURUSD", "long")
|
||||||
|
|
||||||
|
_assert_close(result["entry"], 1.1010)
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
("side", "field", "bad_value"),
|
||||||
|
[
|
||||||
|
("long", "ask", float("nan")),
|
||||||
|
("long", "ask", float("inf")),
|
||||||
|
("long", "ask", float("-inf")),
|
||||||
|
("long", "ask", 0.0),
|
||||||
|
("long", "ask", -1.0),
|
||||||
|
("long", "ask", True),
|
||||||
|
("long", "ask", False),
|
||||||
|
("long", "ask", "invalid"),
|
||||||
|
("short", "bid", float("nan")),
|
||||||
|
("short", "bid", float("inf")),
|
||||||
|
("short", "bid", float("-inf")),
|
||||||
|
("short", "bid", 0.0),
|
||||||
|
("short", "bid", -1.0),
|
||||||
|
("short", "bid", True),
|
||||||
|
("short", "bid", False),
|
||||||
|
("short", "bid", "invalid"),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_rejects_invalid_entry_tick_values(
|
||||||
|
self,
|
||||||
|
side: str,
|
||||||
|
field: str,
|
||||||
|
bad_value: object,
|
||||||
|
) -> None:
|
||||||
|
"""Test invalid entry tick values raise Mt5TradingError."""
|
||||||
|
tick: dict[str, object] = {"ask": 1.1, "bid": 1.0}
|
||||||
|
tick[field] = bad_value
|
||||||
|
client = MagicMock()
|
||||||
|
client.symbol_info_tick_as_dict.return_value = tick
|
||||||
|
|
||||||
|
with pytest.raises(Mt5TradingError, match="Tick price is unavailable"):
|
||||||
|
determine_order_limits(client, "EURUSD", side)
|
||||||
|
|
||||||
def test_rejects_stop_loss_inside_broker_stop_level(self) -> None:
|
def test_rejects_stop_loss_inside_broker_stop_level(self) -> None:
|
||||||
"""Test stop-loss prices closer than trade_stops_level raise Mt5TradingError."""
|
"""Test stop-loss prices closer than trade_stops_level raise Mt5TradingError."""
|
||||||
client = MagicMock()
|
client = MagicMock()
|
||||||
@@ -793,6 +842,50 @@ class TestSnapshotsAndState:
|
|||||||
with pytest.raises(Mt5TradingError):
|
with pytest.raises(Mt5TradingError):
|
||||||
calculate_spread_ratio(client, "EURUSD")
|
calculate_spread_ratio(client, "EURUSD")
|
||||||
|
|
||||||
|
def test_calculate_spread_ratio_accepts_numeric_string_tick(self) -> None:
|
||||||
|
"""Test numeric string bid/ask values are accepted."""
|
||||||
|
client = MagicMock()
|
||||||
|
client.symbol_info_tick_as_dict.return_value = {"bid": "99.0", "ask": "101.0"}
|
||||||
|
|
||||||
|
_assert_close(calculate_spread_ratio(client, "EURUSD"), 0.02)
|
||||||
|
|
||||||
|
@pytest.mark.parametrize(
|
||||||
|
("field", "bad_value"),
|
||||||
|
[
|
||||||
|
("bid", None),
|
||||||
|
("bid", float("nan")),
|
||||||
|
("bid", float("inf")),
|
||||||
|
("bid", float("-inf")),
|
||||||
|
("bid", 0.0),
|
||||||
|
("bid", -1.0),
|
||||||
|
("bid", True),
|
||||||
|
("bid", False),
|
||||||
|
("bid", "invalid"),
|
||||||
|
("ask", None),
|
||||||
|
("ask", float("nan")),
|
||||||
|
("ask", float("inf")),
|
||||||
|
("ask", float("-inf")),
|
||||||
|
("ask", 0.0),
|
||||||
|
("ask", -1.0),
|
||||||
|
("ask", True),
|
||||||
|
("ask", False),
|
||||||
|
("ask", "invalid"),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
def test_calculate_spread_ratio_rejects_invalid_tick_values(
|
||||||
|
self,
|
||||||
|
field: str,
|
||||||
|
bad_value: object,
|
||||||
|
) -> None:
|
||||||
|
"""Test invalid bid/ask values raise Mt5TradingError."""
|
||||||
|
tick: dict[str, object] = {"bid": 100.0, "ask": 100.0}
|
||||||
|
tick[field] = bad_value
|
||||||
|
client = MagicMock()
|
||||||
|
client.symbol_info_tick_as_dict.return_value = tick
|
||||||
|
|
||||||
|
with pytest.raises(Mt5TradingError, match="Tick bid/ask is unavailable"):
|
||||||
|
calculate_spread_ratio(client, "EURUSD")
|
||||||
|
|
||||||
|
|
||||||
class TestNormalizeOrderVolume:
|
class TestNormalizeOrderVolume:
|
||||||
"""Tests for normalize_order_volume."""
|
"""Tests for normalize_order_volume."""
|
||||||
|
|||||||
Reference in New Issue
Block a user