[WIP] Add support for GitHub actions #95

jaimergp · 2019-11-27T00:39:30Z

This PR adds preconfigured support for GitHub Actions, following conversation in #90.

To do

Parameterize existing implementation in openkinome/kinoml
Add support for Windows
Add support for non-conda builds
Apparently Miniconda is preinstalled in all environments, the location being available at $CONDA. Test this is true and functional.
Variables in GH Actions use some sort of Jinja-syntax (${{ variable }}) that interferes with cookiecutter's Jinja processing. I think we just need to escape this on the unprocessed YAML to make the tests pass.

{{cookiecutter.repo_name}}/.github/workflows/CI.yaml

dgasmith · 2019-11-27T14:18:51Z

README.md

@@ -16,7 +16,7 @@ remove deployment platforms, or test with a different suite.
 ## Features
 * Python-centric skeletal structure with initial module files
 * Pre-configured `setup.py` for installation and packaging
-* Pre-configured Window, Linux, and OSX continuous integration on AppVeyor and Travis-CI
+* Pre-configured Window, Linux, and OSX continuous integration on AppVeyor+Travis-CI and/or GitHub Actions.


I am up for removing AppVeyor and Travis entirely. @janash @Lnaden any comments?

I am all for dropping Travis and VERY much for dropping AppVeyor. Go for it!

Shall I remove them as part of this PR or should this be in a separate PR (also merged to dev) for extra clarity?

jaimergp · 2019-11-27T19:50:57Z

I have added Windows support by simply forcing bash to be the default shell in all steps. This is technically supported by GHA, but I'd like to test it with a dummy package first. I think I can use openkinome/kinoml for that in the meantime.

jaimergp · 2019-11-27T20:40:06Z

That seems to work! Look here for a similar setup: https://github.com/openkinome/kinoml/pull/5/checks

jaimergp · 2019-11-27T20:50:03Z

By the way, do we want to update the CI on this repo too so it uses GH Actions?

dgasmith · 2019-12-01T14:40:05Z

This looks really great, let me try this out in a few of our packages and see how it goes.

I would vote to changing the repo's CI in another PR.

dgasmith · 2019-12-01T17:46:35Z

This is my pass at the YAML: https://github.com/MolSSI/QCElemental/pull/172/files

dgasmith · 2019-12-01T18:15:19Z

{{cookiecutter.repo_name}}/.github/workflows/CI.yaml

+{% if cookiecutter.dependency_source != 'Dependencies from pip only (no conda)' %}
+ eval "$(${CONDA_EXE} shell.bash hook)" && conda activate test
+{% endif %}
+ codecov


BTW this doesn't work without a token. I recommend using their image

- name: CodeCov uses: codecov/codecov-action@v1 with: token: ${{ secrets.CODECOV_TOKEN }} file: ./coverage.xml flags: unittests yml: ./.codecov.yml

Add --cov-report=xml to the pytest line as well.

I just noticed and was about to ask! Thanks!

{{cookiecutter.repo_name}}/.github/workflows/CI.yaml

dgasmith · 2019-12-01T18:35:54Z

{{cookiecutter.repo_name}}/.github/workflows/CI.yaml

+ shell: bash
+ if: startsWith(matrix.os, 'windows')
+ run: echo "::set-env name=CONDA_EXE::$CONDA/Scripts/conda.exe"
+{% endif %}


Please do look through this YAML, a lot of this simply isn't needed: https://github.com/MolSSI/QCElemental/blob/827e3e6faf9ba08f327bafff87e893b8f8d559fa/.github/workflows/CI.yml

So conda is in PATH? Cool!

Looks like this is not applicable to Windows! Shall I revert the changes?

Maybe add it to the path for Windows only?

I followed this discussion and there seems to be some problems with path modification. I'll try to use condabin instead, and see if that works better.

Actually, looks like conda is only in PATH in Ubuntu. I keep thinking that the CONDA_EXE approach is the cleanest although it requires one more setup step.

We could use a single step too:

- name: Set-up conda run: | if [ "${{ matrix.os }}" != "windows-latest" ]; then echo '::set-env name=CONDA_EXE::${CONDA}/bin/conda' else echo '::set-env name=CONDA_EXE::C:\Miniconda\condabin\conda.bat' fi shell: bash

Do you think we could add a conda function in this manner? If not I would recommend something simple so that further lines can do eval '$CONDA_INIT' or similar.

Best thing would be modifying the bash_profile/bashrc to autostart conda for us (but we should still run conda activate test once the environment has been created, although we could check for test existence and activate that instead?), but I think that default bash interpreters in GitHub are configured to ignore profile files, so we would need to define a custom shell in each step. I am not aware of any option to override the default shell for all steps - if that's possible, it'd be the best way. Anyway, shall I try with the bashrc+custom bash shell approach?

Ok, I've tried this route but it's not pretty at all because we would need to always specify shell: bash --noprofile --rcfile conda_init.sh -i -e -o pipefail {0}. I find that very confusing and hard to reason about.

Instead, I have opted to create this auxiliary script and call it at the beginning of each step:

- name: Install package shell: bash run: | . devtools/scripts/initialize_conda.sh python -m pip install . --no-deps conda list

That way the intention is clear and the initialization code is easier to maintain. Would you prefer this approach?

I went ahead and added the script in the last commit.

{{cookiecutter.repo_name}}/.github/workflows/CI.yaml

dgasmith · 2019-12-01T18:58:19Z

On the codecov line add --color=yes. Unsure why color isn't shown without this.

jaimergp · 2019-12-01T19:45:51Z

Oh, due to a WSL bug, my commits are dated as 4 days old. Next ones should be ok, though. Sorry for the confusion!

dotsdl · 2020-05-14T17:15:21Z

Shall we try @dgasmith's suggestion that this gets merged into a dev branch so that we can see how it behaves properly? Is there anyone with merge ability that can make this happen?

jchodera · 2020-05-14T17:37:24Z

@Lnaden @janash @bennybp are still active maintainers, as far as I know.

Lnaden · 2020-05-14T19:20:34Z

I've read through all of this, what are the things we are waiting for, @jaimergp? By my thinking, we just need to change the target branch this is being merged in to? Or is there something else I am missing?

jaimergp · 2020-05-14T23:00:13Z

We have been using GHA for a while now in several repositories, and have accumulated some knowledge on how to use it. I have some fixes to add here as well (macOS permissions, for example).

I'd say I can add the fixes here, and then we merge to dev. Additional features could be targeted in separate PRs. What do you think?

janash · 2020-05-15T01:06:48Z

Considering that we are using CookieCutter now for MolSSI Education, I would ask that the GHA change be put on a branch or that GHA is listed as an option in CookieCutter set up (travis-ci or GHA)

jchodera · 2020-05-17T18:41:11Z

I like the idea of being able to select either travis-ci or GHA until we can make sure that the GHA works robustly for a variety of people. At some point, we can just make GHA the default once folks are happy with it (and hopefully the MolSSI Education program can switch over at that point too).

Lnaden · 2020-05-18T19:18:18Z

@jaimergp Is it possible for you to make your changes here that you mentioned, then once you're done, I'll come back in and make it an option to either use travis or GHA as part of the cookiecutter? That way we can get this in without having to worry about a separate branch, and we can mark the GHA experimental while we work out kinks.

jaimergp · 2020-05-18T21:03:30Z

Added to my list. I'll add them some time this week!

dgasmith · 2020-05-20T19:41:12Z

Big fan of:

    - name: Setup Conda Environment
      uses: goanpeca/setup-miniconda@v1
      with:
         activate-environment: test
         environment-file: devtools/conda-envs/test_env.yaml
         auto-activate-base: false

    - name: Environment Information
      shell: bash -l {0}
      run: |
        conda info
        conda list

Seems to work very well these days.

jaimergp · 2020-05-25T09:55:49Z

@Lnaden, I have added the last changes I was talking about. Namely:

Rely on a GitHub Action to deal with the Miniconda stuff, as recommended by @dgasmith.
Added more informative job names
Add a "good enough" general case for the push/pr branch configuration. Only commits on master and PRs against master are tested. This removes the duplicated jobs we saw in early attempts.
Ability to choose the CI provider and update the tests accordingly
Bumped the version to 1.3. Should this be 2.0?

Todo:

I don't know how to deal with the tests for GH Actions. What should we try to cover?
Documentation must be updated to reflect these changes.

Lnaden · 2020-05-26T14:11:10Z

Awesome. I think this has come together fantastically. There is just one small change I would like to request: I am thinking we should make the travis (or +appveyor) default first until we fully debug GHA. Does that work for you?

Things I intend to do after we merge:

Add a GHA CI test
Bump version to 3.7 and 3.8
Reword some of the CI options so they are more clear on their supported OS's
Update docs.

jaimergp · 2020-05-26T14:30:31Z

That's an excellent point, yes! This shouldn't be the default just yet. I'll add the changes now.

Lnaden · 2020-05-26T14:31:00Z

Great. Once those are in, I think we can merge this. I'll keep the 1.3 until we get GHA stable, then move to 2.0

jaimergp · 2020-05-26T14:36:10Z

Hm, I am thinking that maybe we should label GH Actions as experimental in this early stage? Like this:

    "continuous_integration_provider": [
        "Travis",
        "Travis+AppVeyor",
        "GitHub Actions (experimental)"
    ],

What do you think?

Lnaden · 2020-05-26T14:39:19Z

I think thats a great idea. Makes it very clear this is a work in progress.

jaimergp · 2020-05-26T14:40:41Z

Ok, let me add that before you merge.

Lnaden

I think this is ready to merge finally. Thanks for all the work in making this ready!

Lnaden · 2020-05-26T16:28:19Z

I'm getting one more pair of eyes to look over this before I hit merge just in case I missed something obvious, but I'm investigating the nested/chained GHA in the meantime.

jaimergp added 3 commits November 25, 2019 12:51

Add draft implementation for GH actions CI

1b46c55

Escape GH Actions double curly braces

e25cdaf

... in all occurrences :)

04e6aeb

jaimergp changed the title ~~Add support for GitHub actions~~ [WIP] Add support for GitHub actions Nov 27, 2019

dgasmith reviewed Nov 27, 2019

View reviewed changes

jaimergp added 4 commits November 27, 2019 13:49

Remove flake8 linting

3589152

Drop Python 3.8 for now

2c8bd3e

Add support for non-conda testing

dc049dd

Add Windows support via bash shells

190021c

jaimergp added 2 commits November 27, 2019 15:32

Define and use a unified CONDA_EXE env variable

0666058

Do not restrict max parallel tasks

b5984df

jaimergp added 8 commits November 27, 2019 15:55

Rename some GHA steps [skip ci]

1dd4c3a

Add some debugging statements to GHA CI

9b3219f

Rename 'build' job to 'test'

b448a16

Use codecov GH action

5427c3c

Escape GHA braces

61ff699

conda is already in PATH, no need for environment variables.

9e3205c

Use PYVER env var for cleaner code

ea2f3ca

Force colored output in pytest

8cfbb24

dgasmith reviewed Dec 1, 2019

View reviewed changes

{{cookiecutter.repo_name}}/.github/workflows/CI.yaml Outdated Show resolved Hide resolved

dgasmith reviewed Dec 1, 2019

View reviewed changes

{{cookiecutter.repo_name}}/.github/workflows/CI.yaml Outdated Show resolved Hide resolved

jaimergp mentioned this pull request Mar 18, 2020

Tasks for week 2 volkamerlab/opencadd#2

Closed

5 tasks

j-wags mentioned this pull request May 15, 2020

Cookiecutter maintainership #106

Closed

jaimergp added 4 commits May 23, 2020 20:20

Migrate conda support to goanpeca's action

1692ca0

Change CI provider settings

8a025e4

Merge remote-tracking branch 'upstream/master' into gh-actions

8ababd6

Fix tests (GH Actions still needed!)

68734f7

jaimergp mentioned this pull request May 25, 2020

GitHub Actions migration openforcefield/openff-forcefields#12

Merged

Change default options in CI providers

6b41ad8

jaimergp added 2 commits May 26, 2020 16:43

Label GH Actions as experimental

00f1dcb

Add a short paragraph on GHA to the README

01df365

Lnaden approved these changes May 26, 2020

View reviewed changes

Lnaden merged commit 8bb9fdc into MolSSI:master May 27, 2020

RMeli mentioned this pull request Jun 8, 2020

Run pytest as module #108

Closed

[WIP] Add support for GitHub actions #95

[WIP] Add support for GitHub actions #95

Conversation

jaimergp commented Nov 27, 2019 • edited Loading

To do

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jaimergp commented Nov 27, 2019

jaimergp commented Nov 27, 2019 • edited Loading

jaimergp commented Nov 27, 2019

dgasmith commented Dec 1, 2019

dgasmith commented Dec 1, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dgasmith commented Dec 1, 2019 • edited Loading

jaimergp commented Dec 1, 2019

dotsdl commented May 14, 2020

jchodera commented May 14, 2020

Lnaden commented May 14, 2020

jaimergp commented May 14, 2020

janash commented May 15, 2020

jchodera commented May 17, 2020

Lnaden commented May 18, 2020

jaimergp commented May 18, 2020

dgasmith commented May 20, 2020

jaimergp commented May 25, 2020 • edited Loading

Lnaden commented May 26, 2020

jaimergp commented May 26, 2020

Lnaden commented May 26, 2020

jaimergp commented May 26, 2020

Lnaden commented May 26, 2020

jaimergp commented May 26, 2020

Lnaden left a comment

Choose a reason for hiding this comment

Lnaden commented May 26, 2020

jaimergp commented Nov 27, 2019 •

edited

Loading

jaimergp commented Nov 27, 2019 •

edited

Loading

dgasmith commented Dec 1, 2019 •

edited

Loading

jaimergp commented May 25, 2020 •

edited

Loading