Skip to content

WIP: Migrate Travis jobs to GitHub Actions#8514

Merged
larsoner merged 93 commits into
mne-tools:masterfrom
GuillaumeFavelier:maint/gha
Nov 17, 2020
Merged

WIP: Migrate Travis jobs to GitHub Actions#8514
larsoner merged 93 commits into
mne-tools:masterfrom
GuillaumeFavelier:maint/gha

Conversation

@GuillaumeFavelier

@GuillaumeFavelier GuillaumeFavelier commented Nov 13, 2020

Copy link
Copy Markdown
Contributor

This PR follows #8335 (comment) and migrates all remaining jobs on Travis to GitHub Actions.

ToDo:

This is still work in progress.

@GuillaumeFavelier GuillaumeFavelier self-assigned this Nov 13, 2020
@larsoner

Copy link
Copy Markdown
Member

While you're digging around in CIs, could you add a step to all Azure runs that actually checks the commit message for [skip azp], [azp skip], and [ci skip] and just exit the run with succeeded status if detected? I've been hoping for months that Azure would add native support for this but they haven't.

I see two ways of doing it:

  1. Reuse a bit of code in each job, which you could adapt from SciPy here:

    https://github.com/scipy/scipy/blob/master/.circleci/config.yml#L11

  2. Make a "check skip" job that all other jobs depend on, and only run the other jobs if the check skip job says to do so. I don't think this will bottleneck things much since this job would run very quickly, then all N other Azure jobs would spawn.

Not sure which is better, or if there is another better alternative

@larsoner larsoner mentioned this pull request Nov 13, 2020
@GuillaumeFavelier

Copy link
Copy Markdown
Contributor Author

Finally a skip. Now I have to work on a reliable conditional

@GuillaumeFavelier

Copy link
Copy Markdown
Contributor Author

Okay I think Azure is good to go now

if: success()
name: 'Upload coverage to CodeCov'

# Minimal (runs with and without testing data)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Separate files would be better for restartability

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll separate them all then

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes the GitHub ones (Azure can and should stay a single file)

Comment thread .github/workflows/linux.yml Outdated
name: 'Upload coverage to CodeCov'

# PIP + non-default stim channel + log level info
pip:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

same

Comment thread .github/workflows/maint.yml Outdated
@@ -0,0 +1,38 @@
name: 'maint'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe codespell_and_flake ?

Comment thread azure-pipelines.yml Outdated
Comment on lines +56 to +59
- bash: |
make flake
displayName: make flake
condition: always()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
- bash: |
make flake
displayName: make flake
condition: always()

Comment thread azure-pipelines.yml Outdated
Comment on lines +64 to +67
- bash: |
make codespell-error
displayName: make codespell-error
condition: always()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
- bash: |
make codespell-error
displayName: make codespell-error
condition: always()

@GuillaumeFavelier

Copy link
Copy Markdown
Contributor Author

For future reference, I got the infos of the correct usage of stage& dependencies from:

microsoft/azure-pipelines-tasks#4743 (comment)

@GuillaumeFavelier

Copy link
Copy Markdown
Contributor Author

This is ready for reviews but:

  1. I still have doubts about compat / old job (it failed in baae1d1 for example). The failure seems random and I don't notice any big differences compared to the Travis environment.

  2. The GitHub actions don't react to [skip ci], [ci skip] ...etc. And this could be fixed in another PR

@larsoner

Copy link
Copy Markdown
Member

@GuillaumeFavelier our minimum is 1.15 and that part of the test is skipped for 1.16+, we should just skip it entirely. For now maybe just comment it out so that someday we come back to it?

@larsoner

Copy link
Copy Markdown
Member

I'll merge and we can iterate more as necessary, thanks @GuillaumeFavelier !

@larsoner larsoner merged commit c046c9c into mne-tools:master Nov 17, 2020
@agramfort

Copy link
Copy Markdown
Member

can you update the badges on readme @GuillaumeFavelier ? thanks heaps !

@larsoner

Copy link
Copy Markdown
Member

@cbrnr

cbrnr commented Nov 17, 2020

Copy link
Copy Markdown
Contributor

Great work! Do we have any numbers on how long tests take on GitHub Actions vs. how long they took on Travis? AFAIK the longest job on Travis was around 50-55 minutes...

@larsoner

Copy link
Copy Markdown
Member

Yes the pip pre linux on Travis was 50-55, looks like on the last green check mark above this job was ~40 minutes

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.

4 participants