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

Refactor python build test to improve coverage #612

Merged
merged 1 commit into from
Feb 3, 2024

Conversation

cottsay
Copy link
Member

@cottsay cottsay commented Feb 2, 2024

Switching to a parametrized approach also significantly reduces duplicated code in the test.

@cottsay cottsay self-assigned this Feb 2, 2024
@cottsay cottsay force-pushed the cottsay/python-build-test-coverage branch from 90701a9 to 66f51ed Compare February 2, 2024 18:01
Copy link

codecov bot commented Feb 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6529b6b) 83.24% compared to head (c5a3d1a) 83.66%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #612      +/-   ##
==========================================
+ Coverage   83.24%   83.66%   +0.42%     
==========================================
  Files          65       65              
  Lines        3765     3765              
  Branches      728      728              
==========================================
+ Hits         3134     3150      +16     
+ Misses        555      541      -14     
+ Partials       76       74       -2     

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

@cottsay cottsay force-pushed the cottsay/python-build-test-coverage branch 2 times, most recently from 3f5d2ce to 3664418 Compare February 2, 2024 19:37
@@ -94,108 +147,44 @@ def _test_build_package(tmp_path_str, *, symlink_install):
build_base = Path(python_build_task.context.args.build_base)
assert build_base.rglob('my_module/__init__.py')

return Path(python_build_task.context.args.install_base)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we no longer return anything from this function I went through the diff to make sure we've dropped all calls that save a return value and it appears we do.

@cottsay
Copy link
Member Author

cottsay commented Feb 2, 2024

Thanks for the review. Even though the code coverage didn't improve very much with this change, I believe it covers some critical use cases that were previously gaps in our testing.

@cottsay cottsay force-pushed the cottsay/python-build-test-coverage branch from 6888abc to f7a6d05 Compare February 2, 2024 22:50
Switching to a parametrized approach also significantly reduces
duplicated code in the test.
@cottsay cottsay force-pushed the cottsay/python-build-test-coverage branch from f7a6d05 to c5a3d1a Compare February 2, 2024 22:58
@cottsay cottsay merged commit ad0feb4 into master Feb 3, 2024
35 checks passed
@cottsay cottsay deleted the cottsay/python-build-test-coverage branch February 3, 2024 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants