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

Split utilities test file into two to match the source files #127

Merged
merged 1 commit into from
Mar 8, 2024

Conversation

willingc
Copy link
Collaborator

@willingc willingc commented Mar 8, 2024

This small PR breaks up the existing test file for utilities into two test files following the refactor in #121 and #120. No tests were changed.

Copy link

codecov bot commented Mar 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 47.56%. Comparing base (1d3396c) to head (89b26eb).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #127   +/-   ##
=======================================
  Coverage   47.56%   47.56%           
=======================================
  Files           7        7           
  Lines         473      473           
  Branches       74       74           
=======================================
  Hits          225      225           
  Misses        244      244           
  Partials        4        4           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@lwasser lwasser left a comment

Choose a reason for hiding this comment

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

@willingc this is great. i debated about this last night as well. it feels weird to have a single function in a module and then a single test in a test module. are we leaning in to readibility over how much content is in a module here? we can merge this as well.

@lwasser lwasser merged commit 7b9ef86 into pyOpenSci:main Mar 8, 2024
4 checks passed
@willingc
Copy link
Collaborator Author

willingc commented Mar 9, 2024

@lwasser It's fine to have 1 function in a module or 1 test in a file. I tend to lean toward separation of concepts. If we had one file called utils.py, then I would create test_utils.py (which would include all utilities). Since we split the utils into two files which makes good sense utils_clean.py and utils_parse.py, I like to match the test files to the source file. It requires less thought on my part since I know my tests are almost always in a file test_sourcefilename.py when sourcefilename.py is the code file.

@willingc willingc deleted the split_tests branch March 9, 2024 01:54
@willingc
Copy link
Collaborator Author

willingc commented Mar 9, 2024

Keep the great questions coming too.

@lwasser
Copy link
Member

lwasser commented Mar 9, 2024

Thank you @willingc !! That makes sense!!

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

Successfully merging this pull request may close these issues.

2 participants