test: add explicit unit tests for calculate_positions_margin_by_symbol and calculate_positions_margin_safe (#50) (#53)

* test: add explicit unit tests for calculate_positions_margin_by_symbol and calculate_positions_margin_safe (#50)

Covers all acceptance criteria: partial failure with warning log, all-fail,
empty symbol list with no-broker-call assertion, duplicate deduplication,
successful aggregation with first-seen key order, suppress_errors=False
propagation, and three calculate_positions_margin_safe cases (partial skip,
all-fail → 0.0, empty list → 0.0).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* test: fix warning log assertion and parametrize suppress_errors=False test

- Use record.getMessage() + levelno check instead of record.message, which
  is only populated after formatting and can return an empty string.
- Parametrize test_one_symbol_fails_suppress_errors_false over all three
  exception types caught by the implementation (Mt5TradingError,
  Mt5RuntimeError, AttributeError) so any future narrowing of the except
  tuple would be caught by tests.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* style: shorten docstring to fit 88-char line limit

* style: shorten docstring to fit 88-char line limit

---------

Co-authored-by: agent <agent@localhost>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

tests/test_trading.py

@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import logging
 from types import SimpleNamespace
 from typing import Any, cast
 from unittest.mock import MagicMock
@@ -3135,7 +3136,7 @@ class TestCalculatePositionsMarginBySymbol:
     """Tests for calculate_positions_margin_by_symbol (#50)."""
 
     def test_all_symbols_succeed(self, mocker: MockerFixture) -> None:
-        """Returns one entry per symbol when all margin calls succeed."""
+        """Returns one entry per symbol in first-seen order when all calls succeed."""
         client = _mock_trade_client()
         mocker.patch(
             "mt5cli.trading.calculate_positions_margin",
@@ -3146,33 +3147,50 @@ class TestCalculatePositionsMarginBySymbol:
             client, symbols=["EURUSD", "GBPUSD"]
         )
 
-        assert result == {"EURUSD": 12.5, "GBPUSD": 30.0}
+        assert list(result.keys()) == ["EURUSD", "GBPUSD"]
+        _assert_close(result["EURUSD"], 12.5)
+        _assert_close(result["GBPUSD"], 30.0)
 
-    def test_one_symbol_fails_suppress_errors_true(self, mocker: MockerFixture) -> None:
+    def test_one_symbol_fails_suppress_errors_true(
-        """Skips the failing symbol and returns the successful one."""
+        self, mocker: MockerFixture, caplog: pytest.LogCaptureFixture
+    ) -> None:
+        """Skips the failing symbol, emits a warning, and returns the successful one."""
         client = _mock_trade_client()
         mocker.patch(
             "mt5cli.trading.calculate_positions_margin",
             side_effect=[Mt5TradingError("tick unavailable"), 30.0],
         )
 
-        result = calculate_positions_margin_by_symbol(
+        with caplog.at_level(logging.WARNING, logger="mt5cli.trading"):
-            client, symbols=["EURUSD", "GBPUSD"], suppress_errors=True
+            result = calculate_positions_margin_by_symbol(
-        )
+                client, symbols=["EURUSD", "GBPUSD"], suppress_errors=True
+            )
 
         assert result == {"GBPUSD": 30.0}
+        assert any(
+            "EURUSD" in record.getMessage() and record.levelno == logging.WARNING
+            for record in caplog.records
+        )
 
+    @pytest.mark.parametrize(
+        "exc",
+        [
+            Mt5TradingError("trading error"),
+            Mt5RuntimeError("runtime error"),
+            AttributeError("missing attr"),
+        ],
+    )
     def test_one_symbol_fails_suppress_errors_false(
-        self, mocker: MockerFixture
+        self, mocker: MockerFixture, exc: Exception
     ) -> None:
-        """Re-raises the first failure when suppress_errors=False."""
+        """Re-raises the first failure for each caught exception type."""
         client = _mock_trade_client()
         mocker.patch(
             "mt5cli.trading.calculate_positions_margin",
-            side_effect=[Mt5TradingError("tick unavailable"), 30.0],
+            side_effect=[exc, 30.0],
         )
 
-        with pytest.raises(Mt5TradingError, match="tick unavailable"):
+        with pytest.raises(type(exc)):
             calculate_positions_margin_by_symbol(
                 client, symbols=["EURUSD", "GBPUSD"], suppress_errors=False
             )
@@ -3191,11 +3209,13 @@ class TestCalculatePositionsMarginBySymbol:
 
         assert result == {}
 
-    def test_empty_symbol_list(self) -> None:
+    def test_empty_symbol_list(self, mocker: MockerFixture) -> None:
-        """Returns an empty dict for an empty input list."""
+        """Returns an empty dict for an empty input list without any broker calls."""
         client = _mock_trade_client()
+        mock_calc = mocker.patch("mt5cli.trading.calculate_positions_margin")
         result = calculate_positions_margin_by_symbol(client, symbols=[])
         assert result == {}
+        mock_calc.assert_not_called()
 
     def test_duplicate_symbols_preserve_first_seen_order(
         self, mocker: MockerFixture
@@ -3229,3 +3249,33 @@ class TestCalculatePositionsMarginSafe:
         total = calculate_positions_margin_safe(client, symbols=["EURUSD", "GBPUSD"])
 
         _assert_close(total, 42.5)
+
+    def test_partial_failure_skips_and_sums(self, mocker: MockerFixture) -> None:
+        """Sums only successful margins when one symbol raises."""
+        client = _mock_trade_client()
+        mocker.patch(
+            "mt5cli.trading.calculate_positions_margin",
+            side_effect=[Mt5TradingError("tick unavailable"), 30.0],
+        )
+
+        total = calculate_positions_margin_safe(client, symbols=["EURUSD", "GBPUSD"])
+
+        _assert_close(total, 30.0)
+
+    def test_all_symbols_fail_returns_zero(self, mocker: MockerFixture) -> None:
+        """Returns 0.0 when every symbol raises."""
+        client = _mock_trade_client()
+        mocker.patch(
+            "mt5cli.trading.calculate_positions_margin",
+            side_effect=[Mt5TradingError("err1"), Mt5RuntimeError("err2")],
+        )
+
+        total = calculate_positions_margin_safe(client, symbols=["EURUSD", "GBPUSD"])
+
+        _assert_close(total, 0.0)
+
+    def test_empty_symbols_returns_zero(self) -> None:
+        """Returns 0.0 for an empty symbol list."""
+        client = _mock_trade_client()
+        total = calculate_positions_margin_safe(client, symbols=[])
+        _assert_close(total, 0.0)