main
2 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Daichi Narushima
|
a5c2aa8e8f |
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> |
||
|
Daichi Narushima
|
1ffac45d57 |
feat: add publish_grafana_copy, Grafana examples, and optional OTel metrics (#89)
* feat: Grafana copy publishing, dashboard examples, and optional OTel metrics Implements three observability improvements: #82 — publish_grafana_copy(): Uses SQLite online backup API (WAL-safe) to atomically publish a consistent read-only copy beside the target. Adds --publish-copy option to grafana-schema and snapshot CLI commands. #83 — examples/grafana/: Minimal working Grafana setup with docker-compose, provisioning datasource/dashboard YAML, and three dashboard JSON files (mt5cli-overview, mt5cli-trades, mt5cli-market). All queries use grafana_* views; no credentials or private paths included. #84 — mt5cli/telemetry.py: Optional OTel metrics behind mt5cli[otel] extra. Base install is unaffected. Adds _Mt5Metrics singleton (no-op until configure_metrics() is called), wraps update_history() and update_observability() with record_history_update / record_snapshot_update context managers, and emits account/position gauges from snapshots. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: replace ambiguous multiplication sign in comment Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: normalize markdown formatting in grafana README Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: preserve file mode on Grafana copy and fix unsupported time macro - publish_grafana_copy: chmod temp file to match the existing target's permissions (or 0o644 when no prior target exists) before atomic replace, so Grafana running as a different OS user (e.g. UID 472 in Docker) can read the published database - mt5cli-market.json: replace unsupported \$__timeFilter(time) with the epoch-based filter supported by frser-sqlite-datasource: "time" >= \$__from / 1000 AND "time" < \$__to / 1000 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: skip Windows-incompatible mode test, rename compose file to compose.yaml - Skip test_overwrite_preserves_existing_target_mode on win32 since Windows chmod does not preserve Unix group/other permission bits - Simplify test_fresh_target_has_readable_permissions to check owner read bit only (portable across platforms) - Rename docker-compose.yml -> compose.yaml (modern Compose convention) - Update README and test reference to match new filename Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: rename *.yaml to *.yml in examples/grafana Renames compose.yaml, mt5cli-sqlite.yaml, and mt5cli.yaml to .yml; updates README and test references accordingly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore: format Grafana dashboards and expand qa script to include JSON - Update qa.sh prettier pattern to format JSON files alongside markdown - Reformat Grafana dashboard JSONs with consistent spacing Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address owner review comments before merge - qa.sh: fix Prettier glob from `{,d,json}` to `{md,json}` so Markdown files are actually formatted by local QA (P2) - compose.yml: add GF_INSTALL_PLUGINS env var so the frser-sqlite-datasource plugin is installed at container start (P1) - telemetry.py: replace no-op get_meter() call with a real SDK MeterProvider pipeline; add optional `readers` kwarg so callers can inject custom readers (e.g. InMemoryMetricReader in tests) without needing the OTLP package (P1) - sdk.py: aggregate profit and volume by symbol before emitting gauge values so hedging accounts with multiple same-symbol positions emit one point per symbol instead of overwriting with each row (P2) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: emit mt5_history_update_rows_total via conn.total_changes delta The counter was registered but never incremented, making the advertised history-update throughput metric permanently zero. Add add_history_rows() to _Mt5Metrics and call it in update_history() using the SQLite total_changes delta measured around write_incremental_datasets(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address three owner review comments - compose.yml: replace soft fallback with :? error expansion so Compose refuses to start when MT5CLI_DB_PATH is unset or empty (P1) - README.md: tell native Windows users to copy only the datasource provisioning file; the dashboards yml contains a Docker-specific path that is invalid on Windows (P2) - telemetry.py / sdk.py: emit mt5_terminal_connected, mt5_terminal_trade_allowed, and mt5_terminal_trade_expert gauges via a new record_terminal_state() method called from _snapshot_terminal(), completing the connection-status metric surface from issue #84 (P2) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat: add snapshot freshness panel and win-rate column to dashboards - mt5cli-overview.json: add a full-width "Last Snapshot" stat panel (dateTimeFromNow unit) below the account stats, querying MAX(time)*1000 from grafana_account_snapshots so users can tell whether Grafana is reading a current published copy (#83) - mt5cli-trades.json: add win_rate_pct computed column to the Trade Statistics by Symbol table via 100.0 * winning_deals / NULLIF( total_deals, 0), with a percent unit override and "Win Rate (%)" display label (#83) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: reject same source and target path in publish_grafana_copy Adds an early same-path guard to publish_grafana_copy: resolves both paths before any I/O and raises ValueError if they are identical, preventing the function from overwriting the live source database with its own backup copy. Also adds a unit test for the rejected case. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address ruff EM102/TRY003/E501 in same-path guard Assigns the ValueError message to a variable before raising and shortens the test docstring to stay within the 88-char line limit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: apply ruff format to publish_grafana_copy error message Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: remove grafana_ticks panel from default market dashboard The Tick Bid/Ask panel queried grafana_ticks which only exists when users collect tick data (opt-in). Users following the default OHLCV-only setup path hit "no such table: grafana_ticks" on dashboard load. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: close SQLite connections before atomic replace in publish_grafana_copy Wrap both src and dst connections with contextlib.closing() so they are explicitly closed before tmp_path.replace(target_path) runs. Without this, sqlite3.Connection's context manager only commits/rolls back but leaves the file handle open, which can cause PermissionError on Windows. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: rename history.grafana.db to history.mt5cli.db in Grafana examples frser-sqlite-datasource blocks paths containing "grafana.db" via its internal blocklist. Rename the recommended published filename in the README, compose comment, and datasource provisioning comment to avoid a blocked/denied datasource for native Windows users. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: update Docker Compose quick-start to pass MT5CLI_DB_PATH The compose.yml already required MT5CLI_DB_PATH via ${MT5CLI_DB_PATH:?...}, but the README still showed bare `docker compose up -d`. Update the section to show the env-var-prefixed invocation and document the .env file alternative. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: agent <agent@localhost> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |