-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SCSB-174] move DMS requirement <-> test correlations from @metrics_logger
decorators to romanisim/tests/dms_requirement_tests.json
#146
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #146 +/- ##
==========================================
+ Coverage 89.24% 91.07% +1.83%
==========================================
Files 17 16 -1
Lines 2073 2118 +45
==========================================
+ Hits 1850 1929 +79
+ Misses 223 189 -34 ☔ View full report in Codecov by Sentry. |
aaad8b8
to
1036bdf
Compare
1036bdf
to
812d2b7
Compare
abfb1fc
to
4a40160
Compare
metrics_logger
with test_requirements.json
file@metrics_logger
to decorators to romanisim/tests/dms_requirement_tests.json
@metrics_logger
to decorators to romanisim/tests/dms_requirement_tests.json
@metrics_logger
test decorators to romanisim/tests/dms_requirement_tests.json
@metrics_logger
test decorators to romanisim/tests/dms_requirement_tests.json
@metrics_logger
decorators to romanisim/tests/dms_requirement_tests.json
@metrics_logger
decorators to romanisim/tests/dms_requirement_tests.json
@metrics_logger
decorators to romanisim/tests/dms_requirement_tests.json
2b4f57b
to
4e34438
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious what the addition of romanisim/ramp_fit_casertano.c is?
Otherwise, looks good.
oh good catch! I should add that to |
Thanks, looks good to me. One question: the new test_dms_requirements test aims to be a consistency check that the tests I've indicated should exist in the requirements.json file actually exist---correct? If I deleted that file we might have issues in the future if I made a typo in the json file but it's not technically needed for the metrics code or for romanisim to work? |
Correct, it's just a check to flag name changes or restructuring of tests to make sure the JSON file stays up-to-date |
Resolves SCSB-174
consolidates all test requirement correlations into
romanisim/tests/dms_requirement_tests.json
, obviating the need to insert log messages at test execution time withmetrics_logger
This PR (and its sister PRs spacetelescope/romancal#1399 and spacetelescope/crds#1064) blocks https://grit.stsci.edu/dmd/roman-metrics/-/merge_requests/21