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

Make CI green again and remove Python 3.5 from CI #424

Merged
merged 14 commits into from
Oct 21, 2020
Merged

Conversation

kitchoi
Copy link
Contributor

@kitchoi kitchoi commented Oct 14, 2020

This PR's goal is to make CI green again.

Closes #423
Closes #410

I tried my best to limit the scope of this PR, but it was difficult to achieve 😂 .

In summary:

  • EDM version is upgraded to use RH7 repository.
  • The test failure in Test failure on asserting identity between trait lists #423 is fixed. The test was failing in the first place but the test was not run on CI before.
  • Revert half of Add optional demo dependencies to setup.py #386 which caused CI to be run against a released egg of enable instead of local source
  • Fix test errors complaining about GuiTestAssistant not being implemented on null toolkit (see this comment).
  • Pin dependencies on traits, traitsui, pyface, apptools to released versions.
  • Install traits, traitsui, pyface and apptools from EDM, remove Python 3.5 from the CI matrix. (<-- this is a bit opportunistic)

Details:

  • First commit makes sure the current source is exercised by CI and we get the same failure in Test failure on asserting identity between trait lists #423
  • The subsequent commit tried to resolve missing link for liblzma from libtiff. The issue affects the most recent libtiff egg for RH6 repository. Upgrading EDM in order to use the RH7 repository resolves this issue.
  • Avoid Test failure when run with TraitsUI 7.1.0 #425 by pinning released versions of traits, traitsui, pyface and apptools. Further restrict to TraitsUI<7.1.0. I think the issue should be fixed separately
  • Fix the error to do with GuiTestAssistant not being available on null toolkit. It came up in the build for FIX: Make font style- and stretch- check case insensitive #405 as well. Not sure why it only started occurring then.
  • Revert half of Add optional demo dependencies to setup.py #386 which installs dependencies for running demo examples (it brings in chaco, mayavi etc). Those dependencies bring in the released version of enable, and since then enable CI has not been testing the local source. We don't really need these dependencies for running tests nor for developing enable. It seems reasonable not to install it by default.

On this:

  • Install traits, traitsui, pyface and apptools from EDM, remove Python 3.5 from the CI matrix.

This is because many recent releases have not been built for EDM + Python 3.5 any more. Python 3.5 has already passed end-of-life (see #415 too). This step I can revert.

Some other test dependencies bring in the released enable already.
This means attempt to install the local source may not override
the released copy.
Instead of installing the same releases of traits/traitsui/pyface twice,
set the requirements for EDM.

The installation for demo dependencies (see #386) was removed. They
are not used for CI, and are rarely used for development.
Installing demo dependencies brings in the released egg of enable,
causing CI to stop testing local enable source.
It will need to die soon enough.
@kitchoi kitchoi changed the title [WIP] Refresh CI setup Make CI green again and remove Python 3.5 from CI Oct 21, 2020
Copy link
Contributor

@rahulporuri rahulporuri left a comment

Choose a reason for hiding this comment

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

LGTM. Haven't tested the changes to etstool locally though.

enable/tests/tools/hover_tool_test_case.py Outdated Show resolved Hide resolved
@rahulporuri
Copy link
Contributor

rahulporuri commented Oct 21, 2020

also fixes #410

Co-authored-by: Poruri Sai Rahul <rporuri@enthought.com>
Copy link
Member

@jwiggins jwiggins left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-io
Copy link

codecov-io commented Oct 21, 2020

Codecov Report

Merging #424 into master will decrease coverage by 3.81%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #424      +/-   ##
==========================================
- Coverage   32.73%   28.92%   -3.82%     
==========================================
  Files         206      206              
  Lines       18297    18246      -51     
  Branches     2470     2466       -4     
==========================================
- Hits         5990     5278     -712     
- Misses      11929    12641     +712     
+ Partials      378      327      -51     
Impacted Files Coverage Δ
enable/layout/api.py 0.00% <0.00%> (-100.00%) ⬇️
enable/layout/utils.py 0.00% <0.00%> (-100.00%) ⬇️
enable/layout/linear_symbolic.py 0.00% <0.00%> (-100.00%) ⬇️
enable/layout/ab_constrainable.py 0.00% <0.00%> (-100.00%) ⬇️
enable/layout/constraints_namespace.py 0.00% <0.00%> (-100.00%) ⬇️
enable/layout/geometry.py 0.00% <0.00%> (-94.21%) ⬇️
enable/constraints_container.py 0.00% <0.00%> (-81.49%) ⬇️
enable/layout/layout_manager.py 0.00% <0.00%> (-72.53%) ⬇️
enable/layout/layout_helpers.py 0.00% <0.00%> (-71.43%) ⬇️
enable/coordinate_box.py 32.65% <0.00%> (-34.02%) ⬇️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6cf525c...1545226. Read the comment docs.

@kitchoi kitchoi merged commit 1e8a2dd into master Oct 21, 2020
@kitchoi kitchoi deleted the update-ci-setup branch October 21, 2020 14:25
@kitchoi kitchoi mentioned this pull request Nov 30, 2020
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.

Test failure on asserting identity between trait lists Use the latest EDM version (3.0.1)
4 participants