Add tests for log parsing and graph generation - #161
Conversation
7be4475 to
0f04fdc
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds unit and smoke tests covering previously untested, logic-heavy log parsing and static graph generation utilities in sync_tests, improving confidence in node/db-sync metrics extraction and report graph outputs.
Changes:
- Add unit tests for
sync_static_graphsmode detection, db-sync normalization, and PNG output generation (node + dbsync). - Add unit tests for node log metrics extraction (JSON tracer + human-readable tracer formats) and CPU/tip carry-forward behavior.
- Add unit tests for db-sync log metrics extraction (duration parsing + full epoch cycle) and for
log_analyzerstring detection / rollback detection / warnings.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| sync_tests/tests/test_sync_static_graphs.py | Covers JSON mode detection, db-sync normalization branches, and end-to-end PNG generation smoke tests. |
| sync_tests/tests/test_metrics_extractor_node.py | Adds coverage for node log parsing across tracer formats and CPU delta behavior. |
| sync_tests/tests/test_metrics_extractor_dbsync.py | Adds coverage for db-sync duration parsing and epoch/block parsing across a full epoch lifecycle. |
| sync_tests/tests/test_log_analyzer.py | Adds coverage for log string matching, rollback detection behavior, and warning emission. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mkoura
left a comment
There was a problem hiding this comment.
Differentiate between framework tests and actual sync tests.
Added unit tests for the log-parsing and graph-generation code, previously the most logic-dense modules in the repo with zero coverage: - log_analyzer.py: string search, rollback detection, and the combined check_db_sync_logs() warnings - logs/metrics_extractor.py: node log parsing for both the JSON (CentiCpu) and human-readable (Cpu Ticks) tracer formats, CPU delta calculation, and tip/CPU carry-forward between samples - db_sync/metrics_extractor.py: duration string parsing and a full epoch cycle (start, block inserts, stats, completion) - sync_static_graphs.py: detect_json_mode() and normalize_dbsync_data() branch coverage, plus an end-to-end smoke test confirming generate_static_graphs() produces real PNG files from sample data Found and documented one existing quirk while writing these: are_rollbacks_present_in_logs() only returns True once "rolling" appears a second time in the log, so a single rollback event is not reported. Left as-is and covered by a test describing the current behavior, this may be intentional and is worth checking with the team rather than changing silently.
generate_static_graphs() only had a smoke test for dbsync mode. Added the matching node-mode test, verified against real production node_sync_results.json data as well (26k log samples, all 5 documented node PNGs generated correctly).
Addresses both review comments: Martin: differentiate framework tests from actual sync tests. These four files are pure unit tests of our own code (log parsing, metrics extraction, graph generation) with no live node/db-sync dependency, so they belong in framework_tests/ (added in #165), not sync_tests/tests/: - test_log_analyzer.py - test_metrics_extractor_dbsync.py - test_metrics_extractor_node.py - test_sync_static_graphs.py Copilot: the comment explaining why epoch 3 is kept in test_epoch_timings_keeps_entries_with_valid_duration_or_blocks was contradictory. Reworded to state directly that has_valid_blocks treats any blocks_count >= 0, including 0, as valid, which is why epoch 3 survives despite duration_sec=0. Verified: ruff, ruff format, and mypy pass (47 source files); full suite collects 72 tests, 53 passed, 8 skipped (unrelated snapshot tests missing CLI args), 11 deselected (live-infra markers).
b9b42e2 to
5fbf84c
Compare
|
Moved test_log_analyzer.py, test_metrics_extractor_dbsync.py, test_metrics_extractor_node.py, and test_sync_static_graphs.py into framework_tests/ (rebased onto main, which now has that directory from #165). These are pure unit tests of our own code with no live node/db-sync dependency. |
Summary
Added unit tests for the log-parsing and graph-generation code, previously the most logic-dense modules in the repo with zero coverage.
Found and documented one existing quirk while writing these: are_rollbacks_present_in_logs() only returns True once "rolling" appears a second time in the log, so a single rollback event is not reported. Left as-is and covered by a test describing the current behavior since this may be intentional, worth confirming with the team rather than changing silently.
Test plan