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.

Sign up for GitHub

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

Jump to bottom

Improve test coverage of sync.py #1936

Merged
merged 13 commits into from
Oct 12, 2022
Merged

Improve test coverage of sync.py #1936

merged 13 commits into from
Oct 12, 2022

Conversation

Aratz
Copy link
Contributor

@Aratz Aratz commented Oct 11, 2022

This PR improves the code coverage of the sync module and fixes a couple of bugs along the way.

Only the sync function remains untested but this one is kind of a beast so I leave it for now.

Closes #1913

PR checklist

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md is updated
  • If you've fixed a bug or added code that should be tested, add tests!

@Aratz
Test reset_target_dir
1d2ee9b
@Aratz
Test checkout_template_branch raises exception
9855cf2
@Aratz
Test create_merge_base branch
ceb30d9 
There was a bug in this function: because `self.merge_branch` was used
to store the new branch name, it was stored persistently in the `psync`
object. Hence when a new branch was created, the branch number was
appended several times, e.g. `original_merge_branch-2-3-4`.
@Aratz
Test push_merge_branch
373f596
@Aratz
Test close_open_template_merge_prs
a8c99b0
@Aratz
Test close_open_pr
005fecf
@Aratz
Update CHANGELOG.md
bb4b621
@Aratz
Convert exception checks to pytest.raises
7c32258
@Aratz Aratz self-assigned this Oct 11, 2022
@codecov
Copy link

codecov bot commented Oct 11, 2022 •
edited
Loading

Codecov Report

Merging #1936 (0fd13a0) into dev (c84dfb7) will increase coverage by 1.33%.
The diff coverage is 100.00%.

❗ Current head 0fd13a0 differs from pull request most recent head fb8fd60. Consider uploading reports for the commit fb8fd60 to get more accurate results

@@            Coverage Diff             @@
##              dev    #1936      +/-   ##
==========================================
+ Coverage   59.93%   61.26%   +1.33%     
==========================================
  Files          37       37              
  Lines        4677     4642      -35     
==========================================
+ Hits         2803     2844      +41     
+ Misses       1874     1798      -76
Impacted Files Coverage Δ
nf_core/sync.py 77.77% <100.00%> (+26.30%) ⬆️
nf_core/lint/template_strings.py 87.50% <0.00%> (-8.34%) ⬇️
nf_core/modules/modules_repo.py 73.27% <0.00%> (-0.69%) ⬇️
nf_core/__main__.py 53.30% <0.00%> (-0.10%) ⬇️
nf_core/subworkflows/install.py 14.28% <0.00%> (+0.10%) ⬆️
nf_core/modules/modules_command.py 74.48% <0.00%> (+6.72%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

mirpedrol
mirpedrol approved these changes Oct 12, 2022
View reviewed changes
Copy link
Member

@mirpedrol mirpedrol left a comment

Choose a reason for hiding this comment

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

LGTM :)

mashehu
mashehu approved these changes Oct 12, 2022
View reviewed changes
tests/test_sync.py Show resolved Hide resolved
@Aratz Aratz merged commit 3db6e4b into nf-core:dev Oct 12, 2022
@drernie drernie mentioned this pull request Jan 21, 2023
Closed
@mirpedrol mirpedrol mentioned this pull request Mar 9, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mirpedrol mirpedrol mirpedrol approved these changes

@mashehu mashehu mashehu approved these changes

Assignees

@Aratz Aratz

Labels
None yet
Projects
No open projects
nf-core Hackathon March 2023
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Aratz @mashehu @mirpedrol