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

Unit testing #38

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft

Unit testing #38

wants to merge 8 commits into from

Conversation

atteggiani
Copy link
Collaborator

Unit-testing functions written (with 100% coverage) for the hres_ic.py before the latest merge, so it will need to be updated.
Thehres_ic.py script was refactored to allow proper testing.

The latest merge introduced changes to hres_ic.py that influence the refactoring and overall logic, that need to be properly addressed for testing (#36).

Opening this PR so it might get used once unit tests are updated.

@atteggiani atteggiani linked an issue Aug 15, 2024 that may be closed by this pull request
@atteggiani atteggiani requested a review from tmcadam November 4, 2024 23:48
Copy link

@tmcadam tmcadam left a comment

Choose a reason for hiding this comment

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

These are nice unit tests and the module is well broken down into functions. I like the use of mock and patch, it's limiting the tests to your own code, a nice way to test different input combinations.

@atteggiani
Copy link
Collaborator Author

These are nice unit tests and the module is well broken down into functions. I like the use of mock and patch, it's limiting the tests to your own code, a nice way to test different input combinations.

Thank you for the initial review @tmcadam!

Great, then I'll continue with this and incorporate the latest updates.
I will mark this PR as "ready for review" when I have all the scripts tested and will re-ping you for a (complete) review.

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: New Issues
Development

Successfully merging this pull request may close these issues.

Testing CI
2 participants