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

Remove fixed Ubuntu 20.04 testing workflow #2397

Merged
merged 3 commits into from
Aug 15, 2023

Conversation

adamrtalbot
Copy link
Contributor

Removes the pytest fixed version on Ubuntu:20.04 and Python 3.8 and adds it to matrix variables.

Original merge that added it: #2043

Using fewer workflows it better for enforcing code coverage etc. and managing testing throughput.

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!
  • Documentation in docs is updated

@github-actions
Copy link
Contributor

This PR is against the master branch ❌

  • Do not close this PR
  • Click Edit and change the base to dev
  • This CI test will remain failed until you push a new commit

Hi @adamrtalbot,

It looks like this pull-request is has been made against the adamrtalbot/tools master branch.
The master branch on nf-core repositories should always contain code from the latest release.
Because of this, PRs to master are only allowed if they come from the adamrtalbot/tools dev branch.

You do not need to close this PR, you can change the target branch to dev by clicking the "Edit" button at the top of this page.
Note that even after this, the test will continue to show as failing until you push a new commit.

Thanks again for your contribution!

@adamrtalbot adamrtalbot changed the title Use one testing workflow Remove fixed Ubuntu 20.04 testing workflow Aug 14, 2023
@adamrtalbot adamrtalbot changed the base branch from master to dev August 14, 2023 15:15
@codecov
Copy link

codecov bot commented Aug 14, 2023

Codecov Report

Merging #2397 (38f986f) into dev (3ec2697) will increase coverage by 0.05%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##              dev    #2397      +/-   ##
==========================================
+ Coverage   71.09%   71.14%   +0.05%     
==========================================
  Files          87       87              
  Lines        9409     9409              
==========================================
+ Hits         6689     6694       +5     
+ Misses       2720     2715       -5     

see 1 file with indirect coverage changes

Copy link
Contributor

@edmundmiller edmundmiller left a comment

Choose a reason for hiding this comment

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

Looks good to me, I didn't know you could do that!

Comment on lines +17 to +18
env:
GITHUB_TOKEN: ${{ github.token }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
env:
GITHUB_TOKEN: ${{ github.token }}

What's the purpose of adding this? Just wondering for our future selves! 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was hitting API limits, I hoped it might authenticate by default and it seemed to help! Looking at the code I think it's just coincidence but maybe it worked.

I bet if I remove it it will break.

@@ -35,6 +42,14 @@ jobs:
python -m pip install --upgrade pip -r requirements-dev.txt
pip install -e .

- name: Downgrade git to the Ubuntu official repository's version
if: ${{ matrix.runner == 'ubuntu-20.04' && matrix.python-version == '3.8' }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if: ${{ matrix.runner == 'ubuntu-20.04' && matrix.python-version == '3.8' }}
if: ${{ matrix.runner == 'ubuntu-20.04' && matrix.python-version == '3.8' }}
env:
GITHUB_TOKEN: ${{ github.token }}

Is this the scope it's needed in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, it reflects the original test which used a couple of older versions of software together to mimic a scenario. If you look at the tests it seems to be working correctly.

@adamrtalbot adamrtalbot merged commit 9d63f93 into nf-core:dev Aug 15, 2023
20 checks passed
@adamrtalbot adamrtalbot deleted the use_one_testing_workflow branch August 15, 2023 10:13
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.

None yet

2 participants