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
Switch from nose to pytest #5282
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me other than this one comment
@@ -6,13 +6,15 @@ FROM debian:wheezy | |||
# Add an unprivileged user: | |||
RUN useradd --create-home --home-dir /home/lea --shell /bin/bash --groups sudo --uid 1000 lea | |||
|
|||
# Install pip, sudo, openssl, and nose: | |||
# Install pip, sudo, openssl, and pytest: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The lines immediately below this don't install pytest, so just remove pytest from this line. Same edit for anything starting with "Install pip, " in this change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made the pytest
changes.
I think the "Install pip" comments are accurate as we do install pip
from the package manager, but then update it through pipstrap
so I changed comments to clarify this.
Fixes #4377 and #4825.
nose
is no longer maintained so let's switch to a framework that is.A couple things to note here:
_multiprocess_can_split_
lines were fornose
's multiprocessing plugin as documented here and have been removed.nose
improperly reporting the coverage percentage andpytest
reporting coverage to the nearest hundreth of a percent rather than the nearest whole percentage point.route53
was particularly affected becausenose
wasn't reporting coverage on an entire file.