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

Catch dependency conflicts in testing (and update docs) #925

Merged
merged 6 commits into from
May 14, 2019

Conversation

tjd2002
Copy link
Contributor

@tjd2002 tjd2002 commented May 10, 2019

Per discussion at #921.

It is currently possible for our pinned requirements list to specify a set of packages that are in conflict with each other, and for tests to succeed. (This is because pip install does not fail in this case, see: pypa/pip#775 pypa/pip#988 )

Indeed, right now we have such a conflict (until #923 is merged). So adding this test should cause CI to (correctly) fail.

The pip check command checks for this condition, and fails if there are conflicts. This PR adds pip check to the tox testenv, and also recommends running it when initially writing the requirements files.

I am not sure if I put the command in the right place within tox, but I did test that it detects the conflict.

@tjd2002 tjd2002 changed the title Catch dependency conflicts in testing (and update docs) [WIP] Catch dependency conflicts in testing (and update docs) May 10, 2019
-Suggest using pip check to detect package conflicts.
@tjd2002 tjd2002 changed the title [WIP] Catch dependency conflicts in testing (and update docs) Catch dependency conflicts in testing (and update docs) May 10, 2019
@codecov
Copy link

codecov bot commented May 14, 2019

Codecov Report

Merging #925 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev     #925   +/-   ##
=======================================
  Coverage   73.01%   73.01%           
=======================================
  Files          36       36           
  Lines        2594     2594           
  Branches      494      494           
=======================================
  Hits         1894     1894           
  Misses        587      587           
  Partials      113      113

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 355e8e0...3a0af66. Read the comment docs.

@codecov
Copy link

codecov bot commented May 14, 2019

Codecov Report

Merging #925 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev     #925   +/-   ##
=======================================
  Coverage   73.01%   73.01%           
=======================================
  Files          36       36           
  Lines        2594     2594           
  Branches      494      494           
=======================================
  Hits         1894     1894           
  Misses        587      587           
  Partials      113      113

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 355e8e0...a0e58c8. Read the comment docs.

@tjd2002
Copy link
Contributor Author

tjd2002 commented May 14, 2019

Now that #923 is merged, CircleCI is passing as expected 🎉, but Appveyor fails for odd reasons:

Appveyor:py27 fails because virtualenv installs an old version of pip version (8.1.1) that doesn't support pip check. Updating pip within appveyor.yml doesn't help as pip seems to be provided by virtualenv.

Appveyor:py36 fails for a strange reason: it is still installing the conflicting urllib3 version (1.25.2) even though 1.24.3 is listed in requirements.txt and requirements-dev.txt.

Appveyor:py37 uses pip 10.0.1, and passes, so this is not a generic Windows problem.

I am going to try updating virtualenv itself within appveyor.yml to see if perhaps both of these problems will go away.

@tjd2002
Copy link
Contributor Author

tjd2002 commented May 14, 2019

Hurray! Updating virtualenv within appveyor.yml gives us a more recent pip within the virtualenvs, and also fixes the strange dependency behavior I saw in Appveyor:py36.

@tjd2002 tjd2002 requested review from dorukozturk and jcfr May 14, 2019 12:28
@bendichter bendichter merged commit 85ef17d into dev May 14, 2019
@tjd2002 tjd2002 removed the request for review from jcfr May 14, 2019 13:04
@tjd2002 tjd2002 deleted the tox-pip-check branch May 14, 2019 13:04
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