Port CI to gh actions by aaronayres35 · Pull Request #426 · enthought/envisage

aaronayres35 · 2021-06-15T20:44:52Z

part of #376 (can be closed when travis and appveyor are removed)

This PR adds two nearly identical work flows. One of which will run the test suite on every PR using packages from edm, the other which will run the test suite as a cron job using ets packages from source.

(see related PRs enthought/pyface#950 and enthought/apptools#288)

aaronayres35 · 2021-06-15T21:29:12Z

CI is failing with errors of the form:

RuntimeError: No pyface.toolkits plugin could be loaded for wx

aaronayres35 · 2021-06-21T19:33:55Z

@rahulporuri Any chance you could look into the CI failure here? I am pretty stumped... 😕

mdickinson · 2021-06-22T06:56:43Z

@aaronayres35 I don't see wxPython being installed into the test environment. Possibly a bug in etstool?

mdickinson · 2021-06-22T06:59:15Z

Ah, etstool installs wxPython via pip, and I think it's installing for the wrong version of Ubuntu.

mdickinson · 2021-06-22T07:21:23Z

For wx on Linux, the key line is this one:

envisage/etstool.py

Line 237 in 5b28718

    
           "{edm} run -e {environment} -- pip install -f https://extras.wxpython.org/wxPython4/extras/linux/gtk3/ubuntu-16.04/ wxPython<4.1"  # noqa: E501

This installs a version of wxPython suitable for ubuntu-16.04, but the ubuntu-latest runner is (currently) using Ubuntu 20.04.

We'll likely need to fix the version of Ubuntu used in GitHub Actions (e.g., use ubuntu-18.04 instead of ubuntu-latest) and then update that line in etstool.py to match. We could also use ubuntu-20.04, but then there are only Python 3.8 downloads of wxPython available from https://extras.wxpython.org/wxPython4/extras/linux/gtk3/ubuntu-20.04/.

mdickinson · 2021-06-22T07:31:16Z

One more thought: there's not a lot of value in testing on wxPython anyway: Envisage has no toolkit-specific code beyond the Qt console stuff for the IPython plugins. I'd be happy to drop wxPython testing altogether, or just leave a cron job that runs with one version of Python on one platform (e.g., Python 3.8 / Linux).

aaronayres35 · 2021-06-22T11:46:50Z

@mdickinson Thank you!

…re merging

mdickinson · 2021-06-22T13:30:50Z

Be aware that the ubuntu-16.04 GitHub Actions runner is deprecated and will disappear within the next few months. (source: https://github.com/actions/virtual-environments)

aaronayres35 · 2021-06-22T15:08:16Z

I went back to using ubuntu-18.04 per the suggestion above. CI failed, but then on GH Actions, adding an install of libsdl-image1.2 solved the problem. Unfortunately, that same fix did not solve the problem for travis CI...

Given that as soon as this PR is merged I was planning to open a follow up PR to remove travis anyway, I think I am just going to kill travis now to avoid sinking more time into this

mdickinson · 2021-06-22T15:52:06Z

I think I am just going to kill travis now to avoid sinking more time into this

That definitely sounds good to me!

aaronayres35 · 2021-06-22T16:10:05Z

Hopefully this CI run will be all 🟢 . If that is the case I will stop running the cron jobs on PRs and this can get merged

mdickinson · 2021-06-22T16:21:47Z

    """
    parameters = get_parameters(edm, runtime, toolkit, environment)
-    commands = ["{edm} run -e {environment} -- python -m flake8"]
+    targets = [


Instead of this, I'd suggest changing the flake8 settings in a config file. That way the settings apply regardless of how flake8 is run, and not just in CI.

(I presume that this change is because there's some file that needs to be excluded from the flake8 run?)

It actually wasn't a single file with errors, gh actions was trying to run flake8 on everything in the edm environment and was running for 20+ min:

... ./.edm/pkgs/9c/5dfc3b11de9f6cf06b47cbe5f431d4f56bebe5fe9c1c14492052301c13cd51/pip/_vendor/chardet/langcyrillicmodel.py:264:12: E231 missing whitespace after ',' ./.edm/pkgs/9c/5dfc3b11de9f6cf06b47cbe5f431d4f56bebe5fe9c1c14492052301c13cd51/pip/_vendor/chardet/langcyrillicmodel.py:264:14: E231 missing whitespace after ',' ./.edm/pkgs/9c/5dfc3b11de9f6cf06b47cbe5f431d4f56bebe5fe9c1c14492052301c13cd51/pip/_vendor/chardet/langcyrillicmodel.py:264:16: E231 missing whitespace after ',' ./.edm/pkgs/9c/5dfc3b11de9f6cf06b47cbe5f431d4f56bebe5fe9c1c14492052301c13cd51/pip/_vendor/chardet/langcyrillicmodel.py:264:18: E231 missing whitesp This step has been truncated due to its large size. Download the full logs from the menu once the workflow run has completed.

Actually, I don't think I understand why this change was made - flake8 seems to run fine for me on the entire codebase; could you comment further?

Hmm. That's weird. How did .edm end up in the current directory?

Ah, I see; this is related to setting the EDM root directory. But putting the EDM root inside the repository clone doesn't seem right here. Is there something else we could do?

How are other ETS packages managing to avoid this issue?

It was included in the first commit for the relevant file in traitsUI:
enthought/traitsui@4823b3f
and I'm not seeing any references to the error. ~~I opened a quick PR without it on traitsUI to see if I see an error~~:
enthought/traitsui#1697 EDIT: CI finished on the traitsUI PR and there was no failure on windows... Perhaps it is not needed there? and maybe not here either

EDIT: this PR is more relevant enthought/enable#539, and here is the relevant comment thread enthought/enable#539 (comment)

Thanks. I don't want to derail this PR - we can merge this one as-is, but it would still be good to figure out a better solution sometime.

SGTM

Thank you for pointing this out! Far better than it just going unacknowledged

Perhaps it is not needed there? and maybe not here either

Ah, interesting. Could we try removing it here to see what happens? (Can do that in a separate PR if you prefer.)

Also, I note from the enable traceback that the pip install seems to be using pip's "legacy" install mode; that might be because wheel isn't installed, and could explain why this issue doesn't seem to show up in the pip bug tracker.

I went ahead and removed it here. I had already turned off ets-from-source and test-wx workflows on PR runs, but I figure test-with-edm is enough to verify if it is needed or not.

mdickinson

LGTM; a few nitpicks.

It would be good to revert the flake8 changes in etstool.py, but that can be done in a separate PR.

… job timing, grammar

port CI to gh actions

6894a08

rahulporuri self-requested a review June 21, 2021 13:14

aaronayres35 added 4 commits June 22, 2021 07:11

pull wx tests out into a single cron job on ubuntu-18.04

acc7a94

run cron jobs on this PR to ensure they work. Revert this change befo…

1910908

…re merging

use python 3.8

d9aaa2e

go back to ubuntu 16.04 and python 3.6

9e2d58d

mdickinson reviewed Jun 22, 2021

View reviewed changes

Comment thread .github/workflows/ets-from-source.yml Outdated

aaronayres35 added 5 commits June 22, 2021 09:19

use runner.os check

88f00c7

only flake8 no linux and Null

1723ec2

back to 18.04

2eda419

add libsdl-image1.2

5206c2c

add libsdl-image1.2 on travis as well

5ac5404

aaronayres35 added 2 commits June 22, 2021 10:08

remove Travis CI

0f48d52

make flake8 check more specific

d09eddf

typo missed in last commit

98db328

dont run cron jobs on pull request

c6fa58e

mdickinson reviewed Jun 22, 2021

View reviewed changes

aaronayres35 mentioned this pull request Jun 22, 2021

Test not setting edm root directory enthought/traitsui#1697

Closed

remove setting of edm root directory

0551965

mdickinson reviewed Jun 23, 2021

View reviewed changes

Comment thread .github/workflows/ets-from-source.yml

mdickinson reviewed Jun 23, 2021

View reviewed changes

Comment thread .github/workflows/ets-from-source.yml

mdickinson reviewed Jun 23, 2021

View reviewed changes

Comment thread .github/workflows/ets-from-source.yml Outdated

mdickinson reviewed Jun 23, 2021

View reviewed changes

Comment thread .github/workflows/ets-from-source.yml Outdated

mdickinson approved these changes Jun 23, 2021

View reviewed changes

rahulporuri removed their request for review June 23, 2021 09:55

suggestions from code review - add workflow_dispatch, comment on cron…

e7ea947

… job timing, grammar

aaronayres35 merged commit 5364037 into master Jun 23, 2021

aaronayres35 deleted the gh-actions branch June 23, 2021 13:16

This was referenced Jun 23, 2021

Revert recent changes to flake8 command in etstool.py #431

Merged

Migrate to GitHub Actions for CI #376

Closed

Conversation

aaronayres35 commented Jun 15, 2021

Uh oh!

aaronayres35 commented Jun 15, 2021

Uh oh!

aaronayres35 commented Jun 21, 2021

Uh oh!

mdickinson commented Jun 22, 2021

Uh oh!

mdickinson commented Jun 22, 2021

Uh oh!

mdickinson commented Jun 22, 2021

Uh oh!

mdickinson commented Jun 22, 2021

Uh oh!

aaronayres35 commented Jun 22, 2021

Uh oh!

mdickinson commented Jun 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

aaronayres35 commented Jun 22, 2021

Uh oh!

mdickinson commented Jun 22, 2021

Uh oh!

aaronayres35 commented Jun 22, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aaronayres35 Jun 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aaronayres35 Jun 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mdickinson left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mdickinson commented Jun 22, 2021 •

edited

Loading

aaronayres35 Jun 22, 2021 •

edited

Loading

aaronayres35 Jun 22, 2021 •

edited

Loading