nmfs-ost / ss3-source-code

The source code for Stock Synthesis (SS3).
https://nmfs-ost.github.io/ss3-website/
Creative Commons Zero v1.0 Universal
38 stars 16 forks source link

Warning action update #536

Closed e-perl-NOAA closed 11 months ago

e-perl-NOAA commented 11 months ago

Concisely describe what has been changed/addressed in the pull request.

Update build-ss3-warnings.yml to compare to a reference warnings file and fail if the number of warnings from a new build is greater than the number of warnings in that reference warnings file.

What tests have been done?

Where are the relevant files?

What tests/review still need to be done?

None.

Is there an input change for users to Stock Synthesis?

Additional information (optional).

iantaylor-NOAA commented 11 months ago

This is great, thank you @e-gugliotti-NOAA.

Is it worth making a plan for when to update the .github/workflows/reference_files/warnings_ss_ref.txt file? I can think of at least 2 scenarios:

Would it be worth supplementing the if (n_warn > length(ref)) to with a separate failing message when `if (n_warn < length(ref)) to help us see when the second scenario occurs? Maybe that's not needed since the change that makes the warning go away is probably some change that we're making to achieve that end and won't just happen by accident.

e-perl-NOAA commented 11 months ago

@iantaylor-NOAA I added lines similar to what you suggested. The R code written by Kathryn originally to get the n_warn only detects true warnings, so it isn't detecting the 2 lines that say "In file included from ss.cpp:7:" currently in the reference warnings text file, which is why the code has if(n_warn+length(ref) > length(ref)). Could you add your review?