Skip to content
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

Merged
merged 8 commits into from
Sep 13, 2024

Conversation

zacharyburnett
Copy link
Collaborator

@zacharyburnett zacharyburnett commented Sep 9, 2024

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 spacetelescope/romancal#1399 and spacetelescope/crds#1064) blocks https://grit.stsci.edu/dmd/roman-metrics/-/merge_requests/21

@zacharyburnett zacharyburnett self-assigned this Sep 9, 2024
Copy link

codecov bot commented Sep 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

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

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.
📢 Have feedback on the report? Share it here.

@zacharyburnett zacharyburnett changed the title [SCSB-174] replace metrics_logger with test_requirements.json file [SCSB-174] move DMS requirement correlations with tests from @metrics_logger to decorators to romanisim/tests/dms_requirement_tests.json Sep 12, 2024
@zacharyburnett zacharyburnett changed the title [SCSB-174] move DMS requirement correlations with tests from @metrics_logger to decorators to romanisim/tests/dms_requirement_tests.json [SCSB-174] move DMS requirement correlations with tests from @metrics_logger test decorators to romanisim/tests/dms_requirement_tests.json Sep 12, 2024
@zacharyburnett zacharyburnett changed the title [SCSB-174] move DMS requirement correlations with tests from @metrics_logger test decorators to romanisim/tests/dms_requirement_tests.json [SCSB-174] move DMS requirement test correlations from @metrics_logger decorators to romanisim/tests/dms_requirement_tests.json Sep 12, 2024
@zacharyburnett zacharyburnett changed the title [SCSB-174] move DMS requirement test correlations from @metrics_logger decorators to romanisim/tests/dms_requirement_tests.json [SCSB-174] move DMS requirement <-> test correlations from @metrics_logger decorators to romanisim/tests/dms_requirement_tests.json Sep 12, 2024
Copy link

@jglorioso-stsci jglorioso-stsci left a 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.

@zacharyburnett
Copy link
Collaborator Author

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

oh good catch! I should add that to .gitignore

@schlafly
Copy link
Collaborator

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
Copy link
Collaborator Author

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

@schlafly schlafly merged commit ff0fb51 into spacetelescope:main Sep 13, 2024
22 checks passed
@zacharyburnett zacharyburnett deleted the scsb174 branch September 13, 2024 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants