-
Notifications
You must be signed in to change notification settings - Fork 322
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
ctsm5.3.028: Move Izumi intel and gnu tests to Derecho #2975
base: master
Are you sure you want to change the base?
Conversation
Those two Python scripts could actually be used in a GitHub workflow to check that |
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.
I verified that the new testlist only has izumi_nag. And the changes all make sense to me. Some things I hadn't thought about you also brought in which makes sense. This will make the test list on Derecho, slightly longer, but that's fine.
I also think that the tools you made should become more mainstream. The tmp directory makes me think you were going to delete them. But, I don't think you should. We should do more of using tools like this. If they do become more mainstream, I'd like to see more testing for them, and probably a library level that removes some code duplication.
But, I think you bring this in like it is.
tmp/remove_duplicate_tests.py
Outdated
@@ -0,0 +1,104 @@ | |||
# %% | |||
import xml.etree.ElementTree as ET | |||
from xml.dom import minidom |
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.
Putting this in a tmp directory makes it sound like this is a one time thing to do. But, when thinking about it -- we probably ought to keep this around, and run it periodically. Maybe even have a github action for it.
Having some tools that check the testlists and make sure they are valid is really good thing to have. Having more tools around this would be really good. This is moving us towards automation and tools doing work for us, which is a really powerful thing to move towards.
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 now I've moved them into a new directory, tools/contrib/testlist_checks/
.
- File an issue to convert these to GitHub Workflows.
Description of changes
Moves Izumi intel and gnu tests to Derecho.
Notes to reviewer(s)
tmp/
directory. This directory will be deleted before merging, but I wanted to show my work.tmp/remove_testlist_whitespace.py
, rewrotetestlist_clm.xml
with extraneous whitespace removed. This would an inevitable result of using Python to parse, edit, and rewritetestlist_clm.xml
, so I did it in one separate step that got its own commit. However, the result is that it looks like there are many more diffs intestlist_clm.xml
than there really are. Make sure to ignore whitespace in your review to simplify things.tmp/remove_duplicate_tests.py
, removed duplicate tests resulting from the Izumi-to-Derecho change.Specific notes
Contributors other than yourself, if any: None
CTSM Issues Fixed:
Are answers expected to change (and if so in what way)? No. However, a bunch of tests will be moved from Izumi to Derecho.
Any User Interface Changes (namelist or namelist defaults changes)? No
Does this create a need to change or add documentation? Did you do so? No
Testing performed, if any: None so far