Conversation
|
I was seeing how it works
Sent from Outlook for Android<https://aka.ms/AAb9ysg>
…________________________________
From: Luigi Pertoldi ***@***.***>
Sent: Monday, February 2, 2026 8:49:50 PM
To: legend-exp/legend-simflow ***@***.***>
Cc: Toby Dixon ***@***.***>; Assign ***@***.***>
Subject: Re: [legend-exp/legend-simflow] Improve utility function coverage with type hints and comprehensive tests (PR #81)
⚠ Caution: External sender
@gipert commented on this pull request.
________________________________
On TEST_COVERAGE_IMPROVEMENTS.md<#81 (comment)>:
wtf is this
—
Reply to this email directly, view it on GitHub<#81 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ANSET424OZ24ATWUJOXYHAD4J6Z65AVCNFSM6AAAAACTXDERAGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTONBRGY2DSMJTGM>.
You are receiving this because you were assigned.Message ID: ***@***.***>
|
|
looks good except for this useless markdown file |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #81 +/- ##
==========================================
+ Coverage 61.98% 63.33% +1.35%
==========================================
Files 19 19
Lines 1331 1331
==========================================
+ Hits 825 843 +18
+ Misses 506 488 -18 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@copilot you should see that the config can be instead setups:
l200:
paths:
...In addition you should see that some other keys like paths:
par_hit: $_/generated/par/hit
par_pht: $_/generated/par/phtcan be present, add them and use it to test |
... Updated in e55c430. The fixture now uses the
|
@copilot please test both formats |
Added in 38d95d5. Created
Both config formats are now fully tested (16 tests total). |
…ions Co-authored-by: tdixon97 <56904179+tdixon97@users.noreply.github.com>
Co-authored-by: tdixon97 <56904179+tdixon97@users.noreply.github.com>
Co-authored-by: tdixon97 <56904179+tdixon97@users.noreply.github.com>
Co-authored-by: tdixon97 <56904179+tdixon97@users.noreply.github.com>
Co-authored-by: tdixon97 <56904179+tdixon97@users.noreply.github.com>
Co-authored-by: tdixon97 <56904179+tdixon97@users.noreply.github.com>
…d_pars_db test Co-authored-by: tdixon97 <56904179+tdixon97@users.noreply.github.com>
Co-authored-by: tdixon97 <56904179+tdixon97@users.noreply.github.com>
Co-authored-by: tdixon97 <56904179+tdixon97@users.noreply.github.com>
38d95d5 to
0096e4c
Compare
There was a problem hiding this comment.
Pull request overview
This pull request enhances utility function coverage by adding type hints, comprehensive docstrings, and extensive tests for several utility functions in the legendsimflow package. The changes improve code maintainability and increase test coverage from 52% to 90% for utils.py and from 71% to 72% for metadata.py.
Changes:
- Added type hints and comprehensive docstrings to
_merge_defaults,setup_logdir_link, andget_vtx_simconfigfunctions - Added 13 new tests covering dictionary merging, symlink creation, hash generation, tier name detection, parameter database initialization, and integer extraction
- Created two test fixtures (
tier_test_dataandtier_test_data_direct) to support testing both config formats (with and withoutsetups.l200wrapper)
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| workflow/src/legendsimflow/utils.py | Added type hints and docstring to _merge_defaults; added type hints and expanded docstring for setup_logdir_link |
| workflow/src/legendsimflow/metadata.py | Added type hints to get_vtx_simconfig function |
| tests/test_utils.py | Added 13 new test functions and 2 fixtures covering utility functions for both config formats |
| tests/test_metadata.py | Added test for extract_integer function with multiple test cases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary: Improve Coverage of Utility Functions ✅
Type Hints and Docstrings ✅
utils.pyandmetadata.py_merge_defaultssetup_logdir_link(parameters and return type)_merge_defaultsget_vtx_simconfiginmetadata.pyTest Coverage Improvements ✅
utils.py coverage: 52% → 90% (+38%)
_merge_defaultsfunction (dict merging logic)setup_logdir_linkfunction (symlink creation)hash_dictfunction (JSON serialization)get_hit_tier_namewith proper fixtureslookup_dataflow_configwith fixtures for both config formatsinit_generated_pars_dbfunction with par directories for both config formatsextract_integerin metadata moduleImproved Tests with Fixtures ✅
tier_test_datafixture withsetups.l200structure matching production configtier_test_data_directfixture with directpathsstructure for legacy supportdbetto.utils.write_dictfor cleaner config file creationpar,par_hit,par_pht) to both fixturesget_hit_tier_namefor both config formatslookup_dataflow_configfor both config formats with proper variable substitutioninit_generated_pars_dbfor both config formats (full par database and tier-specific databases)Code Quality Validation ✅
setups.l200.pathsand directpathsKey Improvements
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.