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

Add Azure megatests #1340

Merged
merged 1 commit into from
Dec 20, 2023
Merged

Add Azure megatests #1340

merged 1 commit into from
Dec 20, 2023

Conversation

adamrtalbot
Copy link
Contributor

@adamrtalbot adamrtalbot commented Nov 28, 2023 •
edited
Loading

Additions:

  • Adds Azure megatests to Sarek
  • Adds new test profiles for Azure
  • Renames "AWS Test" to "Cloud test"

There's a cleaner way of doing this but I can't solve it right now.

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
  • If necessary, also make a PR on the nf-core/sarek branch on the nf-core/test-datasets repository.
  • 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).

@github-actions GitHub Actions
Copy link

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 nf-core/sarek 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 nf-core/sarek 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 base branch from master to dev November 28, 2023 16:25
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 28, 2023 •
edited
Loading

nf-core lint overall result: Passed ✅ ⚠️

Posted for pipeline commit 269f448

+| ✅ 142 tests passed       |+
#| ❔  10 tests were ignored |#
!| ❗   2 tests had warnings |!

❗ Test warnings:

  • files_exist - File not found: .github/workflows/awstest.yml
  • pipeline_todos - TODO string in WorkflowSarek.groovy: Optionally add in-text citation tools to this list.

❔ Tests ignored:

  • files_exist - File is ignored: .github/workflows/awsfulltest.yml
  • files_exist - File is ignored: conf/modules.config
  • files_unchanged - File ignored due to lint config: assets/nf-core-sarek_logo_light.png
  • files_unchanged - File ignored due to lint config: docs/images/nf-core-sarek_logo_light.png
  • files_unchanged - File ignored due to lint config: docs/images/nf-core-sarek_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/sarek/sarek/.github/workflows/awstest.yml
  • template_strings - template_strings

✅ Tests passed:

Run details

  • nf-core/tools version 2.10
  • Run at 2023-12-19 18:17:08

maxulysse
maxulysse reviewed Nov 28, 2023
View reviewed changes
conf/test_azure.config Outdated Show resolved Hide resolved
@vsmalladi
Copy link
Contributor

how did you setup the license server?

@adamrtalbot
Copy link
Contributor Author

how did you setup the license server?

I did not - it must go across the open internet.

@adamrtalbot
Copy link
Contributor Author

@FriederikeHanssen @maxulysse anything else we want for this?

@maxulysse
Copy link
Member

Sorry, forgot about it with the summit

@maxulysse
Copy link
Member

I like what we're doing there with rnaseq: https://github.com/nf-core/rnaseq/blob/a10f41afa204538d5dcc89a5910c299d68f94f41/nextflow.config#L248-L258

@vsmalladi
Copy link
Contributor

how did you setup the license server?

I did not - it must go across the open internet.

Okay. I will write something up next month about setting a license server for testing.

@vsmalladi
Copy link
Contributor

Do we want default machine types to be bigger for testing on Azure?

maxulysse
maxulysse reviewed Dec 19, 2023
View reviewed changes
nextflow.config Outdated Show resolved Hide resolved
maxulysse
maxulysse reviewed Dec 19, 2023
View reviewed changes
conf/test_aws.config Outdated Show resolved Hide resolved
@adamrtalbot
Copy link
Contributor Author

@maxulysse all done.

Need to test now.

@maxulysse
Copy link
Member

@maxulysse all done.

Need to test now.

so merge and test, or test and merge?

@adamrtalbot
Copy link
Contributor Author

https://github.com/nf-core/sarek/actions/runs/7261289019

Successful launch, let's manually confirm they finish.

@maxulysse
Copy link
Member

Perfection

@vsmalladi
Copy link
Contributor

Woot.

@adamrtalbot
Copy link
Contributor Author

Squash merge so we don't add 27 commits.

@maxulysse
Copy link
Member

Squash merge so we don't add 27 commits.

We disabled that on the pipelines, so you'd have to do it manually

@adamrtalbot
Copy link
Contributor Author

We disabled that on the pipelines, so you'd have to do it manually

I'm gonna rebase so hard.

@adamrtalbot
Add Azure tests to Sarek
269f448 
Change name of awstest to cloudtest since it now does Azure
@adamrtalbot
Copy link
Contributor Author

Squashed like an irritating bug. This is good to go now.

@maxulysse maxulysse merged commit 72a7af6 into dev Dec 20, 2023
23 checks passed
@maxulysse maxulysse deleted the add-azure-megatests branch December 20, 2023 08:52
@maxulysse
Copy link
Member

Thanks a lot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abhi18av abhi18av abhi18av left review comments

@vsmalladi vsmalladi vsmalladi left review comments

@maxulysse maxulysse maxulysse approved these changes

@FriederikeHanssen FriederikeHanssen Awaiting requested review from FriederikeHanssen FriederikeHanssen is a code owner

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

Successfully merging this pull request may close these issues.
5 participants
@adamrtalbot @vsmalladi @maxulysse @FriederikeHanssen @abhi18av