Add targeted coverage tests for tools and core helpers#1208
Add targeted coverage tests for tools and core helpers#1208AdvancedImagingUTSW merged 3 commits intodevelopfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the reliability and completeness of the navigate test suite by adding focused unit tests around tools helpers and a few core runtime helpers, while also fixing CLI argument handling for waveform_constants_file in src/navigate/tools/main_functions.py.
Changes:
- Add targeted tests for tool helpers (slicing, SDF volume generation, linear algebra, decorators, file functions), plus Tk thread guard and headless GUI-adjacent utilities.
- Add tests for core runtime helpers (main entrypoint branch behavior, logging filters/functions, thread warning handling, data source factory dispatch, commit/version helpers).
- Fix CLI parsing/evaluation to correctly validate and use
--waveform-constants-file, and refreshtools-coverage.xml.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tools-coverage.xml | Adds/updates Cobertura XML coverage output for src/navigate/tools. |
| src/navigate/tools/main_functions.py | Fixes waveform_constants_file handling and adjusts some assertion formatting/help text. |
| test/tools/test_tk_thread_guard_additional.py | Adds coverage for skipping non-callable/missing Tk methods during guard installation. |
| test/tools/test_slicing_additional.py | Adds coverage for key_len, ensure_iter, and ensure_slice edge cases. |
| test/tools/test_sdf_additional.py | Adds a test for volume_from_sdf pixel sizing and z-subsampling behavior. |
| test/tools/test_multipos_table_tools.py | Makes multiposition table tests headless via a lightweight table double; minor cleanups. |
| test/tools/test_main_functions_additional.py | Adds parser/evaluation tests for overrides/defaults and waveform constants validation. |
| test/tools/test_linear_algebra_additional.py | Adds tests for combined-axis rotation and shared shear branches. |
| test/tools/test_image.py | Makes arrow-image tests headless by mocking ttk.Style; minor type-check improvement. |
| test/tools/test_file_functions_more.py | Adds a test for get_ram_info using a psutil mock. |
| test/tools/test_decorators_additional.py | Adds tests for performance_monitor payload behavior and log_initialization logging. |
| test/tools/test_common_functions_additional.py | Adds tests for module/param loaders, byte decoding, and VariableWithLock. |
| test/test_main_additional.py | Adds tests for navigate.main controller vs configurator branch behavior and listener shutdown. |
| test/test_commit_additional.py | Adds tests for git hash retrieval behavior and version file reading. |
| test/model/test_threads.py | Adds tests for ThreadWithWarning queue behavior and default logger usage. |
| test/model/data_sources/test_factory.py | Adds tests for file-type to data-source dispatch and unknown-type error logging. |
| test/log_files/test_log_functions_additional.py | Adds tests for log filename matching, pruning, performance log loading, and log_setup. |
| test/log_files/test_filters.py | Adds tests for routing performance vs non-performance log records. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tools-coverage.xml
Outdated
| <!-- Generated by coverage.py: https://coverage.readthedocs.io/en/7.10.7 --> | ||
| <!-- Based on https://raw.githubusercontent.com/cobertura/web/master/htdocs/xml/coverage-04.dtd --> | ||
| <sources> | ||
| <source>/Users/Dean/Documents/GitHub/navigate/src/navigate/tools</source> |
There was a problem hiding this comment.
The Cobertura
| <source>/Users/Dean/Documents/GitHub/navigate/src/navigate/tools</source> | |
| <source>src/navigate/tools</source> |
| if args.experiment_file: | ||
| assert ( | ||
| args.experiment_file.exists() | ||
| ), "experiment_file file Path {} not valid".format(args.experiment_file) | ||
| assert args.experiment_file.exists(), ( | ||
| "experiment_file file Path {} not valid".format(args.experiment_file) | ||
| ) | ||
| experiment_path = args.experiment_file | ||
|
|
||
| if args.waveform_constants_file: | ||
| assert ( | ||
| args.waveform_constants_path.exists() | ||
| ), "waveform_constants_path Path {} not valid".format( | ||
| args.waveform_constants_path | ||
| assert args.waveform_constants_file.exists(), ( | ||
| "waveform_constants_file Path {} not valid".format( | ||
| args.waveform_constants_file | ||
| ) | ||
| ) | ||
| waveform_constants_path = args.waveform_constants_path | ||
| waveform_constants_path = args.waveform_constants_file |
There was a problem hiding this comment.
Input validation here relies on assert statements. Assertions are stripped when Python runs with optimizations (-O), which would disable these checks in some deployments. Prefer raising a real exception (e.g., FileNotFoundError/ValueError) or calling parser.error(...) during argument parsing so invalid paths are always rejected.
| args.experiment_file.exists() | ||
| ), "experiment_file file Path {} not valid".format(args.experiment_file) | ||
| assert args.experiment_file.exists(), ( | ||
| "experiment_file file Path {} not valid".format(args.experiment_file) |
There was a problem hiding this comment.
The assertion message has a duplicated word: "experiment_file file Path ...". This reads like a typo and makes CLI failures less clear; consider changing it to something like "experiment_file Path ... not valid" for consistency with the other messages.
| "experiment_file file Path {} not valid".format(args.experiment_file) | |
| "experiment_file Path {} not valid".format(args.experiment_file) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1208 +/- ##
===========================================
+ Coverage 63.65% 63.84% +0.18%
===========================================
Files 189 189
Lines 26029 26029
===========================================
+ Hits 16569 16618 +49
+ Misses 9460 9411 -49
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
waveform_constants_fileargument handling insrc/navigate/tools/main_functions.pytools-coverage.xmlTesting
uv run pytest test/tools --override-ini "addopts=--strict-markers -m 'not hardware'" --cov=src/navigate/tools --cov-report=term-missing:skip-covered --cov-report=xml:tools-coverage.xmluv run ruff check test/tools/test_common_functions_additional.py test/tools/test_decorators_additional.py test/tools/test_file_functions_more.py test/tools/test_image.py test/tools/test_linear_algebra_additional.py test/tools/test_multipos_table_tools.py test/tools/test_sdf_additional.py test/tools/test_slicing_additional.py test/tools/test_tk_thread_guard_additional.pyuv run ruff format --check test/tools/test_common_functions_additional.py test/tools/test_decorators_additional.py test/tools/test_file_functions_more.py test/tools/test_image.py test/tools/test_linear_algebra_additional.py test/tools/test_multipos_table_tools.py test/tools/test_sdf_additional.py test/tools/test_slicing_additional.py test/tools/test_tk_thread_guard_additional.py