search

spacetelescope / romanisim

Nancy Grace Roman Space Telescope WFI Data Simulator
https://romanisim.readthedocs.io
Other
15 stars 13 forks source link

[SCSB-174] move DMS requirement <-> test correlations from `@metrics_logger` decorators to `romanisim/tests/dms_requirement_tests.json` #146

Closed zacharyburnett closed 1 month ago

zacharyburnett commented 1 month ago

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 with metrics_logger

This PR (and its sister PRs https://github.com/spacetelescope/romancal/pull/1399 and https://github.com/spacetelescope/crds/pull/1064) blocks https://grit.stsci.edu/dmd/roman-metrics/-/merge_requests/21

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 91.07%. Comparing base (81028a8) to head (eb30bcc). Report is 7 commits behind head on main.

Additional details and impacted files ```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

zacharyburnett commented 1 month ago

Curious what the addition of romanisim/ramp_fit_casertano.c is?

oh good catch! I should add that to .gitignore

schlafly commented 1 month ago

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?

zacharyburnett commented 1 month ago

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