Skip to content
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

Add back append_testlog to CIME/utils.py #4703

Open
ekluzek opened this issue Oct 31, 2024 · 3 comments
Open

Add back append_testlog to CIME/utils.py #4703

ekluzek opened this issue Oct 31, 2024 · 3 comments
Assignees
Labels
Responsibility: CESM Responsibility to manage and accomplish this issue is through CESM Responsibility: CTSM Responsibility to manage and accomplish this issue is the CTSM Software group st: wontfix tp: CIMElib tp: system tests ty: enhancement

Comments

@ekluzek
Copy link
Contributor

ekluzek commented Oct 31, 2024

The function append_testlog was removed from CIME/utils.py in 1fa26aa. I didn't see a reason for it in the log, but assume it likely was just because it might not have been referenced elsewhere in cime? However, CTSM relies on this function for the LILACSMOKE and SSPMATRIXCN SystemTest tests. In general this is something useful to have in order to have tests add extra output to the TestLog. So keeping it around allows SystemTests to easily add to the TestLog output which in principle is an important thing to allow.

@ekluzek ekluzek added ty: enhancement Responsibility: CESM Responsibility to manage and accomplish this issue is through CESM tp: system tests tp: CIMElib Responsibility: CTSM Responsibility to manage and accomplish this issue is the CTSM Software group labels Oct 31, 2024
@ekluzek ekluzek self-assigned this Oct 31, 2024
@ekluzek
Copy link
Contributor Author

ekluzek commented Oct 31, 2024

@jedwards4b and @billsacks if this sounds acceptable, I'll do a PR for this shortly. It doesn't hurt anything, just adds some code currently only used for example in the CTSM LILAC tests.

To prevent this kind of thing in the future, I'll add a comment about this in the code for where it's needed. It would also be good to have a unit test for it in the cime testing. The latter is a little more involved -- but is the best thing for preventing this type of removal.

@jedwards4b
Copy link
Contributor

No - it was moved from utils.py to status.py as noted in the PR. The correct action is to update from
import CIME.utils to import CIME.status

@ekluzek
Copy link
Contributor Author

ekluzek commented Oct 31, 2024

Awesome, thanks @jedwards4b I missed that comment. But, see it now

#4660 (comment)

Thanks for letting me know. I'll close this after I test it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Responsibility: CESM Responsibility to manage and accomplish this issue is through CESM Responsibility: CTSM Responsibility to manage and accomplish this issue is the CTSM Software group st: wontfix tp: CIMElib tp: system tests ty: enhancement
Projects
None yet
Development

No branches or pull requests

2 participants