test: consolidate duplicate test cases into parameterized tests (#91)
* test: consolidate duplicate test cases into parameterized tests - Combine test_symbol_info, test_symbol_info_tick, test_market_book into test_symbol_command with parametrize - Combine test_order_check variants into test_order_request with parametrize - Combine test_order_send variants into test_order_request with parametrize - Consolidate grafana view skip tests into test_grafana_view_skipped_when_required_cols_missing - Consolidate grafana index skip tests into test_index_skipped_when_cols_missing - Consolidate _NoOp method tests into test_method_is_noop - Consolidate record_*_update noop tests into test_record_update_noop_before_configure This improves test maintainability by reducing duplication while maintaining coverage. * test: parametrize remaining duplicated cases * test: parameterize remaining duplicate test cases * test: parameterize remaining duplicate test cases * test: parameterize remaining duplicate test cases * test: parameterize remaining duplicate test cases * test: parameterize count and start_pos validation tests * test: parameterize remaining duplicate test cases - tests/test_utils.py: merge valid string/integer cases for parse_timeframe and parse_tick_flags. - tests/test_cli.py: consolidate collect-history --if-exists append/fail tests and drop the duplicate collect-history --dataset ticks default flags=-1 assertion. - tests/test_trading.py: parametrize buy/sell projected margin ratio, invalid place_market_order mode validation, core calculate_volume_by_margin boundary cases, and zero-ratio calculate_margin_and_volume sizing cases. * test: parameterize remaining duplicate test cases Consolidate the last high-value duplicate test cases outside the already-changed areas while keeping semantics and 100% coverage. - tests/test_sdk.py:TestMt5CliClient - merge copy_rates_range, copy_ticks_from, history_orders, and latest_rates delegation/ normalization tests into test_method_delegates_with_normalization parameterized by call/expected_method/expected_kwargs with readable ids for each normalization path. - tests/test_sdk.py:TestSubstituteEnvPlaceholders - merge brace/whole-dollar/plain/partial substitution cases into test_substitute_env_placeholders, and missing-env cases into test_substitute_env_placeholders_raises_on_missing_env. - tests/test_trading.py:TestVolumeAndExecution - merge buy/sell trailing-stop main cases into test_calculate_trailing_stop_updates_by_side, opposite-side invalid-quote cases into test_calculate_trailing_stop_updates_ignores_opposite_side_price, and the two mixed-positions scenarios into the parametrized test_calculate_trailing_stop_updates_mixed_positions_skip_invalid_side. All ids explicitly describe the side and behavior. * test: parameterize collect-history and telemetry update tests Consolidate duplicate default-vs-ticks collect-history cases in CLI and SDK tests, and merge history/snapshot record_*_update success and failure tests in test_telemetry.py. Co-authored-by: Cursor <cursoragent@cursor.com> * test: parameterize telemetry record_*_state gauge tests Co-authored-by: Cursor <cursoragent@cursor.com> * test: parameterize Grafana example file checks * test: centralize remaining parametrized cases * test: add parametrized replacements for remaining cases * test: fix remaining parametrized lint * Fix Ruff warnings * test: remove duplicate parametrization sweep file * test: remove duplicate-test deselection hook * test: restore conftest formatting * test: add remaining parametrized regression coverage * test: fix lint issues in parametrization completion tests * test: avoid dynamic SQL in parametrization completion tests * test: format parametrization completion tests * test: fix lint and pyright issues in parametrization coverage * Fix a Pyright error * Update .agents/skills/pr-feedback-triage/SKILL.md * test: dissolve parametrize-completion catch-all into owning test files Move the remaining consolidated cases from test_parametrize_completion.py into their owning modules and delete the file: - test_cli.py: merge snapshot/grafana-schema --publish-copy gating into one parametrized test. - test_trading.py: merge calculate_spread_ratio numeric/numeric-string cases; fold non-finite volume_max into test_normalize_order_volume_deterministic; merge estimate_order_margin invalid-volume cases (0.0/nan/inf); merge calculate_positions_margin invalid-row filtering cases. - test_history.py: parametrize resolve_history_tick_flags and resolve_granularity_name. - test_sdk.py: parametrize collect_latest_closed_rates_for_accounts empty-effective-frame cases. - test_grafana.py: merge account/terminal snapshot insert tests inside TestSnapshotInserts. All remaining cases in the removed file were exact duplicates of existing coverage, so no new test scenarios are introduced. 100% coverage and all checks (ruff, pyright, pytest, local-qa) pass. * test: parameterize remaining account-event, snapshot, and CLI edge cases Consolidates six more pairs of duplicate tests flagged during PR #91 review into pytest.mark.parametrize tables (incremental history_deals account-event edge cases, dedup scope column variants, snapshot view status filtering, snapshot kwargs forwarding, injected-client lifecycle, and close-positions --yes gate), preserving explicit ids and 100% coverage. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test: consolidate build_config login coercion into parametrized cases Fold separate backward-compat and env-expansion tests into named pytest.param rows for clearer failure output. Co-authored-by: Cursor <cursoragent@cursor.com> * test: consolidate deduplication and publish test cases into parametrized variants Merge related test cases into single parametrized tests: - Consolidate collect-history validation tests into a single parametrized case - Merge snapshot-view skip conditions into one test with parametrized inputs - Combine publish_grafana_copy target variations into one parametrized test - Consolidate incremental-start and drop-duplicates error cases - Remove duplicate test_drop_duplicates_rejects_invalid_identifiers Reduces test file duplication while maintaining full coverage. * test: consolidate remaining review-identified parameterized duplicates Merge close-position filter, margin-ratio suppress/reraise, SQLite path validation, and grafana_symbol_pnl schema cases into clearer parametrized tests. Co-authored-by: Cursor <cursoragent@cursor.com> * test: restore default if_exists coverage in SQLite append test The default-append parametrized case now omits if_exists so the API default remains exercised instead of passing IfExists.APPEND explicitly. Co-authored-by: Cursor <cursoragent@cursor.com> * test: tighten trading parametrization cleanup * test: consolidate remaining parameterized cases * test: consolidate additional duplicate cases into parametrized tests Merge overlapping success-path tests in contracts, history, SDK, and trading modules while preserving 100% coverage and explicit test ids. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: agent <agent@localhost> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
+163
-100
@@ -20,23 +20,13 @@ from mt5cli.telemetry import (
|
||||
class TestNoOp:
|
||||
"""Tests for _NoOp no-op instrument."""
|
||||
|
||||
def test_add_is_noop(self) -> None:
|
||||
"""_NoOp.add accepts amount and optional attributes without error."""
|
||||
@pytest.mark.parametrize("method", ["add", "set", "record"])
|
||||
def test_method_is_noop(self, method: str) -> None:
|
||||
"""_NoOp methods accept amount and optional attributes without error."""
|
||||
noop = _NoOp()
|
||||
noop.add(1.0)
|
||||
noop.add(1.0, {"key": "val"})
|
||||
|
||||
def test_set_is_noop(self) -> None:
|
||||
"""_NoOp.set accepts amount and optional attributes without error."""
|
||||
noop = _NoOp()
|
||||
noop.set(2.0)
|
||||
noop.set(2.0, {"key": "val"})
|
||||
|
||||
def test_record_is_noop(self) -> None:
|
||||
"""_NoOp.record accepts amount and optional attributes without error."""
|
||||
noop = _NoOp()
|
||||
noop.record(3.0)
|
||||
noop.record(3.0, {"key": "val"})
|
||||
bound = getattr(noop, method)
|
||||
bound(1.0)
|
||||
bound(1.0, {"key": "val"})
|
||||
|
||||
|
||||
class TestMt5Metrics:
|
||||
@@ -64,32 +54,105 @@ class TestMt5Metrics:
|
||||
assert meter.create_counter.called
|
||||
assert meter.create_gauge.called
|
||||
|
||||
def test_record_history_update_success(self) -> None:
|
||||
"""record_history_update records duration and timestamp on success."""
|
||||
@pytest.mark.parametrize(
|
||||
(
|
||||
"method",
|
||||
"kwargs",
|
||||
"duration_attr",
|
||||
"failures_attr",
|
||||
"last_success_attr",
|
||||
),
|
||||
[
|
||||
pytest.param(
|
||||
"record_history_update",
|
||||
{"dataset": "rates"},
|
||||
"_history_duration",
|
||||
"_history_failures",
|
||||
"_last_successful_update",
|
||||
id="history-update",
|
||||
),
|
||||
pytest.param(
|
||||
"record_snapshot_update",
|
||||
{},
|
||||
"_snapshot_duration",
|
||||
"_snapshot_failures",
|
||||
None,
|
||||
id="snapshot-update",
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_record_update_success(
|
||||
self,
|
||||
method: str,
|
||||
kwargs: dict[str, str],
|
||||
duration_attr: str,
|
||||
failures_attr: str,
|
||||
last_success_attr: str | None,
|
||||
) -> None:
|
||||
"""record_*_update records duration and timestamp on success."""
|
||||
meter = MagicMock()
|
||||
m = _Mt5Metrics()
|
||||
m.configure(meter)
|
||||
with m.record_history_update(dataset="rates"):
|
||||
with getattr(m, method)(**kwargs):
|
||||
pass
|
||||
m._history_duration.record.assert_called_once() # type: ignore[reportPrivateUsage]
|
||||
m._last_successful_update.set.assert_called_once() # type: ignore[reportPrivateUsage]
|
||||
m._history_failures.add.assert_not_called() # type: ignore[reportPrivateUsage]
|
||||
getattr(m, duration_attr).record.assert_called_once() # type: ignore[reportPrivateUsage]
|
||||
getattr(m, failures_attr).add.assert_not_called() # type: ignore[reportPrivateUsage]
|
||||
if last_success_attr is not None:
|
||||
getattr(m, last_success_attr).set.assert_called_once() # type: ignore[reportPrivateUsage]
|
||||
|
||||
def test_record_history_update_failure(self) -> None:
|
||||
"""record_history_update increments failure counter and re-raises on error."""
|
||||
@pytest.mark.parametrize(
|
||||
(
|
||||
"method",
|
||||
"kwargs",
|
||||
"exc",
|
||||
"duration_attr",
|
||||
"failures_attr",
|
||||
"failure_labels",
|
||||
),
|
||||
[
|
||||
pytest.param(
|
||||
"record_history_update",
|
||||
{"dataset": "rates"},
|
||||
ValueError("boom"),
|
||||
"_history_duration",
|
||||
"_history_failures",
|
||||
{"dataset": "rates"},
|
||||
id="history-update",
|
||||
),
|
||||
pytest.param(
|
||||
"record_snapshot_update",
|
||||
{},
|
||||
RuntimeError("snap fail"),
|
||||
"_snapshot_duration",
|
||||
"_snapshot_failures",
|
||||
{},
|
||||
id="snapshot-update",
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_record_update_failure(
|
||||
self,
|
||||
method: str,
|
||||
kwargs: dict[str, str],
|
||||
exc: BaseException,
|
||||
duration_attr: str,
|
||||
failures_attr: str,
|
||||
failure_labels: dict[str, str],
|
||||
) -> None:
|
||||
"""record_*_update increments failure counter and re-raises on error."""
|
||||
meter = MagicMock()
|
||||
m = _Mt5Metrics()
|
||||
m.configure(meter)
|
||||
exc = ValueError("boom")
|
||||
with (
|
||||
pytest.raises(ValueError, match="boom"),
|
||||
m.record_history_update(dataset="rates"),
|
||||
pytest.raises(type(exc), match=str(exc)),
|
||||
getattr(m, method)(**kwargs),
|
||||
):
|
||||
raise exc
|
||||
m._history_failures.add.assert_called_once_with( # type: ignore[reportPrivateUsage]
|
||||
1, {"dataset": "rates"}
|
||||
getattr(m, failures_attr).add.assert_called_once_with( # type: ignore[reportPrivateUsage]
|
||||
1,
|
||||
failure_labels,
|
||||
)
|
||||
m._history_duration.record.assert_not_called() # type: ignore[reportPrivateUsage]
|
||||
getattr(m, duration_attr).record.assert_not_called() # type: ignore[reportPrivateUsage]
|
||||
|
||||
def test_add_history_rows(self) -> None:
|
||||
"""add_history_rows increments the rows-written counter."""
|
||||
@@ -101,82 +164,82 @@ class TestMt5Metrics:
|
||||
42, {"dataset": "rates"}
|
||||
)
|
||||
|
||||
def test_record_snapshot_update_success(self) -> None:
|
||||
"""record_snapshot_update records duration on success."""
|
||||
@pytest.mark.parametrize(
|
||||
("method", "kwargs", "gauge_attr", "expected_set_count"),
|
||||
[
|
||||
pytest.param(
|
||||
"record_position_state",
|
||||
{
|
||||
"login": "42",
|
||||
"server": "demo",
|
||||
"symbol": "EURUSD",
|
||||
"profit": 12.5,
|
||||
"volume": 0.01,
|
||||
},
|
||||
"_position_profit",
|
||||
2,
|
||||
id="position-state",
|
||||
),
|
||||
pytest.param(
|
||||
"record_terminal_state",
|
||||
{
|
||||
"connected": 1.0,
|
||||
"trade_allowed": 1.0,
|
||||
"trade_expert": 0.0,
|
||||
},
|
||||
"_terminal_connected",
|
||||
3,
|
||||
id="terminal-state",
|
||||
),
|
||||
pytest.param(
|
||||
"record_account_state",
|
||||
{
|
||||
"login": "99",
|
||||
"server": "live",
|
||||
"balance": 5000.0,
|
||||
"equity": 5100.0,
|
||||
"margin": 200.0,
|
||||
"margin_free": 4800.0,
|
||||
"margin_level": 2550.0,
|
||||
},
|
||||
"_account_balance",
|
||||
5,
|
||||
id="account-state",
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_record_state_emits_gauges(
|
||||
self,
|
||||
method: str,
|
||||
kwargs: dict[str, float | str],
|
||||
gauge_attr: str,
|
||||
expected_set_count: int,
|
||||
) -> None:
|
||||
"""record_*_state emits the expected gauge set calls after configure."""
|
||||
meter = MagicMock()
|
||||
m = _Mt5Metrics()
|
||||
m.configure(meter)
|
||||
with m.record_snapshot_update():
|
||||
pass
|
||||
m._snapshot_duration.record.assert_called_once() # type: ignore[reportPrivateUsage]
|
||||
m._snapshot_failures.add.assert_not_called() # type: ignore[reportPrivateUsage]
|
||||
|
||||
def test_record_snapshot_update_failure(self) -> None:
|
||||
"""record_snapshot_update increments failure counter and re-raises on error."""
|
||||
meter = MagicMock()
|
||||
m = _Mt5Metrics()
|
||||
m.configure(meter)
|
||||
exc = RuntimeError("snap fail")
|
||||
with (
|
||||
pytest.raises(RuntimeError, match="snap fail"),
|
||||
m.record_snapshot_update(),
|
||||
):
|
||||
raise exc
|
||||
m._snapshot_failures.add.assert_called_once_with(1, {}) # type: ignore[reportPrivateUsage]
|
||||
m._snapshot_duration.record.assert_not_called() # type: ignore[reportPrivateUsage]
|
||||
|
||||
def test_record_position_state(self) -> None:
|
||||
"""record_position_state emits profit and volume gauges."""
|
||||
meter = MagicMock()
|
||||
m = _Mt5Metrics()
|
||||
m.configure(meter)
|
||||
m.record_position_state(
|
||||
login="42",
|
||||
server="demo",
|
||||
symbol="EURUSD",
|
||||
profit=12.5,
|
||||
volume=0.01,
|
||||
getattr(m, method)(**kwargs)
|
||||
# Related gauges share the same create_gauge mock; verify total set calls.
|
||||
assert ( # type: ignore[reportPrivateUsage]
|
||||
getattr(m, gauge_attr).set.call_count == expected_set_count
|
||||
)
|
||||
# Both profit and volume share the same gauge mock via create_gauge.
|
||||
# Verify that set was called exactly twice (once each).
|
||||
assert m._position_profit.set.call_count == 2 # type: ignore[reportPrivateUsage]
|
||||
|
||||
def test_record_terminal_state(self) -> None:
|
||||
"""record_terminal_state emits connected, trade_allowed, trade_expert gauges."""
|
||||
meter = MagicMock()
|
||||
@pytest.mark.parametrize(
|
||||
("method", "kwargs"),
|
||||
[
|
||||
("record_history_update", {"dataset": "ticks"}),
|
||||
("record_snapshot_update", {}),
|
||||
],
|
||||
)
|
||||
def test_record_update_noop_before_configure(
|
||||
self,
|
||||
method: str,
|
||||
kwargs: dict[str, str],
|
||||
) -> None:
|
||||
"""record_*_update works without configure (no-op instruments)."""
|
||||
m = _Mt5Metrics()
|
||||
m.configure(meter)
|
||||
m.record_terminal_state(connected=1.0, trade_allowed=1.0, trade_expert=0.0)
|
||||
# All three terminal gauges share the same mock; set is called 3 times.
|
||||
assert m._terminal_connected.set.call_count == 3 # type: ignore[reportPrivateUsage]
|
||||
|
||||
def test_record_account_state_after_configure(self) -> None:
|
||||
"""record_account_state emits all five account gauges."""
|
||||
meter = MagicMock()
|
||||
m = _Mt5Metrics()
|
||||
m.configure(meter)
|
||||
m.record_account_state(
|
||||
login="99",
|
||||
server="live",
|
||||
balance=5000.0,
|
||||
equity=5100.0,
|
||||
margin=200.0,
|
||||
margin_free=4800.0,
|
||||
margin_level=2550.0,
|
||||
)
|
||||
# All five account gauges share the same gauge mock; set is called 5 times.
|
||||
assert m._account_balance.set.call_count == 5 # type: ignore[reportPrivateUsage]
|
||||
|
||||
def test_record_history_update_noop_before_configure(self) -> None:
|
||||
"""record_history_update works without configure (no-op instruments)."""
|
||||
m = _Mt5Metrics()
|
||||
with m.record_history_update(dataset="ticks"):
|
||||
pass
|
||||
|
||||
def test_record_snapshot_update_noop_before_configure(self) -> None:
|
||||
"""record_snapshot_update works without configure (no-op instruments)."""
|
||||
m = _Mt5Metrics()
|
||||
with m.record_snapshot_update():
|
||||
with getattr(m, method)(**kwargs):
|
||||
pass
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user