Redesign Testing and Add BMI Unit Tests#151
Redesign Testing and Add BMI Unit Tests#151robertbartel wants to merge 29 commits intoNOAA-OWP:masterfrom
Conversation
robertbartel
force-pushed
the
i/organized_unit_tests/main
branch
from
8a7b56a to
58cccee
Compare
|
Regarding the current linker error, I never encountered this on MacOS with clang, but I was able to reproduce it on another Linux system. After making the last change in the CMake config for the linker, I no longer see the problem in my Linux system (😃), but it's still happening in the Action Runner (😕) . I'm not 100% sure at this point if something is cached, if there is some subtle difference in the Runner compared to my Linux system, or if I'm just missing something else. The idea a subtle difference seems less likely when we we are talking about involves a standard C math library, but I'm not sure. Any thoughts @hellkite500 or @aaraney? |
|
|
||
|
|
||
| if (NOT NGEN) | ||
| target_link_libraries(${exe_name} PRIVATE m) |
There was a problem hiding this comment.
@robertbartel this may be the problem in the runner tests -- try linking the math lib even if NGEN is set and see if that works?
There was a problem hiding this comment.
Indeed that fixed it, but I don't understand why. I get the need for the math lib, but not why what I was doing before wasn't sufficient (given that it did correct the issue in some cases).
But unless that's clear to someone already, we can save answer that question for another time.
There was a problem hiding this comment.
Different compiler versions and platforms may have libm implicitly linked for all kinds of different reasons... My best guess is just slightly different toolchain versions.
Creating helper test function (with naive implementation) for getting arbitrary values for BMI input variables to use for setting module inputs, with parameters for (in the future) allowing different values depending on the example test case and current time of the module.
Add func to set arbitrary values for BMI inputs ahead of a call to update() advancing the module.
Adding for remaining grid functions and for time functions.
- Add start time and time step size values as defined macros - Adjust function(s) for setting values prior to update to support direct control over what values are set
Adding tests for BMI functions that are not fully implemented and only have a shell implementation that returns BMI_FAILURE, to ensure this is what is returned (so that if implementations are ever added, these tests fail and let someone know proper tests need to be added).
Fixing test by having function properly account for case (which applies to the current test example config) when module does not fully use the "soil_moisture_profile" variable, and thus nbytes can/should be skipped for it.
Renaming previously existing test file to better reflect what it is, as it is more of a comprehensive test of BMI functionality than a file with true unit tests.
Update CMake/CTest config with executable and tests for running the previously existing (though since renamed) combined test file testing all BMI functionality within the single test routine.
Removing, since now those tests should be built in the normal ngen build.
Removing previous test script now that testing integrated with CMake and CTest.
robertbartel
force-pushed
the
i/organized_unit_tests/main
branch
from
f8f3f21 to
7d6d4d5
Compare
|
Pinging on this again, as I still think it's a worthy change. I'll also tag @christophertubbs and @donaldwj in case they want to look at it. We might want/need to expand and strengthen testing ahead of incorporating some of the NGWPC contributions, and I think this would lay a good foundation for that. |
|
Looks like I also need to add the |
2 participants
Redesign automated testing for the project to use CMake and CTest, rather than ad hoc test script. Also, add several, more focused unit tests for BMI functions. Also add some helper testing utils and documentation.
Additions
test/bmi_test_utils.*)test/general_test_utils.*)test/test_bmi_model.c)Removals
UNITTESToption and related parts of CMake configChanges
CMakeLists.txtfor easier readingCMakeLists.txtfor aforementioned combined test fileCMakeLists.txtfor BMI unit tests, based on same example config as previously existing testTesting
Todos
get_var_nbytes; see TODOs and notes on Rootzone-based AET andsoil_moisture_profileChecklist