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

The testmod includes directories concatenation for shell_commands files only add the shell_commands files after the first two include directories #4709

Open
ekluzek opened this issue Nov 26, 2024 · 3 comments
Labels
Responsibility: CESM Responsibility to manage and accomplish this issue is through CESM tp: CIMElib

Comments

@ekluzek
Copy link
Contributor

ekluzek commented Nov 26, 2024

When a testmod is setup with several include directories, the shell_commands files don't all get added to the resulting final final in the case created for the final test. It only adds the shell_commands files for the testmod includes after the first two directories.

The CTSM issue regarding this is here...

ESCOMP/CTSM#2037

I found this behavior in this CTSM PR:

ESCOMP/CTSM#2888

This is in cime6.1.37, but we have seen this for long before that.

@ekluzek ekluzek added Responsibility: CESM Responsibility to manage and accomplish this issue is through CESM tp: CIMElib labels Nov 26, 2024
@ekluzek
Copy link
Contributor Author

ekluzek commented Nov 26, 2024

It looks to me that this is in the apply_user_mods subroutine in:

CIME/user_mod_support.py

It could also be in the calls to it in case.py and case_clone.py.

If we do find issues here, we should add more unit testing for this in:
CIME/tests/test_unit_case.py
CIME/tests/test_unit_user_mod_support.py

@ekluzek
Copy link
Contributor Author

ekluzek commented Nov 26, 2024

I think it still updates the case with the behavior from the first two, but it doesn't add them to the final shell_commands file for the final test case.

@billsacks
Copy link
Member

@ekluzek - based on my understanding from reading back through ESCOMP/CTSM#2037 and from re-examining the code in user_mod_support.py, I think the current expected behavior is:

  • If a test only directly includes a single testmod, then all shell commands from that testmod and all included testmods (recursively) will appear in the shell_commands file
  • If a test directly includes multiple testmods (with the double hyphen syntax), then each of those is processed separately. So the first will be applied, along with all of its includes (recursively), and a shell_commands file will be created and executed for all of these. Then the second will be applied; the first thing that happens there is that the existing shell_commands file is removed to avoid reapplying any commands from the first set; then a new shell_commands file is created that contains all of the shell commands from the second testmod (and all of its includes).

Does this description match what you're seeing?

If you want a file containing a record of all shell_commands that have been executed, my suggestion is this: ESCOMP/CTSM#2037 (comment) - i.e., create a new file that contains a record of everything that has been executed in shell_commands. I think this will be easy and safe to do: by creating a new file that is just for documentation purposes, we won't run any risk of changing logic or breaking some weird cases. It's possible that we'd end up with extra copies of commands in some circumstances (I'm not sure what circumstances, but there may be some), but if so I think that would be because the given shell commands have actually been executed multiple times in those circumstances, so having them appear multiple times in the documentation file actually feels correct.

Alternatives that feel less appealing:

  • Change the logic in apply_user_mods to not remove existing shell_commands files. But I can envision (theoretical) cases where this will be problematic, because you'll end up rerunning commands from the first set of testmods.
  • Along with this change in apply_user_mods, do a more significant rework of the logic so that the application of of shell_commands is no longer done in apply_user_mods (which handles a single directly-specified testmod and all of its includes), but instead happens after all testmods have been processed. However, this will require changes that extend outside of user_mod_support.py to places where it is called (of which there are multiple). This would be both a more extensive and riskier set of changes, and I don't personally see much value in this beyond what could be provided with the documentation-oriented suggestion in shell_commands for tests with two testmods listed don't concatenate both together.. ESCOMP/CTSM#2037 (comment).

I can see an argument that the suggestion in ESCOMP/CTSM#2037 (comment) could lead to a confusing situation of having two similar-but-different files. So actually one improvement on this would be: After copying the contents of shell_commands into shell_commands_all_testmods, remove shell_commands. So in the end, all that would be left would be shell_commands_all_testmods. (With a bit more rework, we could even keep this called shell_commands. That would require changing the "working" file to be called something different, like shell_commands_temp.)

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 tp: CIMElib
Projects
None yet
Development

No branches or pull requests

2 participants