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

Test ci #82

Merged
merged 20 commits into from
Sep 26, 2023
Merged

Test ci #82

merged 20 commits into from
Sep 26, 2023

Conversation

muffato
Copy link
Member

@muffato muffato commented Aug 1, 2023 •
edited
Loading

The purpose of this pull-request is to reinstate the CI tests on GitHub. The main trick is to simply add some steps to the CI workflow to download the necessary databases (NCBI Taxonomy and BUSCO).

At the same time, I recreated the reduced BUSCO databases with a script that is now under /lustre/scratch123/tol/resources/nextflow/busco. The new databases have 10 genes (or less) that are known hit and 5 genes that have no hit. Like in sanger-tol/genomenote#77 I change the metauk parameters to make BUSCO even faster.

Other changes:

  • All the test data are now under /lustre/scratch123/tol/resources/nextflow/test-data, which is fully synchronised with the S3 server.
  • I've changed the minimum version to 22.10, now tested on GitHub. No error so far !
  • I'm using the new, recommended, GitHub Action to launch Tower.
  • I've renamed our Tower integration scripts as per our new convention.
  • All "pipeline_info" outputs are now under pipeline_info/blobtoolkit, as per our new convention.
  • I had to add the docker.io/ prefix to a few containers as the default now is quay.io. This is actually the result of getting the CI to work ! We had not spotted the error because we only use Singularity on the farm.

PR checklist

  • This comment contains a description of changes (with reason).
  • If you've fixed a bug or added code that should be tested, add tests!
  • If you've added a new tool - have you followed the pipeline conventions in the contribution docs
  • Make sure your code lints (nf-core lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Usage Documentation in docs/usage.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

@muffato muffato changed the base branch from main to dev August 1, 2023 13:59
@github-actions
Copy link

github-actions bot commented Aug 1, 2023 •
edited
Loading

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit 54f5568

+| ✅ 129 tests passed       |+
#| ❔  18 tests were ignored |#
!| ❗   4 tests had warnings |!

❗ Test warnings:

  • files_exist - File not found: conf/igenomes.config
  • files_exist - File not found: .github/workflows/awstest.yml
  • files_exist - File not found: .github/workflows/awsfulltest.yml
  • nextflow_config - Config manifest.version should end in dev: '0.2.0'

❔ Tests ignored:

  • files_exist - File is ignored: assets/nf-core-blobtoolkit_logo_light.png
  • files_exist - File is ignored: docs/images/nf-core-blobtoolkit_logo_light.png
  • files_exist - File is ignored: docs/images/nf-core-blobtoolkit_logo_dark.png
  • files_exist - File is ignored: .github/ISSUE_TEMPLATE/config.yml
  • nextflow_config - Config variable ignored: manifest.name
  • nextflow_config - Config variable ignored: manifest.homePage
  • files_unchanged - File ignored due to lint config: LICENSE or LICENSE.md or LICENCE or LICENCE.md
  • files_unchanged - File ignored due to lint config: .github/ISSUE_TEMPLATE/bug_report.yml
  • files_unchanged - File does not exist: .github/ISSUE_TEMPLATE/config.yml
  • files_unchanged - File ignored due to lint config: .github/workflows/linting.yml
  • files_unchanged - File ignored due to lint config: assets/sendmail_template.txt
  • files_unchanged - File does not exist: assets/nf-core-blobtoolkit_logo_light.png
  • files_unchanged - File does not exist: docs/images/nf-core-blobtoolkit_logo_light.png
  • files_unchanged - File does not exist: docs/images/nf-core-blobtoolkit_logo_dark.png
  • files_unchanged - File ignored due to lint config: lib/NfcoreTemplate.groovy
  • files_unchanged - File ignored due to lint config: .gitignore or .prettierignore or pyproject.toml
  • actions_ci - actions_ci
  • actions_awstest - 'awstest.yml' workflow not found: /home/runner/work/blobtoolkit/blobtoolkit/.github/workflows/awstest.yml

✅ Tests passed:

Run details

  • nf-core/tools version 2.8
  • Run at 2023-09-18 19:55:47

@muffato muffato changed the base branch from dev to release August 15, 2023 07:20
@muffato muffato marked this pull request as ready for review August 15, 2023 07:46
alxndrdiaz
alxndrdiaz reviewed Sep 1, 2023
View reviewed changes
Copy link
Collaborator

@alxndrdiaz alxndrdiaz left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM. The only comment is that the blastn databases for running the small and full tests are about 70 MB in size, if these
files are too large to be part of the repo then the remote paths for them have to be updated accordingly in this branch and in the blastn branch. Otherwise it can be merged since both databases are already saved in assets folder in the blastn branch.

@muffato
Copy link
Member Author

muffato commented Sep 1, 2023

The only comment is that the blastn databases for running the small and full tests are about 70 MB in size, if these
files are too large to be part of the repo then the remote paths for them have to be updated accordingly in this branch and in the blastn branch.

That's absolutely fine. I actually think the .dmnd files should be moved from assets/ to remote paths as well. Even if they're small, it's not the sort of things that should really be kept in the code repository.

@priyanka-surana priyanka-surana changed the base branch from release to alignment September 18, 2023 07:10
@priyanka-surana
Copy link
Contributor

@muffato Any other changes to this PR. Otherwise I would like to merge this into #83 and run the tests.

@muffato
Copy link
Member Author

muffato commented Sep 18, 2023

@priyanka-surana I moved the .dmnd files to S3 and updated the test config. Ready to merge !

priyanka-surana
priyanka-surana approved these changes Sep 26, 2023
View reviewed changes
Copy link
Contributor

@priyanka-surana priyanka-surana left a comment

Choose a reason for hiding this comment

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

Merging into alignment. Any further changes needed will be done in release branch.

@priyanka-surana priyanka-surana merged commit f09fa15 into alignment Sep 26, 2023
6 checks passed
@priyanka-surana priyanka-surana deleted the test_ci branch September 26, 2023 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alxndrdiaz alxndrdiaz alxndrdiaz left review comments

@priyanka-surana priyanka-surana priyanka-surana approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@muffato @priyanka-surana @alxndrdiaz @gq1