Skip to content

Conversation

@jianshu93
Copy link
Collaborator

add more check to each slurm script.

@jianshu93 jianshu93 requested a review from antgonza October 10, 2025 01:43
@jianshu93
Copy link
Collaborator Author

Note that I use flexible header tests (sbatch only) for step 2 to step 7. Otherwise for each other step. e.g., add a new line will lead to the failure of the test. Let me know your thoughts.

Thanks,

Jianshu

with open(step1_path, "r") as f:
obs_lines = [ln.rstrip("\n") for ln in f.readlines()]

self.assertCountEqual(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm really confused about this PR; why not continue using this kind of test vs. adding all the rest of methods to compare string? Is it to minimize changes in the future or something else? FWIW, the idea of testing this is to make sure that whatever change is made in the main scripts/code, we have to change the code in the test too - as a sanity check. Additionally, for most tests you are only testing the header which, if I understand the concept, it will not test the full script, no?

@jianshu93
Copy link
Collaborator Author

@antgonza, please review and then merge it. Everything looks good now.

@antgonza antgonza merged commit 986c534 into qiita-spots:main Oct 14, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants