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

GitHub Actions #1806

Merged
merged 30 commits into from
Sep 17, 2020
Merged

GitHub Actions #1806

merged 30 commits into from
Sep 17, 2020

Conversation

valeriupredoi
Copy link
Contributor

@valeriupredoi valeriupredoi commented Sep 4, 2020

Summary

Implementation of cross-platform, cross-Python version testing:

  • Linux(Ubuntu) and OSX, both latest
  • Python 3.6-7-8
  • Installation from Conda and source
  • Tests
  • Conda package build for latest python and Linux only (as backup for the Circle test that fails coz of exceeding memory limit)

Notes

  • all tests pass for Linux and all Python versions
  • we have issues with OSX and R-packages and Python 3.8, see Installation and tests on OSX #1807
  • working tests for OSX:
    • source install bar Python 3.8
    • Conda installation for esmvaltool-python and esmvaltool-ncl
    • tests in not installation mode
  • I have added an installation decorator for the r-lint test so it's skipped when there is no working r-linter

Issues to be looked at after merge

  • The problems with OSX and Python 3.8 and also the R-packages

@valeriupredoi
Copy link
Contributor Author

@bouweandela @nielsdrost would you reckon the conda_build test should be done only on Linux and only with latest Python?

@valeriupredoi
Copy link
Contributor Author

valeriupredoi commented Sep 4, 2020

test_r_lint fails both in the GA testing environment and on my machine -- I thought we'd fixed that with either skip or run? @bouweandela

EDIT am being a dummy and I forgot to install the R packages - done now but R is being a turkey for OSX

@valeriupredoi
Copy link
Contributor Author

OSX related issues in separate issue: #1807

@valeriupredoi
Copy link
Contributor Author

OK this one's ready for merge 👍

@@ -169,9 +164,10 @@ jobs:
working_directory: /esmvaltool
docker:
- image: continuumio/miniconda3
resource_class: medium+
Copy link
Member

Choose a reason for hiding this comment

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

This breaks the circle-ci build for me :-(

Copy link
Member

Choose a reason for hiding this comment

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

https://app.circleci.com/pipelines/github/ESMValGroup/ESMValTool/3128/workflows/237313fa-aac4-41da-bbe5-c40f961c40de/jobs/36447

This job has been blocked because resource-class:medium+ is not available on your plan. Please upgrade to continue building.

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 removed that a while ago man, why are you still picking it up?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

meh proves out I didnt - sorry about that @nielsdrost - that test failes in Circle coz 137 (memory), that's why we now have a GA test that does that

@bouweandela
Copy link
Member

@valeriupredoi Nice work! It looks like you branched off from a branch in a now merged pull request. Could you merge master into this branch to minimize the changes shown in the diff tab?

@bouweandela
Copy link
Member

@bouweandela @nielsdrost would you reckon the conda_build test should be done only on Linux and only with latest Python?

Yes, that would be great.

@valeriupredoi
Copy link
Contributor Author

@bouweandela @nielsdrost would you reckon the conda_build test should be done only on Linux and only with latest Python?

Yes, that would be great.

did it last week, man, and it passes nicely all the time

Copy link
Member

@nielsdrost nielsdrost 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 go to me, thanks @valeriupredoi


# runs on a push on master and at the end of every day
on:
# triggering on push without branch name will run tests everytime
Copy link
Member

Choose a reason for hiding this comment

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

this comment could perhaps be removed.

push:
branches:
- master
- github-actions2
Copy link
Member

Choose a reason for hiding this comment

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

perhaps add a note that this is a test branch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good idea!

miniconda-version: "latest"
channels: conda-forge
- shell: bash -l {0}
run: mkdir -p condabuild_artifacts_python_${{ matrix.python-version }}
Copy link
Member

Choose a reason for hiding this comment

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

shame there is no way (that I could find) to auto-save all logs as artifacts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there is none yet (apparently, after I too have dug quite a bit); but the output is identical to the one you see on the test screen so it's prob not that useful now

@valeriupredoi
Copy link
Contributor Author

thanks very much for the review @nielsdrost - I'll go ahead and push these tests now but we'll have to revisit the OSX bits since a couple improvements from the core env ESMValGroup/ESMValCore#786 and ESMValGroup/ESMValCore#780 will have improved some aspects; there still is the R issues with OSX that I'll look into too 🍺

@valeriupredoi valeriupredoi merged commit 1125c36 into master Sep 17, 2020
@valeriupredoi valeriupredoi deleted the github-actions2 branch September 17, 2020 11:19
@valeriupredoi valeriupredoi restored the github-actions2 branch September 17, 2020 11:19
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.

3 participants