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

EDM-based CI #365

Merged
merged 8 commits into from
Jul 11, 2017
Merged

EDM-based CI #365

merged 8 commits into from
Jul 11, 2017

Conversation

jvkersch
Copy link
Contributor

@jvkersch jvkersch commented Jul 9, 2017

Cleaned-up version of #364

This adapts the EDM-based CI code for Enable in enthought/enable#281 to Chaco. The biggest change is to run the backend tests in addition to the standard tests (however, judging from the README we could probably move the backend tests back in with the rest of the tests).

@corranwebster @itziakos @jwiggins Would you mind taking a look?

@jvkersch jvkersch changed the title Ci/edm chaco EDM-based CI Jul 9, 2017
@corranwebster
Copy link
Contributor

Basically looks good to me. Obvious improvements would be:

  • minimizing the number of different runtimes installed (ie. install a single 2.7 runtime with wx and pyside and then run tests for each). Doing this improved runtimes and lowered cache sizes.
  • running on OS X (should be straightforward)
  • setting up Appveyor (more work)

but those are all second order compared to getting this into master.

@jwiggins
Copy link
Member

I would remove all the tomfoolery concerning pillow in edmtool.py since it's not needed.

.travis.yml Outdated
cache:
- pip
- ccache

addons:
apt:
packages:
- python-qt4
Copy link
Member

@itziakos itziakos Jul 10, 2017

Choose a reason for hiding this comment

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

since we use the edm binary packages we should not need to install python* packages and zlibg-dev, libpng-dev, libfreetype6-dev from apt

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 call, thanks. Updated.

matrix:
include:
- env: RUNTIME=2.7 TOOLKIT=wx
- env: RUNTIME=2.7 TOOLKIT=pyqt
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason why pyside is not included in the matrix jobs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No particular one, other than that I looked at the current Travis file and misread what it does. I'll add PySide.

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've added a configuration for PySide under 2.7.

ci/edmtool.py Outdated
installed to access required source code from github repositories.
You can then do::
python edmtool.py install --runtime=... --toolkit=...
to create a test environment from the current codebase and::
Copy link
Member

@itziakos itziakos Jul 10, 2017

Choose a reason for hiding this comment

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

Adding some empty lines will improve readability in these sections

@jvkersch
Copy link
Contributor Author

@jwiggins @itziakos I think I've addressed your comments, would you care to give this another look?

@jwiggins
Copy link
Member

Looks good from this end

@jvkersch
Copy link
Contributor Author

Thanks all, merging.

@corranwebster I'll take a shot at OS X and Windows soon.

@jvkersch jvkersch merged commit a439a92 into master Jul 11, 2017
@jvkersch jvkersch deleted the ci/edm-chaco branch July 11, 2017 09:41
@jwiggins
Copy link
Member

Nice work! Any insights to be added to enthought/enable#281?

@jvkersch
Copy link
Contributor Author

Any insights to be added to enthought/enable#281?

@jwiggins Just one perhaps: Ioannis pointed out that installing the python-* packages via apt is no longer necessary, so it looks like those could be removed from the Enable setup as well. Other than that, nothing to add: the Python code is copied in its entirety from your branch.

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.

None yet

4 participants