NOAA-FIMS / FIMS

The repository for development of FIMS
https://noaa-fims.github.io/FIMS/
GNU General Public License v3.0
14 stars 9 forks source link

[Test]: Add tests to check invalid models exit correctly from CreateModel() #506

Open nathanvaughan-NOAA opened 11 months ago

nathanvaughan-NOAA commented 11 months ago

Issue details

exit(1) calls were added to the CreateModel() function to abort model creation if an invalid model is identified. We should add tests to confirm that these exit calls work correctly and provide the expected error codes. These tests were suggested in the review of pull request #499

msupernaw commented 11 months ago

@Nathan Vaughan - NOAA Affiliate @.***> The exit calls should be replaced with valid_model = false; . The method is expected to return true if the model is valid, false otherwise. This allows the CreateModel method to fully populate the log file with all relevant error messages, rather than just exiting on the first error.

On Thu, Nov 2, 2023 at 10:30 AM Nathan Vaughan @.***> wrote:

Issue details

exit(1) calls were added to the CreateModel() function to abort model creation if an invalid model is identified. We should add tests to confirm that these exit calls work correctly and provide the expected error codes. These tests were suggested in the review of pull request #499 https://github.com/NOAA-FIMS/FIMS/pull/499

— Reply to this email directly, view it on GitHub https://github.com/NOAA-FIMS/FIMS/issues/506, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABFUSECZ3L6VI35GUJXE2RTYCOU7VAVCNFSM6AAAAAA627RSIWVHI2DSMVQWIX3LMV43ASLTON2WKOZRHE3TIMZYGY2TEOA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- Matthew Supernaw Scientific Software Developer National Oceanic and Atmospheric Administration Office Of Science and Technology NOAA Fisheries | U.S. Department of Commerce Phone 248 - 396 - 7797

nathanvaughan-NOAA commented 11 months ago

Thanks @msupernaw the exit calls were suggested in the previous logging pull request review by @Cole-Monnahan-NOAA specifically to avoid having the full model run which resulted in the error logs getting buried in the rest of the outputs. I'm happy either way but seems we don't have a consensus on which is the best approach. I did create a separate error log file in the current pull request so maybe that is sufficient to allow a full model run while also capturing any error reports in a targeted fashion so everyone is happy? Happy to adjust the code to whichever approach is decided to be optimal.