From 15bfd17db3b014a18df08105b1e209ce4af72b36 Mon Sep 17 00:00:00 2001 From: Daichi Narushima <1938249+dceoy@users.noreply.github.com> Date: Thu, 25 Jun 2026 01:55:04 +0900 Subject: [PATCH] test: reduce test_trading.py duplication with parametrize (#64) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test: reduce test_trading.py duplication with parametrize Collapse repetitive individual tests in test_trading.py into parametrized equivalents, cutting 267 lines without losing any cases. - TestExtractTickPrice: 13 tests → 2 parametrized (×3 valid, ×10 None) - TestEstimateOrderMargin: 4 invalid-margin tests → 1 parametrized ×4; nan/inf volume tests → 1 parametrized ×2 - TestNormalizeOrderVolume: multi-assert bodies split into parametrized cases for non-finite volume and constraints - TestVolumeAndExecution: 9 place_market_order retcode tests → 1 ×11; 5 update_sltp retcode tests → 1 ×5 - test_calculate_trailing_stop_updates_missing_symbol_digits: inline double-assert body → 1 parametrized ×2 Co-Authored-By: Claude Sonnet 4.6 * test: further reduce test_trading.py duplication with parametrize Merge six broker stop-level tests into two parametrized tests, collapse two default-digits fallback tests and three symbol-filter zero-margin tests into one each. Co-Authored-By: Claude Sonnet 4.6 * test: address claude[bot] review on PR #64 - Consolidate _MISSING_RETCODE sentinel to one line with corrected comment - Add comment explaining ids list is required for deterministic node IDs - Document intentional narrower retcode coverage in update_sltp test Co-Authored-By: Claude Sonnet 4.6 * test: reduce duplication in test_sdk, test_history, test_contracts - TestBuildConfigWholeDollarEnv: 3 field tests (server/password/path) → 1 parametrized ×3 - TestResolveAccountSpec: whole-dollar expand/no-expand pair → 1 parametrized ×2 - test_normalize_mt5_exception_maps_types: 2 isinstance asserts → parametrized ×2 - test_resolve_history_tick_flags_invalid: 2 pytest.raises blocks → parametrized ×2 Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: agent Co-authored-by: Claude Sonnet 4.6 --- tests/test_contracts.py | 66 ++-- tests/test_history.py | 73 +--- tests/test_sdk.py | 74 ++-- tests/test_trading.py | 771 ++++++++++++---------------------------- 4 files changed, 307 insertions(+), 677 deletions(-) diff --git a/tests/test_contracts.py b/tests/test_contracts.py index 6a3c9ab..2fbc424 100644 --- a/tests/test_contracts.py +++ b/tests/test_contracts.py @@ -234,16 +234,19 @@ def test_is_recoverable_mt5_error(exc: Exception) -> None: assert is_recoverable_mt5_error(exc) -def test_normalize_mt5_exception_maps_types() -> None: +@pytest.mark.parametrize( + ("exc", "expected_type"), + [ + (Mt5RuntimeError("x"), Mt5ConnectionError), + (Mt5TradingError("x"), Mt5OperationError), + ], +) +def test_normalize_mt5_exception_maps_types( + exc: Exception, + expected_type: type[Mt5ConnectionError | Mt5OperationError], +) -> None: """MT5 exceptions map to stable mt5cli types.""" - assert isinstance( - normalize_mt5_exception(Mt5RuntimeError("x")), - Mt5ConnectionError, - ) - assert isinstance( - normalize_mt5_exception(Mt5TradingError("x")), - Mt5OperationError, - ) + assert isinstance(normalize_mt5_exception(exc), expected_type) def test_call_with_normalized_errors_reraises_mapped_type() -> None: @@ -420,26 +423,24 @@ def test_normalize_time_columns_skips_absent_time_fields() -> None: assert list(result.columns) == ["open"] -def test_normalize_time_columns_converts_unix_seconds() -> None: - """Numeric MT5 ``time`` values are interpreted as Unix seconds.""" - frame = pd.DataFrame({"time": [1704067200]}) - result = normalize_time_columns(frame, DataKind.rates) - assert result.loc[0, "time"] == pd.Timestamp("2024-01-01T00:00:00+00:00") - - -def test_normalize_time_columns_converts_unix_milliseconds() -> None: - """Numeric MT5 ``time_msc`` values are interpreted as Unix milliseconds.""" - frame = pd.DataFrame({"time_msc": [1704067200000]}) - result = normalize_time_columns(frame, DataKind.ticks) - assert result.loc[0, "time_msc"] == pd.Timestamp("2024-01-01T00:00:00+00:00") - - -def test_normalize_time_columns_preserves_utc_datetimes() -> None: - """Already-converted datetime values remain UTC-normalized.""" - aware = datetime(2024, 1, 1, tzinfo=UTC) - frame = pd.DataFrame({"time": [aware]}) - result = normalize_time_columns(frame, DataKind.rates) - assert result.loc[0, "time"] == pd.Timestamp("2024-01-01T00:00:00+00:00") +@pytest.mark.parametrize( + ("col", "value", "kind"), + [ + ("time", 1704067200, DataKind.rates), + ("time_msc", 1704067200000, DataKind.ticks), + ("time", datetime(2024, 1, 1, tzinfo=UTC), DataKind.rates), + ("time", "2024-01-01T00:00:00+00:00", DataKind.rates), + ], +) +def test_normalize_time_columns_coerces_value( + col: str, + value: object, + kind: DataKind, +) -> None: + """Time column values are coerced to UTC timestamps regardless of input type.""" + frame = pd.DataFrame({col: [value]}) + result = normalize_time_columns(frame, kind) + assert result.loc[0, col] == pd.Timestamp("2024-01-01T00:00:00+00:00") def test_normalize_time_columns_handles_optional_order_times() -> None: @@ -489,13 +490,6 @@ def test_ensure_utc_columns_skips_missing_columns() -> None: assert "time" in result.columns -def test_normalize_time_columns_coerces_string_timestamps() -> None: - """String timestamps are parsed with timezone-aware datetime coercion.""" - frame = pd.DataFrame({"time": ["2024-01-01T00:00:00+00:00"]}) - result = normalize_time_columns(frame, DataKind.rates) - assert result.loc[0, "time"] == pd.Timestamp("2024-01-01T00:00:00+00:00") - - def test_ensure_utc_columns_coerces_non_mt5_columns() -> None: """Non-MT5 columns still coerce to UTC datetimes.""" frame = pd.DataFrame({"created_at": ["2024-01-01T00:00:00+00:00"]}) diff --git a/tests/test_history.py b/tests/test_history.py index ddadd01..3041d67 100644 --- a/tests/test_history.py +++ b/tests/test_history.py @@ -705,19 +705,25 @@ class TestIncrementalStart: assert starts["EURUSD", 1] == datetime(2024, 1, 2, tzinfo=UTC) assert starts["GBPUSD", 1] == datetime(2024, 1, 3, tzinfo=UTC) - def test_load_incremental_start_datetimes_requires_timeframe_column( + @pytest.mark.parametrize( + ("ddl", "missing_col"), + [ + ("CREATE TABLE rates(symbol TEXT, time TEXT, open REAL)", "timeframe"), + ("CREATE TABLE rates(timeframe INTEGER, time TEXT, open REAL)", "symbol"), + ("CREATE TABLE rates(symbol TEXT, timeframe INTEGER, open REAL)", "time"), + ], + ) + def test_load_incremental_start_datetimes_requires_column( self, tmp_path: Path, + ddl: str, + missing_col: str, ) -> None: - """Test rates tables without timeframe fail fast during incremental resume.""" + """Test rates tables missing a required column fail fast.""" fallback = datetime(2024, 1, 1, tzinfo=UTC) - with sqlite3.connect(tmp_path / "rates-without-timeframe.db") as conn: - conn.execute("CREATE TABLE rates(symbol TEXT, time TEXT, open REAL)") - conn.execute( - "INSERT INTO rates(symbol, time, open) VALUES (?, ?, ?)", - ("EURUSD", "2024-01-02T00:00:00+00:00", 1.0), - ) - with pytest.raises(ValueError, match="missing: timeframe") as exc_info: + with sqlite3.connect(tmp_path / f"rates-no-{missing_col}.db") as conn: + conn.execute(ddl) + with pytest.raises(ValueError, match=f"missing: {missing_col}") as exc_info: load_incremental_start_datetimes( conn, Dataset.rates, @@ -725,47 +731,7 @@ class TestIncrementalStart: timeframes=[1], fallback_start=fallback, ) - assert "timeframe" in str(exc_info.value) - - def test_load_incremental_start_datetimes_requires_symbol_column( - self, - tmp_path: Path, - ) -> None: - """Test rates tables without symbol fail fast during incremental resume.""" - fallback = datetime(2024, 1, 1, tzinfo=UTC) - with sqlite3.connect(tmp_path / "rates-no-symbol.db") as conn: - conn.execute( - "CREATE TABLE rates(timeframe INTEGER, time TEXT, open REAL)", - ) - with pytest.raises(ValueError, match="missing: symbol") as exc_info: - load_incremental_start_datetimes( - conn, - Dataset.rates, - symbols=["EURUSD"], - timeframes=[1], - fallback_start=fallback, - ) - assert "symbol" in str(exc_info.value) - - def test_load_incremental_start_datetimes_requires_time_column( - self, - tmp_path: Path, - ) -> None: - """Test rates tables without time fail fast during incremental resume.""" - fallback = datetime(2024, 1, 1, tzinfo=UTC) - with sqlite3.connect(tmp_path / "rates-no-time.db") as conn: - conn.execute( - "CREATE TABLE rates(symbol TEXT, timeframe INTEGER, open REAL)", - ) - with pytest.raises(ValueError, match="missing: time") as exc_info: - load_incremental_start_datetimes( - conn, - Dataset.rates, - symbols=["EURUSD"], - timeframes=[1], - fallback_start=fallback, - ) - assert "time" in str(exc_info.value) + assert missing_col in str(exc_info.value) def test_load_incremental_start_datetimes_rejects_unrelated_rates_columns( self, @@ -1800,12 +1766,11 @@ class TestIncrementalIntegration: ) assert written_tables == set() - def test_resolve_history_tick_flags_invalid(self) -> None: + @pytest.mark.parametrize("flags", ["BAD", 7]) + def test_resolve_history_tick_flags_invalid(self, flags: str | int) -> None: """Test invalid tick flags raise ValueError.""" with pytest.raises(ValueError, match="Invalid tick flags"): - resolve_history_tick_flags("BAD") - with pytest.raises(ValueError, match="Invalid tick flags"): - resolve_history_tick_flags(7) + resolve_history_tick_flags(flags) def test_resolve_history_timeframes_invalid(self) -> None: """Test invalid timeframes raise ValueError.""" diff --git a/tests/test_sdk.py b/tests/test_sdk.py index aeebb2b..bfc8fc8 100644 --- a/tests/test_sdk.py +++ b/tests/test_sdk.py @@ -1938,29 +1938,28 @@ class TestResolveAccountSpec: assert [a.server for a in resolved] == ["Shared", "Fixed"] assert all(a.timeout == 1000 for a in resolved) - def test_resolve_account_spec_with_whole_dollar_env( + @pytest.mark.parametrize( + ("allow_whole_dollar_env", "expected"), + [ + (True, "secret"), + (False, "$MT5_PASSWORD"), + ], + ) + def test_resolve_account_spec_whole_dollar_password( self, monkeypatch: pytest.MonkeyPatch, + allow_whole_dollar_env: bool, + expected: str, ) -> None: - """Account spec expands $ENV_NAME when allow_whole_dollar_env=True.""" + """Test resolve_account_spec expands $ENV_NAME password only with opt-in.""" monkeypatch.setenv("MT5_PASSWORD", "secret") account = AccountSpec(symbols=["EURUSD"], password="$MT5_PASSWORD") - resolved = resolve_account_spec(account, allow_whole_dollar_env=True) + resolved = resolve_account_spec( + account, allow_whole_dollar_env=allow_whole_dollar_env + ) - assert resolved.password == "secret" # noqa: S105 - - def test_resolve_account_spec_whole_dollar_not_expanded_by_default( - self, - monkeypatch: pytest.MonkeyPatch, - ) -> None: - """Test resolve_account_spec leaves $ENV_NAME literal by default.""" - monkeypatch.setenv("MT5_PASSWORD", "secret") - account = AccountSpec(symbols=["EURUSD"], password="$MT5_PASSWORD") - - resolved = resolve_account_spec(account) - - assert resolved.password == "$MT5_PASSWORD" # noqa: S105 + assert resolved.password == expected def test_resolve_account_specs_with_whole_dollar_env( self, @@ -1994,38 +1993,27 @@ class TestResolveAccountSpec: class TestBuildConfigWholeDollarEnv: """Tests for build_config with allow_whole_dollar_env.""" - def test_build_config_substitutes_server_with_opt_in( + @pytest.mark.parametrize( + ("env_var", "field", "env_value"), + [ + ("MT5_SERVER", "server", "Broker-Demo"), + ("MT5_PASSWORD", "password", "secret"), + ("MT5_PATH", "path", "/opt/mt5/terminal64.exe"), + ], + ) + def test_build_config_substitutes_field_with_opt_in( self, monkeypatch: pytest.MonkeyPatch, + env_var: str, + field: str, + env_value: str, ) -> None: - """build_config expands $ENV_NAME server when allow_whole_dollar_env=True.""" - monkeypatch.setenv("MT5_SERVER", "Broker-Demo") + """Test build_config expands $ENV_NAME fields when opt-in is enabled.""" + monkeypatch.setenv(env_var, env_value) - config = build_config(server="$MT5_SERVER", allow_whole_dollar_env=True) + config = build_config(**{field: f"${env_var}"}, allow_whole_dollar_env=True) # type: ignore[arg-type] - assert config.server == "Broker-Demo" - - def test_build_config_substitutes_password_with_opt_in( - self, - monkeypatch: pytest.MonkeyPatch, - ) -> None: - """build_config expands $ENV_NAME password when allow_whole_dollar_env=True.""" - monkeypatch.setenv("MT5_PASSWORD", "secret") - - config = build_config(password="$MT5_PASSWORD", allow_whole_dollar_env=True) - - assert config.password == "secret" # noqa: S105 - - def test_build_config_substitutes_path_with_opt_in( - self, - monkeypatch: pytest.MonkeyPatch, - ) -> None: - """Test build_config expands $ENV_NAME path when allow_whole_dollar_env=True.""" - monkeypatch.setenv("MT5_PATH", "/opt/mt5/terminal64.exe") - - config = build_config(path="$MT5_PATH", allow_whole_dollar_env=True) - - assert config.path == "/opt/mt5/terminal64.exe" + assert getattr(config, field) == env_value def test_build_config_leaves_dollar_literal_by_default( self, diff --git a/tests/test_trading.py b/tests/test_trading.py index d71532d..e48ac34 100644 --- a/tests/test_trading.py +++ b/tests/test_trading.py @@ -75,6 +75,11 @@ def _request_from_result(result: OrderExecutionResult) -> dict[str, object]: # return result["request"] +_MISSING_RETCODE: object = ( + object() +) # sentinel: absent "retcode" key (used in two parametrized retcode tests) + + class TestDetectPositionSide: """Tests for detect_position_side.""" @@ -376,27 +381,25 @@ class TestDetermineOrderLimits: **kwargs, ) - def test_uses_default_digits_when_symbol_snapshot_fails(self) -> None: - """Test order limit rounding falls back when symbol metadata is missing.""" + @pytest.mark.parametrize( + ("return_value", "side_effect"), + [ + ({"digits": "invalid"}, None), + (None, AttributeError("missing")), + ], + ) + def test_uses_default_digits_when_symbol_info_fails( + self, + return_value: dict[str, object] | None, + side_effect: Exception | None, + ) -> None: + """Test order limit rounding falls back when symbol metadata is unavailable.""" client = MagicMock() client.symbol_info_tick_as_dict.return_value = {"ask": 1.234567891, "bid": 1.0} - client.symbol_info_as_dict.return_value = {"digits": "invalid"} - - result = determine_order_limits( - client, - "EURUSD", - "long", - stop_loss_limit_ratio=0.01, - take_profit_limit_ratio=0.01, - ) - - _assert_close(result["stop_loss"], 1.22222221) - - def test_uses_default_digits_when_symbol_lookup_raises(self) -> None: - """Test order limits fall back when symbol metadata lookup fails.""" - client = MagicMock() - client.symbol_info_tick_as_dict.return_value = {"ask": 1.234567891, "bid": 1.0} - client.symbol_info_as_dict.side_effect = AttributeError("missing") + if side_effect is not None: + client.symbol_info_as_dict.side_effect = side_effect + else: + client.symbol_info_as_dict.return_value = return_value result = determine_order_limits( client, @@ -482,23 +485,34 @@ class TestDetermineOrderLimits: 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: - """Test stop-loss prices closer than trade_stops_level raise Mt5TradingError.""" + @pytest.mark.parametrize( + ("side", "ask", "bid", "kwarg", "match"), + [ + ("long", 1.0, 0.99, "stop_loss_limit_ratio", "Stop loss for 'EURUSD'"), + ("long", 1.0, 0.99, "take_profit_limit_ratio", "Take profit for 'EURUSD'"), + ("short", 1.01, 1.0, "stop_loss_limit_ratio", "Stop loss for 'EURUSD'"), + ("short", 1.01, 1.0, "take_profit_limit_ratio", "Take profit for 'EURUSD'"), + ], + ) + def test_rejects_protective_level_inside_broker_stop_level( + self, + side: str, + ask: float, + bid: float, + kwarg: str, + match: str, + ) -> None: + """Test protective levels inside trade_stops_level raise Mt5TradingError.""" client = MagicMock() - client.symbol_info_tick_as_dict.return_value = {"ask": 1.0, "bid": 0.99} + client.symbol_info_tick_as_dict.return_value = {"ask": ask, "bid": bid} client.symbol_info_as_dict.return_value = { "digits": 2, "trade_stops_level": 100, "point": 0.0001, } - with pytest.raises(Mt5TradingError, match="Stop loss for 'EURUSD'"): - determine_order_limits( - client, - "EURUSD", - "long", - stop_loss_limit_ratio=0.0001, - ) + with pytest.raises(Mt5TradingError, match=match): + determine_order_limits(client, "EURUSD", side, **{kwarg: 0.0001}) def test_accepts_stop_loss_exactly_at_minimum_stop_distance(self) -> None: """Test protective levels exactly at trade_stops_level distance pass.""" @@ -520,10 +534,24 @@ class TestDetermineOrderLimits: _assert_close(result["stop_loss"], 0.99) - def test_allows_protective_levels_beyond_broker_stop_level(self) -> None: + @pytest.mark.parametrize( + ("side", "ask", "bid", "expected_sl", "expected_tp"), + [ + ("long", 1.0, 0.99, 0.95, 1.05), + ("short", 1.01, 1.0, 1.05, 0.95), + ], + ) + def test_allows_protective_levels_beyond_broker_stop_level( + self, + side: str, + ask: float, + bid: float, + expected_sl: float, + expected_tp: float, + ) -> None: """Test SL/TP beyond trade_stops_level pass validation.""" client = MagicMock() - client.symbol_info_tick_as_dict.return_value = {"ask": 1.0, "bid": 0.99} + client.symbol_info_tick_as_dict.return_value = {"ask": ask, "bid": bid} client.symbol_info_as_dict.return_value = { "digits": 2, "trade_stops_level": 10, @@ -533,88 +561,13 @@ class TestDetermineOrderLimits: result = determine_order_limits( client, "EURUSD", - "long", + side, stop_loss_limit_ratio=0.05, take_profit_limit_ratio=0.05, ) - _assert_close(result["stop_loss"], 0.95) - _assert_close(result["take_profit"], 1.05) - - def test_rejects_take_profit_inside_broker_stop_level(self) -> None: - """Test long take-profit inside trade_stops_level raises Mt5TradingError.""" - client = MagicMock() - client.symbol_info_tick_as_dict.return_value = {"ask": 1.0, "bid": 0.99} - client.symbol_info_as_dict.return_value = { - "digits": 2, - "trade_stops_level": 100, - "point": 0.0001, - } - - with pytest.raises(Mt5TradingError, match="Take profit for 'EURUSD'"): - determine_order_limits( - client, - "EURUSD", - "long", - take_profit_limit_ratio=0.0001, - ) - - def test_rejects_short_stop_loss_inside_broker_stop_level(self) -> None: - """Test short stop-loss inside trade_stops_level raises Mt5TradingError.""" - client = MagicMock() - client.symbol_info_tick_as_dict.return_value = {"ask": 1.01, "bid": 1.0} - client.symbol_info_as_dict.return_value = { - "digits": 2, - "trade_stops_level": 100, - "point": 0.0001, - } - - with pytest.raises(Mt5TradingError, match="Stop loss for 'EURUSD'"): - determine_order_limits( - client, - "EURUSD", - "short", - stop_loss_limit_ratio=0.0001, - ) - - def test_rejects_short_take_profit_inside_broker_stop_level(self) -> None: - """Test short take-profit inside trade_stops_level raises Mt5TradingError.""" - client = MagicMock() - client.symbol_info_tick_as_dict.return_value = {"ask": 1.01, "bid": 1.0} - client.symbol_info_as_dict.return_value = { - "digits": 2, - "trade_stops_level": 100, - "point": 0.0001, - } - - with pytest.raises(Mt5TradingError, match="Take profit for 'EURUSD'"): - determine_order_limits( - client, - "EURUSD", - "short", - take_profit_limit_ratio=0.0001, - ) - - def test_allows_short_protective_levels_beyond_broker_stop_level(self) -> None: - """Test short SL/TP beyond trade_stops_level pass validation.""" - client = MagicMock() - client.symbol_info_tick_as_dict.return_value = {"ask": 1.01, "bid": 1.0} - client.symbol_info_as_dict.return_value = { - "digits": 2, - "trade_stops_level": 10, - "point": 0.0001, - } - - result = determine_order_limits( - client, - "EURUSD", - "short", - stop_loss_limit_ratio=0.05, - take_profit_limit_ratio=0.05, - ) - - _assert_close(result["stop_loss"], 1.05) - _assert_close(result["take_profit"], 0.95) + _assert_close(result["stop_loss"], expected_sl) + _assert_close(result["take_profit"], expected_tp) def test_ignores_non_positive_broker_stop_level(self) -> None: """Test zero trade_stops_level skips stop-distance validation.""" @@ -992,44 +945,28 @@ class TestNormalizeOrderVolume: 0.34, ) - def test_returns_zero_for_non_finite_volume(self) -> None: + @pytest.mark.parametrize("volume", [float("nan"), float("inf")], ids=["nan", "inf"]) + def test_returns_zero_for_non_finite_volume(self, volume: float) -> None: """Test NaN or infinite requested volume returns zero.""" _assert_close( normalize_order_volume( - float("nan"), - volume_min=0.1, - volume_max=1.0, - volume_step=0.1, - ), - 0.0, - ) - _assert_close( - normalize_order_volume( - float("inf"), - volume_min=0.1, - volume_max=1.0, - volume_step=0.1, + volume, volume_min=0.1, volume_max=1.0, volume_step=0.1 ), 0.0, ) - def test_returns_zero_for_non_finite_constraints(self) -> None: + @pytest.mark.parametrize( + ("volume_min", "volume_step"), + [(float("nan"), 0.1), (0.1, float("inf"))], + ids=["nan-min", "inf-step"], + ) + def test_returns_zero_for_non_finite_constraints( + self, volume_min: float, volume_step: float + ) -> None: """Test NaN or infinite volume_min/volume_step returns zero.""" _assert_close( normalize_order_volume( - 1.0, - volume_min=float("nan"), - volume_max=1.0, - volume_step=0.1, - ), - 0.0, - ) - _assert_close( - normalize_order_volume( - 1.0, - volume_min=0.1, - volume_max=1.0, - volume_step=float("inf"), + 1.0, volume_min=volume_min, volume_max=1.0, volume_step=volume_step ), 0.0, ) @@ -1098,22 +1035,13 @@ class TestEstimateOrderMargin: with pytest.raises(Mt5TradingError, match="positive finite number"): estimate_order_margin(client, "EURUSD", "BUY", 0.0) - def test_rejects_nan_volume(self) -> None: - """Test NaN volume raises Mt5TradingError without broker calls.""" + @pytest.mark.parametrize("volume", [float("nan"), float("inf")], ids=["nan", "inf"]) + def test_rejects_non_finite_volume(self, volume: float) -> None: + """Test NaN or infinite volume raises Mt5TradingError without broker calls.""" client = _mock_trade_client() with pytest.raises(Mt5TradingError, match="positive finite number"): - estimate_order_margin(client, "EURUSD", "BUY", float("nan")) - - client.symbol_info_tick_as_dict.assert_not_called() - client.order_calc_margin.assert_not_called() - - def test_rejects_infinite_volume(self) -> None: - """Test infinite volume raises Mt5TradingError without broker calls.""" - client = _mock_trade_client() - - with pytest.raises(Mt5TradingError, match="positive finite number"): - estimate_order_margin(client, "EURUSD", "BUY", float("inf")) + estimate_order_margin(client, "EURUSD", "BUY", volume) client.symbol_info_tick_as_dict.assert_not_called() client.order_calc_margin.assert_not_called() @@ -1145,38 +1073,16 @@ class TestEstimateOrderMargin: with pytest.raises(Mt5TradingError, match="Tick price is unavailable"): estimate_order_margin(client, "EURUSD", "BUY", 0.1) - def test_rejects_invalid_margin_result(self) -> None: - """Test non-positive margin estimates raise Mt5TradingError.""" + @pytest.mark.parametrize( + "margin_value", + [0.0, float("inf"), None, "invalid"], + ids=["zero", "inf", "none", "string"], + ) + def test_rejects_invalid_margin_result(self, margin_value: object) -> None: + """Test invalid margin estimates raise Mt5TradingError.""" client = _mock_trade_client() client.symbol_info_tick_as_dict.return_value = {"ask": 1.1010, "bid": 1.1000} - client.order_calc_margin.return_value = 0.0 - - with pytest.raises(Mt5TradingError, match="Margin estimate is invalid"): - estimate_order_margin(client, "EURUSD", "BUY", 0.1) - - def test_rejects_non_finite_margin_result(self) -> None: - """Test non-finite margin estimates raise Mt5TradingError.""" - client = _mock_trade_client() - client.symbol_info_tick_as_dict.return_value = {"ask": 1.1010, "bid": 1.1000} - client.order_calc_margin.return_value = float("inf") - - with pytest.raises(Mt5TradingError, match="Margin estimate is invalid"): - estimate_order_margin(client, "EURUSD", "BUY", 0.1) - - def test_rejects_none_margin_result(self) -> None: - """Test None margin results raise Mt5TradingError.""" - client = _mock_trade_client() - client.symbol_info_tick_as_dict.return_value = {"ask": 1.1010, "bid": 1.1000} - client.order_calc_margin.return_value = None - - with pytest.raises(Mt5TradingError, match="Margin estimate is invalid"): - estimate_order_margin(client, "EURUSD", "BUY", 0.1) - - def test_rejects_non_numeric_margin_result(self) -> None: - """Test non-numeric margin results raise Mt5TradingError.""" - client = _mock_trade_client() - client.symbol_info_tick_as_dict.return_value = {"ask": 1.1010, "bid": 1.1000} - client.order_calc_margin.return_value = "invalid" + client.order_calc_margin.return_value = margin_value with pytest.raises(Mt5TradingError, match="Margin estimate is invalid"): estimate_order_margin(client, "EURUSD", "BUY", 0.1) @@ -1331,33 +1237,29 @@ class TestCalculatePositionsMargin: client.order_calc_margin.assert_not_called() client.symbol_info_tick_as_dict.assert_not_called() - def test_returns_zero_when_symbol_filter_matches_nothing(self) -> None: - """Test filtered symbol lists with no matches return zero.""" - client = _mock_trade_client() - client.positions_get_as_df.return_value = pd.DataFrame( - [{"symbol": "EURUSD", "type": 0, "volume": 0.1}], - ) - - _assert_close(calculate_positions_margin(client, symbols=["GBPUSD"]), 0.0) - client.order_calc_margin.assert_not_called() - - def test_returns_zero_for_empty_positions_with_symbol_filter(self) -> None: - """Test empty positions with a symbol filter return zero.""" - client = _mock_trade_client() - client.positions_get_as_df.return_value = pd.DataFrame() - - _assert_close(calculate_positions_margin(client, symbols=["EURUSD"]), 0.0) - - def test_returns_zero_for_positions_without_symbol_column_with_symbol_filter( + @pytest.mark.parametrize( + ("positions_records", "symbols_filter"), + [ + ([{"symbol": "EURUSD", "type": 0, "volume": 0.1}], ["GBPUSD"]), + (None, ["EURUSD"]), + ([{"type": 0, "volume": 0.1}], ["EURUSD"]), + ], + ids=["symbol_mismatch", "empty_df", "no_symbol_column"], + ) + def test_returns_zero_with_symbol_filter_and_no_match( self, + positions_records: list[dict[str, object]] | None, + symbols_filter: list[str], ) -> None: - """Test positions missing a symbol column return zero when filtered.""" + """Test symbol filter with no matching positions returns zero margin.""" client = _mock_trade_client() - client.positions_get_as_df.return_value = pd.DataFrame( - [{"type": 0, "volume": 0.1}], + client.positions_get_as_df.return_value = ( + pd.DataFrame(positions_records) + if positions_records is not None + else pd.DataFrame() ) - _assert_close(calculate_positions_margin(client, symbols=["EURUSD"]), 0.0) + _assert_close(calculate_positions_margin(client, symbols=symbols_filter), 0.0) client.order_calc_margin.assert_not_called() @@ -2300,107 +2202,48 @@ class TestVolumeAndExecution: assert result["retcode"] == 10009 client.order_send.assert_called_once() - def test_place_market_order_marks_failed_retcode(self) -> None: - """Test order_send responses with failed retcodes are normalized.""" - client = _mock_trade_client() - client.symbol_info_tick_as_dict.return_value = {"ask": 1.2, "bid": 1.1} - client.order_send.return_value = pd.DataFrame( - [{"retcode": 10013, "comment": "invalid request"}], - ) - - result = place_market_order( - client, - symbol="EURUSD", - volume=0.1, - order_side="BUY", - ) - - assert result["status"] == "failed" - assert result["retcode"] == 10013 - - def test_place_market_order_marks_failed_numpy_retcode(self) -> None: - """Test numpy integer retcodes normalize to failed status.""" - client = _mock_trade_client() - client.symbol_info_tick_as_dict.return_value = {"ask": 1.2, "bid": 1.1} - client.order_send.return_value = pd.DataFrame( - [{"retcode": np_int64(10013), "comment": "invalid request"}], - ) - - result = place_market_order( - client, - symbol="EURUSD", - volume=0.1, - order_side="BUY", - ) - - assert result["status"] == "failed" - assert result["retcode"] == 10013 - - def test_place_market_order_rejects_bool_retcode(self) -> None: - """Test bool retcodes are not treated as integer broker codes.""" - client = _mock_trade_client() - client.symbol_info_tick_as_dict.return_value = {"ask": 1.2, "bid": 1.1} - client.order_send.return_value = pd.DataFrame( - [{"retcode": True, "comment": "weird"}], - ) - - result = place_market_order( - client, - symbol="EURUSD", - volume=0.1, - order_side="BUY", - ) - - assert result["retcode"] is None - assert result["status"] == "failed" - - def test_place_market_order_marks_failed_string_retcode(self) -> None: - """Test digit-string failure retcodes normalize to failed status.""" - client = _mock_trade_client() - client.symbol_info_tick_as_dict.return_value = {"ask": 1.2, "bid": 1.1} - client.order_send.return_value = pd.DataFrame( - [{"retcode": "10013", "comment": "invalid request"}], - ) - - result = place_market_order( - client, - symbol="EURUSD", - volume=0.1, - order_side="BUY", - ) - - assert result["retcode"] == 10013 - assert result["status"] == "failed" - - def test_place_market_order_marks_failed_whitespace_string_retcode(self) -> None: - """Test whitespace-padded digit-string retcodes normalize to failed status.""" - client = _mock_trade_client() - client.symbol_info_tick_as_dict.return_value = {"ask": 1.2, "bid": 1.1} - client.order_send.return_value = pd.DataFrame( - [{"retcode": " 10013 ", "comment": "invalid request"}], - ) - - result = place_market_order( - client, - symbol="EURUSD", - volume=0.1, - order_side="BUY", - ) - - assert result["retcode"] == 10013 - assert result["status"] == "failed" - - @pytest.mark.parametrize("retcode", ["+10013", "-10013"]) - def test_place_market_order_marks_signed_string_retcode_as_failed( + @pytest.mark.parametrize( + ("raw_retcode", "expected_retcode"), + [ + (10013, 10013), + (np_int64(10013), 10013), + ("10013", 10013), + (" 10013 ", 10013), + ("+10013", 10013), + ("-10013", -10013), + (True, None), + ("invalid", None), + (" ", None), + (object(), None), + (_MISSING_RETCODE, None), + ], + # ids required: repr(object()) is non-deterministic, breaking --lf/-k + ids=[ + "int", + "np-int", + "str", + "str-padded", + "str-plus", + "str-minus", + "bool", + "malformed", + "empty-str", + "object", + "missing-key", + ], + ) + def test_place_market_order_normalizes_failed_retcode( self, - retcode: str, + raw_retcode: object, + expected_retcode: int | None, ) -> None: - """Test signed digit-string failure retcodes normalize to failed status.""" + """Test failed or malformed retcodes from order_send normalize correctly.""" client = _mock_trade_client() client.symbol_info_tick_as_dict.return_value = {"ask": 1.2, "bid": 1.1} - client.order_send.return_value = pd.DataFrame( - [{"retcode": retcode, "comment": "invalid request"}], - ) + response: dict[str, object] = {"comment": "x"} + if raw_retcode is not _MISSING_RETCODE: + response["retcode"] = raw_retcode + client.order_send.return_value = pd.DataFrame([response]) result = place_market_order( client, @@ -2409,80 +2252,7 @@ class TestVolumeAndExecution: order_side="BUY", ) - expected = 10013 if retcode.startswith("+") else -10013 - assert result["retcode"] == expected - assert result["status"] == "failed" - - def test_place_market_order_marks_missing_retcode_as_failed(self) -> None: - """Test live responses without retcode are fail-closed.""" - client = _mock_trade_client() - client.symbol_info_tick_as_dict.return_value = {"ask": 1.2, "bid": 1.1} - client.order_send.return_value = pd.DataFrame( - [{"comment": "missing retcode"}], - ) - - result = place_market_order( - client, - symbol="EURUSD", - volume=0.1, - order_side="BUY", - ) - - assert result["retcode"] is None - assert result["status"] == "failed" - - def test_place_market_order_marks_malformed_retcode_as_failed(self) -> None: - """Test malformed non-None retcodes are fail-closed.""" - client = _mock_trade_client() - client.symbol_info_tick_as_dict.return_value = {"ask": 1.2, "bid": 1.1} - client.order_send.return_value = pd.DataFrame( - [{"retcode": "invalid", "comment": "invalid request"}], - ) - - result = place_market_order( - client, - symbol="EURUSD", - volume=0.1, - order_side="BUY", - ) - - assert result["retcode"] is None - assert result["status"] == "failed" - - def test_place_market_order_marks_empty_string_retcode_as_failed(self) -> None: - """Test empty string retcodes are fail-closed.""" - client = _mock_trade_client() - client.symbol_info_tick_as_dict.return_value = {"ask": 1.2, "bid": 1.1} - client.order_send.return_value = pd.DataFrame( - [{"retcode": " ", "comment": "invalid request"}], - ) - - result = place_market_order( - client, - symbol="EURUSD", - volume=0.1, - order_side="BUY", - ) - - assert result["retcode"] is None - assert result["status"] == "failed" - - def test_place_market_order_marks_object_retcode_as_failed(self) -> None: - """Test unsupported retcode object types are fail-closed.""" - client = _mock_trade_client() - client.symbol_info_tick_as_dict.return_value = {"ask": 1.2, "bid": 1.1} - client.order_send.return_value = pd.DataFrame( - [{"retcode": object(), "comment": "invalid request"}], - ) - - result = place_market_order( - client, - symbol="EURUSD", - volume=0.1, - order_side="BUY", - ) - - assert result["retcode"] is None + assert result["retcode"] == expected_retcode assert result["status"] == "failed" def test_close_open_positions_filters_and_dry_runs(self) -> None: @@ -2702,25 +2472,22 @@ class TestVolumeAndExecution: == {} ) - def test_calculate_trailing_stop_updates_missing_symbol_digits(self) -> None: - """Test missing symbol digits fail safely without rounded updates.""" + @pytest.mark.parametrize( + "symbol_info", + [{}, {"digits": None}], + ids=["missing", "none"], + ) + def test_calculate_trailing_stop_updates_missing_symbol_digits( + self, + symbol_info: dict[str, object], + ) -> None: + """Test missing or None symbol digits fail safely without rounded updates.""" client = _mock_trade_client() client.positions_get_as_df.return_value = pd.DataFrame( [{"ticket": 1, "symbol": "EURUSD", "type": 0, "volume": 0.1, "sl": 1.0}], ) client.symbol_info_tick_as_dict.return_value = {"bid": 1.2, "ask": 1.201} - client.symbol_info_as_dict.return_value = {} - - assert ( - calculate_trailing_stop_updates( - client, - symbol="EURUSD", - trailing_stop_ratio=0.01, - ) - == {} - ) - - client.symbol_info_as_dict.return_value = {"digits": None} + client.symbol_info_as_dict.return_value = symbol_info assert ( calculate_trailing_stop_updates( @@ -3025,8 +2792,28 @@ class TestVolumeAndExecution: client.symbol_select.assert_called_once_with("EURUSD", enable=True) client.order_send.assert_called_once() - def test_update_sltp_marks_failed_retcode(self) -> None: - """Test SL/TP updates normalize failed broker retcodes.""" + @pytest.mark.parametrize( + ("raw_retcode", "expected_retcode"), + [ + (10013, 10013), + (np_int64(10013), 10013), + ("10013", 10013), + ("invalid", None), + (_MISSING_RETCODE, None), + ], + ids=["int", "np-int", "str", "malformed", "missing-key"], + ) + def test_update_sltp_normalizes_failed_retcode( + self, + raw_retcode: object, + expected_retcode: int | None, + ) -> None: + """Test failed or malformed SL/TP retcodes normalize correctly. + + Exhaustive retcode variants are covered in + test_place_market_order_normalizes_failed_retcode; this set is + representative because both functions share the same normalization path. + """ client = _mock_trade_client() client.symbol_info_as_dict.return_value = {"visible": True} client.positions_get_as_df.return_value = pd.DataFrame( @@ -3041,113 +2828,14 @@ class TestVolumeAndExecution: }, ], ) - client.order_send.return_value = pd.DataFrame( - [{"retcode": 10013, "comment": "invalid stops"}], - ) + response: dict[str, object] = {"comment": "x"} + if raw_retcode is not _MISSING_RETCODE: + response["retcode"] = raw_retcode + client.order_send.return_value = pd.DataFrame([response]) result = update_sltp_for_open_positions(client, tickets=[1], stop_loss=1.1) - assert result[0]["status"] == "failed" - assert result[0]["retcode"] == 10013 - - def test_update_sltp_marks_failed_numpy_retcode(self) -> None: - """Test numpy integer retcodes normalize to failed SL/TP status.""" - client = _mock_trade_client() - client.symbol_info_as_dict.return_value = {"visible": True} - client.positions_get_as_df.return_value = pd.DataFrame( - [ - { - "ticket": 1, - "symbol": "EURUSD", - "type": 0, - "volume": 0.1, - "sl": 1.0, - "tp": 1.4, - }, - ], - ) - client.order_send.return_value = pd.DataFrame( - [{"retcode": np_int64(10013), "comment": "invalid stops"}], - ) - - result = update_sltp_for_open_positions(client, tickets=[1], stop_loss=1.1) - - assert result[0]["status"] == "failed" - assert result[0]["retcode"] == 10013 - - def test_update_sltp_marks_failed_string_retcode(self) -> None: - """Test digit-string failure retcodes normalize to failed SL/TP status.""" - client = _mock_trade_client() - client.symbol_info_as_dict.return_value = {"visible": True} - client.positions_get_as_df.return_value = pd.DataFrame( - [ - { - "ticket": 1, - "symbol": "EURUSD", - "type": 0, - "volume": 0.1, - "sl": 1.0, - "tp": 1.4, - }, - ], - ) - client.order_send.return_value = pd.DataFrame( - [{"retcode": "10013", "comment": "invalid stops"}], - ) - - result = update_sltp_for_open_positions(client, tickets=[1], stop_loss=1.1) - - assert result[0]["retcode"] == 10013 - assert result[0]["status"] == "failed" - - def test_update_sltp_marks_malformed_retcode_as_failed(self) -> None: - """Test malformed non-None SL/TP retcodes are fail-closed.""" - client = _mock_trade_client() - client.symbol_info_as_dict.return_value = {"visible": True} - client.positions_get_as_df.return_value = pd.DataFrame( - [ - { - "ticket": 1, - "symbol": "EURUSD", - "type": 0, - "volume": 0.1, - "sl": 1.0, - "tp": 1.4, - }, - ], - ) - client.order_send.return_value = pd.DataFrame( - [{"retcode": "invalid", "comment": "invalid stops"}], - ) - - result = update_sltp_for_open_positions(client, tickets=[1], stop_loss=1.1) - - assert result[0]["retcode"] is None - assert result[0]["status"] == "failed" - - def test_update_sltp_marks_missing_retcode_as_failed(self) -> None: - """Test live SL/TP responses without retcode are fail-closed.""" - client = _mock_trade_client() - client.symbol_info_as_dict.return_value = {"visible": True} - client.positions_get_as_df.return_value = pd.DataFrame( - [ - { - "ticket": 1, - "symbol": "EURUSD", - "type": 0, - "volume": 0.1, - "sl": 1.0, - "tp": 1.4, - }, - ], - ) - client.order_send.return_value = pd.DataFrame( - [{"comment": "missing retcode"}], - ) - - result = update_sltp_for_open_positions(client, tickets=[1], stop_loss=1.1) - - assert result[0]["retcode"] is None + assert result[0]["retcode"] == expected_retcode assert result[0]["status"] == "failed" def test_trading_typed_dict_exports(self) -> None: @@ -3646,59 +3334,54 @@ class TestFetchLatestClosedRatesIndexed: class TestExtractTickPrice: """Tests for the public extract_tick_price helper.""" - def test_valid_positive_float(self) -> None: - """Returns a positive float value unchanged.""" - _assert_close(extract_tick_price({"bid": 1.1000}, "bid"), 1.1000) - - def test_valid_positive_int(self) -> None: - """Accepts an integer value and returns it as float.""" - result = extract_tick_price({"bid": 2}, "bid") - _assert_close(result, 2.0) + @pytest.mark.parametrize( + ("tick", "expected"), + [ + ({"bid": 1.1000}, 1.1000), + ({"bid": 2}, 2.0), + ({"bid": "1.5"}, 1.5), + ], + ids=["float", "int", "numeric-string"], + ) + def test_returns_valid_price( + self, tick: dict[str, object], expected: float + ) -> None: + """Returns a valid positive float for numeric inputs.""" + result = extract_tick_price(tick, "bid") + assert result is not None assert isinstance(result, float) + _assert_close(result, expected) - def test_valid_numeric_string(self) -> None: - """Accepts a numeric string and returns the parsed float.""" - _assert_close(extract_tick_price({"bid": "1.5"}, "bid"), 1.5) - - def test_missing_key(self) -> None: - """Returns None when the key is absent from the tick dict.""" - assert extract_tick_price({}, "bid") is None - - def test_none_value(self) -> None: - """Returns None when the stored value is None.""" - assert extract_tick_price({"bid": None}, "bid") is None - - def test_bool_value(self) -> None: - """Returns None for bool values even though bool is int-like.""" - assert extract_tick_price({"bid": True}, "bid") is None - - def test_invalid_string(self) -> None: - """Returns None for a non-numeric string.""" - assert extract_tick_price({"bid": "not_a_number"}, "bid") is None - - def test_nan(self) -> None: - """Returns None for a NaN float.""" - assert extract_tick_price({"bid": float("nan")}, "bid") is None - - def test_positive_infinity(self) -> None: - """Returns None for positive infinity.""" - assert extract_tick_price({"bid": float("inf")}, "bid") is None - - def test_negative_infinity(self) -> None: - """Returns None for negative infinity.""" - assert extract_tick_price({"bid": float("-inf")}, "bid") is None - - def test_zero(self) -> None: - """Returns None for zero (not a valid price).""" - assert extract_tick_price({"bid": 0.0}, "bid") is None - - def test_negative_value(self) -> None: - """Returns None for a negative price.""" - assert extract_tick_price({"bid": -1.0}, "bid") is None - - def test_unsupported_type(self) -> None: - """Returns None for unsupported value types such as list.""" - assert extract_tick_price({"bid": [1.0]}, "bid") is None + @pytest.mark.parametrize( + "tick", + [ + {}, + {"bid": None}, + {"bid": True}, + {"bid": "not_a_number"}, + {"bid": float("nan")}, + {"bid": float("inf")}, + {"bid": float("-inf")}, + {"bid": 0.0}, + {"bid": -1.0}, + {"bid": [1.0]}, + ], + ids=[ + "missing-key", + "none", + "bool", + "invalid-string", + "nan", + "inf", + "neg-inf", + "zero", + "negative", + "list", + ], + ) + def test_returns_none(self, tick: dict[str, object]) -> None: + """Returns None for missing, invalid, or non-positive price values.""" + assert extract_tick_price(tick, "bid") is None class TestCalculatePositionsMarginBySymbol: