-
Notifications
You must be signed in to change notification settings - Fork 64
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
feature/capgen: add new tests for unit conversions #434
feature/capgen: add new tests for unit conversions #434
Conversation
Please do not overload capgen_test, it is already complicated enough. |
The other option I can offer is to rsync This way we have the same framework for these end-to-end style tests, but with fewer code in each of them. Some code duplication, but maybe still better than overloading |
Yes please! That is the approach I took with the advection_test. |
…ited to unit conversions
712df77
to
ed34891
Compare
Done. This looks much simpler now, and the tests are failing when comparing the return values to the expected values, so that's good because we expect this. |
…to metavar.py and var_props.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for creating this, I have a couple of questions and would like some minor cleanup.
This review was of 2e2d5da.
I made the changes as requested, including reducing the tolerance to
All other tests pass, therefore this is expected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great now, thanks!!
Description
Add new tests for unit conversions as (1) unit tests and (2) system tests in a separate
var_action_test
folder. These are testing variables inside a DDT and outside, and with different intents (in, out, inout).The unit tests all pass, because the unit conversion at variable level (class function
compatible
) is already implemented. The system tests fail, because capgen does not yet generate the unit conversion code at the suite level.What's not included?
I was looking for possibilities to add unit tests at the
suite_objects.py
level, in particular around theanalyze
function of thescheme
class. Implementing tests there in form of doc tests would be complicated and not so much different from the system tests in the newvar_action_test
folder.It is also difficult to write unit tests alongside the new system tests, because I don't know how the exact code block should look like in capgen-generated code. Certainly different from the prebuild-generated code!
Additional details
User interface changes?: No
Fixes #430.
Testing:
test removed:
unit tests: unit tests (doc tests) for testing variable compatibility/unit conversions to
metavar.py
andvar_props.py
(these tests pass)system tests: new test directory
tests/var_action_test
(these tests fail)manual testing: