Add trading session helpers and extend ThrottledHistoryUpdater (#25)
* Add trading session helpers and extend ThrottledHistoryUpdater Introduce mt5cli.trading with mt5_trading_session() for Mt5TradingClient lifecycle management and reusable operational helpers for position-side detection, margin/volume sizing, and protective order price derivation. Extend ThrottledHistoryUpdater to validate inputs before updates and to optionally suppress ValueError, OSError, and missing-method errors without advancing the throttle timestamp. Export the new helpers from mt5cli.__init__, add unit tests with mocked clients, and document migration guidance for downstream projects such as mteor. Co-authored-by: Daichi Narushima <dceoy@users.noreply.github.com> * Narrow ThrottledHistoryUpdater suppress_errors handling (#27) * Narrow ThrottledHistoryUpdater suppress_errors for MT5 capability only Remove broad AttributeError/TypeError handling from recoverable errors. Add _is_mt5_client_capability_error() to detect missing history API methods or non-callable client attributes by message and attribute name. Generic AttributeError/TypeError values always propagate even when suppress_errors=True. Update docs and tests accordingly. Co-authored-by: Daichi Narushima <dceoy@users.noreply.github.com> * Detect non-callable history client methods in suppress_errors Address review feedback: when a history API attribute exists but is not callable, Python raises a generic TypeError. Inspect the traceback for mt5cli.history client call sites so these capability mismatches are still suppressed without matching all TypeError values. Co-authored-by: Daichi Narushima <dceoy@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Daichi Narushima <dceoy@users.noreply.github.com> * Address PR review feedback on trading helpers - Resolve history module path once at import time - Only treat non-callable TypeErrors as capability errors at the raise site - Validate SL/TP ratios in determine_order_limits - Add tests for margin_free edge cases, body-raise shutdown, and internal TypeError propagation - Clarify ThrottledHistoryUpdater suppress_errors docs - Split README migration example into trading vs read-only history sessions Co-authored-by: Daichi Narushima <dceoy@users.noreply.github.com> * Tighten protective ratio validation and clamp negative margin_free Add _require_protective_ratio enforcing 0 <= ratio < 1 for SL/TP limits so a ratio of 1.0 cannot produce zero protective prices. Clamp negative margin_free to 0.0 in calculate_margin_and_volume before sizing. Add boundary and negative-margin tests; document constraints in trading API docs. Co-authored-by: Daichi Narushima <dceoy@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Daichi Narushima <dceoy@users.noreply.github.com>
This commit is contained in:
+139
-2
@@ -19,7 +19,7 @@ if TYPE_CHECKING:
|
||||
from pdmt5 import Mt5Config, Mt5DataClient
|
||||
|
||||
from mt5cli import sdk
|
||||
from mt5cli.history import DEFAULT_HISTORY_TIMEFRAMES
|
||||
from mt5cli.history import DEFAULT_HISTORY_TIMEFRAMES, write_rates_dataset
|
||||
from mt5cli.sdk import (
|
||||
AccountSpec,
|
||||
Mt5CliClient,
|
||||
@@ -61,7 +61,7 @@ from mt5cli.sdk import (
|
||||
update_history_with_config,
|
||||
version,
|
||||
)
|
||||
from mt5cli.utils import Dataset
|
||||
from mt5cli.utils import Dataset, IfExists
|
||||
|
||||
|
||||
class _TerminalInfo(NamedTuple):
|
||||
@@ -1913,6 +1913,16 @@ class TestThrottledHistoryUpdater:
|
||||
Mt5RuntimeError("boom"),
|
||||
Mt5TradingError("trade failed"),
|
||||
sqlite3.OperationalError("locked"),
|
||||
ValueError("invalid symbols"),
|
||||
OSError("disk full"),
|
||||
AttributeError(
|
||||
"'StubClient' object has no attribute 'copy_rates_range_as_df'",
|
||||
name="copy_rates_range_as_df",
|
||||
),
|
||||
AttributeError(
|
||||
"MT5 client is missing required method: copy_ticks_range_as_df"
|
||||
),
|
||||
TypeError("MT5 client attribute is not callable: history_orders_get_as_df"),
|
||||
],
|
||||
)
|
||||
def test_suppresses_errors_when_requested(
|
||||
@@ -1932,3 +1942,130 @@ class TestThrottledHistoryUpdater:
|
||||
|
||||
assert updater.update(MagicMock(), ["EURUSD"]) is False
|
||||
assert updater.last_update_monotonic is None
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"error",
|
||||
[
|
||||
AttributeError("'dict' object has no attribute 'typo'"),
|
||||
TypeError("unsupported operand types"),
|
||||
],
|
||||
)
|
||||
def test_suppress_errors_does_not_hide_programming_errors(
|
||||
self,
|
||||
mocker: MockerFixture,
|
||||
error: Exception,
|
||||
) -> None:
|
||||
"""Test generic AttributeError/TypeError still propagate when suppressed."""
|
||||
mocker.patch(
|
||||
"mt5cli.sdk.update_history",
|
||||
side_effect=error,
|
||||
)
|
||||
updater = ThrottledHistoryUpdater(
|
||||
output="history.db",
|
||||
suppress_errors=True,
|
||||
)
|
||||
|
||||
with pytest.raises(type(error)):
|
||||
updater.update(MagicMock(), ["EURUSD"])
|
||||
|
||||
assert updater.last_update_monotonic is None
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("error", "expected"),
|
||||
[
|
||||
(AttributeError("MT5 client is missing required method: version"), True),
|
||||
(
|
||||
AttributeError(
|
||||
"'Stub' object has no attribute 'copy_rates_range_as_df'",
|
||||
name="copy_rates_range_as_df",
|
||||
),
|
||||
True,
|
||||
),
|
||||
(AttributeError("'dict' object has no attribute 'typo'"), False),
|
||||
(TypeError("MT5 client attribute is not callable: version"), True),
|
||||
(TypeError("unsupported operand types"), False),
|
||||
(TypeError("'NoneType' object is not callable"), False),
|
||||
(ValueError("invalid"), False),
|
||||
],
|
||||
)
|
||||
def test_is_mt5_client_capability_error(
|
||||
self,
|
||||
error: BaseException,
|
||||
expected: bool,
|
||||
) -> None:
|
||||
"""Test MT5 client capability error detection."""
|
||||
assert sdk._is_mt5_client_capability_error(error) is expected # type: ignore[reportPrivateUsage]
|
||||
|
||||
def test_is_mt5_client_capability_error_for_non_callable_history_client(
|
||||
self,
|
||||
) -> None:
|
||||
"""Test non-callable history client attributes are capability errors."""
|
||||
client = MagicMock()
|
||||
client.copy_rates_range_as_df = None
|
||||
with (
|
||||
sqlite3.connect(":memory:") as conn,
|
||||
pytest.raises(TypeError, match="not callable") as exc_info,
|
||||
):
|
||||
write_rates_dataset(
|
||||
conn,
|
||||
client,
|
||||
["EURUSD"],
|
||||
1,
|
||||
datetime.now(UTC),
|
||||
datetime.now(UTC),
|
||||
IfExists.APPEND,
|
||||
{},
|
||||
)
|
||||
|
||||
assert sdk._is_mt5_client_capability_error(exc_info.value) is True # type: ignore[reportPrivateUsage]
|
||||
|
||||
def test_suppresses_non_callable_history_client_method(
|
||||
self,
|
||||
tmp_path: Path,
|
||||
) -> None:
|
||||
"""Test suppress_errors swallows non-callable history client API attributes."""
|
||||
client = MagicMock()
|
||||
client.copy_rates_range_as_df = None
|
||||
updater = ThrottledHistoryUpdater(
|
||||
output=tmp_path / "history.db",
|
||||
datasets={Dataset.rates},
|
||||
timeframes=["M1"],
|
||||
suppress_errors=True,
|
||||
)
|
||||
|
||||
assert updater.update(client, ["EURUSD"]) is False
|
||||
assert updater.last_update_monotonic is None
|
||||
|
||||
def test_suppress_errors_does_not_hide_internal_client_type_error(
|
||||
self,
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""Test TypeError raised inside a callable client method still propagates."""
|
||||
mocker.patch(
|
||||
"mt5cli.sdk.update_history",
|
||||
side_effect=TypeError("'int' object is not callable"),
|
||||
)
|
||||
updater = ThrottledHistoryUpdater(
|
||||
output="history.db",
|
||||
suppress_errors=True,
|
||||
)
|
||||
|
||||
with pytest.raises(TypeError, match="not callable"):
|
||||
updater.update(MagicMock(), ["EURUSD"])
|
||||
|
||||
assert updater.last_update_monotonic is None
|
||||
|
||||
def test_suppresses_validation_errors_before_update(
|
||||
self,
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""Test validation failures are suppressed without calling update_history."""
|
||||
update = mocker.patch("mt5cli.sdk.update_history")
|
||||
updater = ThrottledHistoryUpdater(
|
||||
output="history.db",
|
||||
suppress_errors=True,
|
||||
)
|
||||
|
||||
assert updater.update(MagicMock(), []) is False
|
||||
update.assert_not_called()
|
||||
assert updater.last_update_monotonic is None
|
||||
|
||||
@@ -0,0 +1,356 @@
|
||||
"""Tests for trading session helpers and operational utilities."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import pandas as pd
|
||||
import pytest
|
||||
from pdmt5 import Mt5RuntimeError
|
||||
from pytest_mock import MockerFixture # noqa: TC002
|
||||
|
||||
from mt5cli.sdk import build_config
|
||||
from mt5cli.trading import (
|
||||
calculate_margin_and_volume,
|
||||
detect_position_side,
|
||||
determine_order_limits,
|
||||
mt5_trading_session,
|
||||
)
|
||||
|
||||
|
||||
class TestDetectPositionSide:
|
||||
"""Tests for detect_position_side."""
|
||||
|
||||
def test_returns_none_when_no_positions(self) -> None:
|
||||
"""Test None is returned when no open positions exist."""
|
||||
client = MagicMock()
|
||||
client.positions_get_as_df.return_value = pd.DataFrame()
|
||||
|
||||
assert detect_position_side(client, "EURUSD") is None
|
||||
|
||||
def test_returns_long_for_net_buy_volume(self) -> None:
|
||||
"""Test long is returned when buy volume exceeds sell volume."""
|
||||
client = MagicMock()
|
||||
client.mt5.POSITION_TYPE_BUY = 0
|
||||
client.mt5.POSITION_TYPE_SELL = 1
|
||||
client.positions_get_as_df.return_value = pd.DataFrame(
|
||||
{
|
||||
"type": [0, 0, 1],
|
||||
"volume": [0.2, 0.1, 0.05],
|
||||
},
|
||||
)
|
||||
|
||||
assert detect_position_side(client, "EURUSD") == "long"
|
||||
|
||||
def test_returns_short_for_net_sell_volume(self) -> None:
|
||||
"""Test short is returned when sell volume exceeds buy volume."""
|
||||
client = MagicMock()
|
||||
client.mt5.POSITION_TYPE_BUY = 0
|
||||
client.mt5.POSITION_TYPE_SELL = 1
|
||||
client.positions_get_as_df.return_value = pd.DataFrame(
|
||||
{
|
||||
"type": [1, 1],
|
||||
"volume": [0.3, 0.1],
|
||||
},
|
||||
)
|
||||
|
||||
assert detect_position_side(client, "EURUSD") == "short"
|
||||
|
||||
def test_returns_none_for_balanced_hedged_positions(self) -> None:
|
||||
"""Test None is returned when buy and sell volumes net to zero."""
|
||||
client = MagicMock()
|
||||
client.mt5.POSITION_TYPE_BUY = 0
|
||||
client.mt5.POSITION_TYPE_SELL = 1
|
||||
client.positions_get_as_df.return_value = pd.DataFrame(
|
||||
{
|
||||
"type": [0, 1],
|
||||
"volume": [0.2, 0.2],
|
||||
},
|
||||
)
|
||||
|
||||
assert detect_position_side(client, "EURUSD") is None
|
||||
|
||||
|
||||
class TestCalculateMarginAndVolume:
|
||||
"""Tests for calculate_margin_and_volume."""
|
||||
|
||||
def test_calculates_margin_budget_and_volumes(self) -> None:
|
||||
"""Test margin budget and buy/sell volumes are derived from ratios."""
|
||||
client = MagicMock()
|
||||
client.account_info_as_dict.return_value = {"margin_free": 1000.0}
|
||||
client.calculate_volume_by_margin.side_effect = [0.3, 0.2]
|
||||
|
||||
result = calculate_margin_and_volume(
|
||||
client,
|
||||
"EURUSD",
|
||||
unit_margin_ratio=0.5,
|
||||
preserved_margin_ratio=0.2,
|
||||
)
|
||||
|
||||
assert result == {
|
||||
"margin_free": 1000.0,
|
||||
"available_margin": 800.0,
|
||||
"trade_margin": 400.0,
|
||||
"buy_volume": 0.3,
|
||||
"sell_volume": 0.2,
|
||||
}
|
||||
client.calculate_volume_by_margin.assert_any_call("EURUSD", 400.0, "BUY")
|
||||
client.calculate_volume_by_margin.assert_any_call("EURUSD", 400.0, "SELL")
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("account_dict", "expected_margin_free"),
|
||||
[
|
||||
({"margin_free": 0.0}, 0.0),
|
||||
({}, 0.0),
|
||||
({"margin_free": None}, 0.0),
|
||||
],
|
||||
)
|
||||
def test_zero_or_missing_margin_free(
|
||||
self,
|
||||
account_dict: dict[str, float | None],
|
||||
expected_margin_free: float,
|
||||
) -> None:
|
||||
"""Test missing or zero margin_free yields zero trade margin."""
|
||||
client = MagicMock()
|
||||
client.account_info_as_dict.return_value = account_dict
|
||||
client.calculate_volume_by_margin.return_value = 0.0
|
||||
|
||||
result = calculate_margin_and_volume(
|
||||
client,
|
||||
"EURUSD",
|
||||
unit_margin_ratio=0.5,
|
||||
preserved_margin_ratio=0.2,
|
||||
)
|
||||
|
||||
assert result["margin_free"] == expected_margin_free
|
||||
client.calculate_volume_by_margin.assert_any_call("EURUSD", 0.0, "BUY")
|
||||
client.calculate_volume_by_margin.assert_any_call("EURUSD", 0.0, "SELL")
|
||||
|
||||
def test_clamps_negative_margin_free_to_zero(self) -> None:
|
||||
"""Test negative margin_free is clamped to zero before sizing."""
|
||||
client = MagicMock()
|
||||
client.account_info_as_dict.return_value = {"margin_free": -500.0}
|
||||
client.calculate_volume_by_margin.return_value = 0.0
|
||||
|
||||
result = calculate_margin_and_volume(
|
||||
client,
|
||||
"EURUSD",
|
||||
unit_margin_ratio=0.5,
|
||||
preserved_margin_ratio=0.2,
|
||||
)
|
||||
|
||||
expected_margin_free = 0.0
|
||||
assert result["margin_free"] == expected_margin_free
|
||||
client.calculate_volume_by_margin.assert_any_call("EURUSD", 0.0, "BUY")
|
||||
client.calculate_volume_by_margin.assert_any_call("EURUSD", 0.0, "SELL")
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("unit_ratio", "preserved_ratio"),
|
||||
[
|
||||
(-0.1, 0.0),
|
||||
(1.1, 0.0),
|
||||
(0.5, -0.1),
|
||||
(0.5, 1.1),
|
||||
],
|
||||
)
|
||||
def test_rejects_invalid_ratios(
|
||||
self,
|
||||
unit_ratio: float,
|
||||
preserved_ratio: float,
|
||||
) -> None:
|
||||
"""Test invalid ratio values raise ValueError."""
|
||||
with pytest.raises(ValueError, match="must be between 0 and 1"):
|
||||
calculate_margin_and_volume(
|
||||
MagicMock(),
|
||||
"EURUSD",
|
||||
unit_margin_ratio=unit_ratio,
|
||||
preserved_margin_ratio=preserved_ratio,
|
||||
)
|
||||
|
||||
|
||||
class TestDetermineOrderLimits:
|
||||
"""Tests for determine_order_limits."""
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("side", "expected_entry_key"),
|
||||
[
|
||||
("long", "ask"),
|
||||
("short", "bid"),
|
||||
("buy", "ask"),
|
||||
("sell", "bid"),
|
||||
],
|
||||
)
|
||||
def test_uses_expected_quote_for_entry(
|
||||
self,
|
||||
side: str,
|
||||
expected_entry_key: str,
|
||||
) -> None:
|
||||
"""Test entry price is taken from ask for long/buy and bid for short/sell."""
|
||||
client = MagicMock()
|
||||
client.symbol_info_tick_as_dict.return_value = {"ask": 1.1010, "bid": 1.1000}
|
||||
|
||||
result = determine_order_limits(
|
||||
client,
|
||||
"EURUSD",
|
||||
side,
|
||||
stop_loss_limit_ratio=0.0,
|
||||
take_profit_limit_ratio=0.0,
|
||||
)
|
||||
|
||||
assert (
|
||||
result["entry"]
|
||||
== client.symbol_info_tick_as_dict.return_value[expected_entry_key]
|
||||
)
|
||||
assert result["stop_loss"] is None
|
||||
assert result["take_profit"] is None
|
||||
|
||||
def test_calculates_long_protective_levels(self) -> None:
|
||||
"""Test long stop loss and take profit are placed below/above entry."""
|
||||
client = MagicMock()
|
||||
client.symbol_info_tick_as_dict.return_value = {"ask": 100.0, "bid": 99.0}
|
||||
|
||||
result = determine_order_limits(
|
||||
client,
|
||||
"EURUSD",
|
||||
"long",
|
||||
stop_loss_limit_ratio=0.02,
|
||||
take_profit_limit_ratio=0.03,
|
||||
)
|
||||
|
||||
assert result == {
|
||||
"entry": 100.0,
|
||||
"stop_loss": 98.0,
|
||||
"take_profit": 103.0,
|
||||
}
|
||||
|
||||
def test_calculates_short_protective_levels(self) -> None:
|
||||
"""Test short stop loss and take profit are placed above/below entry."""
|
||||
client = MagicMock()
|
||||
client.symbol_info_tick_as_dict.return_value = {"ask": 100.0, "bid": 99.0}
|
||||
|
||||
result = determine_order_limits(
|
||||
client,
|
||||
"EURUSD",
|
||||
"short",
|
||||
stop_loss_limit_ratio=0.02,
|
||||
take_profit_limit_ratio=0.03,
|
||||
)
|
||||
|
||||
assert result == {
|
||||
"entry": 99.0,
|
||||
"stop_loss": 100.98,
|
||||
"take_profit": 96.03,
|
||||
}
|
||||
|
||||
def test_rejects_unknown_side(self) -> None:
|
||||
"""Test unsupported side values raise ValueError."""
|
||||
with pytest.raises(ValueError, match="Unsupported order side"):
|
||||
determine_order_limits(
|
||||
MagicMock(),
|
||||
"EURUSD",
|
||||
"flat",
|
||||
stop_loss_limit_ratio=0.01,
|
||||
take_profit_limit_ratio=0.01,
|
||||
)
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("stop_loss_ratio", "take_profit_ratio"),
|
||||
[
|
||||
(-0.05, 0.01),
|
||||
(0.01, 2.0),
|
||||
],
|
||||
)
|
||||
def test_rejects_invalid_protective_ratios(
|
||||
self,
|
||||
stop_loss_ratio: float,
|
||||
take_profit_ratio: float,
|
||||
) -> None:
|
||||
"""Test out-of-range protective ratios raise ValueError."""
|
||||
with pytest.raises(ValueError, match="must be at least 0 and less than 1"):
|
||||
determine_order_limits(
|
||||
MagicMock(),
|
||||
"EURUSD",
|
||||
"long",
|
||||
stop_loss_limit_ratio=stop_loss_ratio,
|
||||
take_profit_limit_ratio=take_profit_ratio,
|
||||
)
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("field", "ratio"),
|
||||
[
|
||||
("stop_loss_limit_ratio", 1.0),
|
||||
("take_profit_limit_ratio", 1.0),
|
||||
],
|
||||
)
|
||||
def test_rejects_unit_boundary_protective_ratios(
|
||||
self,
|
||||
field: str,
|
||||
ratio: float,
|
||||
) -> None:
|
||||
"""Test protective ratios of exactly 1.0 are rejected."""
|
||||
kwargs = {
|
||||
"stop_loss_limit_ratio": 0.01,
|
||||
"take_profit_limit_ratio": 0.01,
|
||||
field: ratio,
|
||||
}
|
||||
with pytest.raises(ValueError, match="must be at least 0 and less than 1"):
|
||||
determine_order_limits(
|
||||
MagicMock(),
|
||||
"EURUSD",
|
||||
"long",
|
||||
**kwargs,
|
||||
)
|
||||
|
||||
|
||||
class TestMt5TradingSession:
|
||||
"""Tests for the mt5_trading_session context manager."""
|
||||
|
||||
def test_yields_connected_client_and_shuts_down(
|
||||
self,
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""Test mt5_trading_session connects, yields a client, and shuts down."""
|
||||
mock_client = MagicMock()
|
||||
trading_client = mocker.patch(
|
||||
"mt5cli.trading.Mt5TradingClient",
|
||||
return_value=mock_client,
|
||||
)
|
||||
|
||||
with mt5_trading_session(
|
||||
build_config(path="/opt/mt5/terminal64.exe"),
|
||||
retry_count=2,
|
||||
) as client:
|
||||
mock_client.initialize_and_login_mt5.assert_called_once()
|
||||
assert client is mock_client
|
||||
|
||||
trading_client.assert_called_once()
|
||||
assert trading_client.call_args.kwargs["retry_count"] == 2
|
||||
assert (
|
||||
trading_client.call_args.kwargs["config"].path == "/opt/mt5/terminal64.exe"
|
||||
)
|
||||
mock_client.shutdown.assert_called_once()
|
||||
|
||||
def test_shuts_down_when_initialize_raises(
|
||||
self,
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""Test shutdown is called when initialization fails."""
|
||||
mock_client = MagicMock()
|
||||
mock_client.initialize_and_login_mt5.side_effect = Mt5RuntimeError("boom")
|
||||
mocker.patch("mt5cli.trading.Mt5TradingClient", return_value=mock_client)
|
||||
|
||||
with pytest.raises(Mt5RuntimeError, match="boom"), mt5_trading_session():
|
||||
pass
|
||||
|
||||
mock_client.shutdown.assert_called_once()
|
||||
|
||||
def test_shuts_down_when_body_raises(self, mocker: MockerFixture) -> None:
|
||||
"""Test shutdown is called when the context body raises."""
|
||||
mock_client = MagicMock()
|
||||
mocker.patch("mt5cli.trading.Mt5TradingClient", return_value=mock_client)
|
||||
|
||||
body_error = "body error"
|
||||
with pytest.raises(RuntimeError, match=body_error), mt5_trading_session():
|
||||
raise RuntimeError(body_error)
|
||||
|
||||
mock_client.shutdown.assert_called_once()
|
||||
Reference in New Issue
Block a user