Added test using a DDT host object to pass information
Added test using a DDT host object to pass information Fix problems so that test passes Improve formatting for readability
User interface changes?: No
Fixes: #589
Testing: test removed: None unit tests: Pass system tests: Pass, added DDT host object test manual testing: Ran doctests, examined generated code for system tests
A couple of thoughts:
- Instead of a new Fortran test, I could move the ccpp object into a different test (e.g., the advection_test).
- Another test mod I thought of but did not try is to move the error variables (and/or the loop variables) out of the host-model call strings and into module data. Is anyone interested in this feature?
Do we really need such a complex test for just making sure that errmsg and errflg can be part of a DDT?
That's what I thought which is why I made this comment.
@gold2718 As discussed on Monday, we should proceed with this PR, including adding the new test, once the comment on what base_only means (minimal documentation in the code?) is addressed and the other typo is fixed. Thanks!
Thanks for getting this in @gold2718! Just had a few recommendations for style and consistency (and 1 or 2 functional changes) but just let me know if any of those are problematic, otherwise nothing major.
I did notice the test logs in the runner are printing out the error message:
-- Running: /home/runner/work/ccpp-framework/ccpp-framework/scripts/ccpp_capgen.py --host-files test_host_data.meta,test_host_mod.meta,test_host.meta --scheme-files cld_suite_files.txt --suites cld_suite.xml --host-name test_host --output-root /home/runner/work/ccpp-framework/ccpp-framework/test/at_build/ccpp --debug -- xmllint not found, could not validate file /home/runner/work/ccpp-framework/ccpp-framework/test/advection_test/cld_suite.xmlIf these tests should be failing if xmllint isn't on the path, I'm torn as to how to handle that as this looks like a pre-existing issue but it should be as simple as adding
libxml2-utilsto the capgen yaml workflow file on theapt-get install ...line but I would have to do some more testing on that front.If this is outside the scope of this PR, we can make an issue to address that in a different PR.
Regarding the xmllint dependency error, let's hold off on any changes with regard to it here and we will address it if need be in #601.
@dustinswales Can you take a look at this to make sure it's fine by you?
Any more takes for this PR? It's got two approvals so far, from NRL and CGD. Can we get one from NOAA, please?
@dustinswales or @grantfirl Please review when it's convenient for you so that we have the green light from NOAA to merge. Thanks!