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

Replace use of nose with pytest #92

Merged
merged 1 commit into from
Dec 21, 2020
Merged

Replace use of nose with pytest #92

merged 1 commit into from
Dec 21, 2020

Conversation

jdufresne
Copy link
Contributor

The nose project has ceased development. The last commit is from Mar 3,
2016. From their docs page:

https://nose.readthedocs.io/

Note to Users

Nose has been in maintenance mode for the past several years and will
likely cease without a new person/team to take over maintainership.
New projects should consider using Nose2, py.test, or just plain
unittest/unittest2.

pytest was already configured, so use that instead.

Rename [pytest] to [tool:pytest] to avoid deprecation warning.

Copy link
Owner

@mgedmin mgedmin left a comment

Choose a reason for hiding this comment

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

I'm in favor of this. but

  • Can py.test run my doctests? (If not, not a big loss - there are only two, and they can be rewritten as regular tests)

  • What's this new Windows failure I see in Appveyor?

@jdufresne
Copy link
Contributor Author

Can py.test run my doctests? (If not, not a big loss - there are only two, and they can be rewritten as regular tests)

Yup! 🙂 This is achieved by the --doctest-modules option in setup.cfg. You can read the pytest docs about this here: https://docs.pytest.org/en/latest/doctest.html

@mgedmin
Copy link
Owner

mgedmin commented Nov 13, 2018

No idea why appveyor was ignoring your last rebase/forced push; I've asked it to rebuild this manually.

Copy link
Owner

@mgedmin mgedmin left a comment

Choose a reason for hiding this comment

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

So what does pytest do differently to break my tests on Appveyor?

setup.py Show resolved Hide resolved
@GhostofGoes
Copy link
Contributor

GhostofGoes commented Nov 14, 2018

The Appveyor failure is odd. It's failing with a encoding error on 2.7, 3.4, and 3.5, but passing on 3.6 and 3.7.

It's unlikely but possible pytest somehow kicked up a bug that wasn't triggering under nose for whatever reason.

@jdufresne
Copy link
Contributor Author

I restored the [test] extra.

Unfortunately I don't have quick access to a Windows environment to debug this. Debugging through Appveyor is too slow.

My best guess is Bazaar._get_terminal_encoding needs some tweaking.

@mgedmin
Copy link
Owner

mgedmin commented Nov 14, 2018

It's unlikely but possible pytest somehow kicked up a bug that wasn't triggering under nose for whatever reason.

Ooh, I've an idea! py.test replaces sys.stdout, that could plausibly affect my tests!

Background: I need to detect the encoding that bzr, a Python program, uses to encode its own output on Windows. That's surprisingly tricky, but it involves the use of sys.stdout.encoding, on Pythons up to 3.5:

def _get_terminal_encoding(self):
# Python 3.6 lets us name the OEM codepage directly, which is lucky
# because it also breaks our old method of OEM codepage detection
# (PEP-528 changed sys.stdout.encoding to UTF-8).
try:
codecs.lookup('oem')
except LookupError:
pass
else: # pragma: nocover
return 'oem'
# Based on bzrlib.osutils.get_terminal_encoding()
encoding = getattr(sys.stdout, 'encoding', None)
if not encoding:
encoding = getattr(sys.stdin, 'encoding', None)
if encoding == 'cp0': # "no codepage"
encoding = None
# NB: bzrlib falls back on bzrlib.osutils.get_user_encoding(),
# which is like locale.getpreferredencoding() on steroids, and
# also includes a fallback from 'ascii' to 'utf-8' when
# sys.platform is 'darwin'. This is probably something we might
# want to do in run(), but I'll wait for somebody to complain
# first, since I don't have a Mac OS X machine and cannot test.
return encoding

tox.ini Outdated
envlist =
py27,py34,py35,py36,py37,pypy,pypy3,flake8

[testenv]
passenv = LANG LC_CTYPE LC_ALL MSYSTEM
passenv = SKIP_NO_TESTS
Copy link
Owner

Choose a reason for hiding this comment

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

Hey, why haven't I noticed this before? This could also be causing Windows test breakages?

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 restored this line and no luck. So it may be unrelated and something else is going on.

Just recently PyPy on Travis started failing too, but I can't reproduce that locally on Fedora 30.

Copy link
Owner

@mgedmin mgedmin Oct 30, 2019

Choose a reason for hiding this comment

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

The pytest issue is real, and it's the main cause of the failures. But I was wondering, if that was fixed, would we also get failures because of this?

I had to add all these envvars to tox at some point to fix various test failures on Windows. Not necessarily on Appveyor (or my Jenkins); the MSYSTEM one is probably for me manually running the tests in a Git bash shell (which uses msys2 and needs MSYSTEM for ... I don't even remember what for).

Copy link
Contributor Author

@jdufresne jdufresne Oct 30, 2019

Choose a reason for hiding this comment

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

Do you know if the pytest issue is documented in a GH issue that I could follow?

Copy link
Owner

Choose a reason for hiding this comment

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

The pytest bug is here: pytest-dev/pytest#4389

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

mgedmin added a commit that referenced this pull request Jan 23, 2020
setup.py test is deprecated and broken.  AFAIU it still uses
easy_install, which doesn't pay attention to python_requires of
dependencies, so when a transitive dependency drops Python 2.7 support
with a python_requires, pip can handle that, but easy_install can't.

I did not spend too much time investigating why exactly setup.py test
breaks: life's too short.  It's deprecated and that's reason enough to
stop using it.

I'm going to use pytest instead (because nose is also deprecated, lol).
I'm not going to switch to pytest wholesale because that would break
Windows builds (see #92).
The nose project has ceased development. The last commit is from Mar 3,
2016. From their docs page:

https://nose.readthedocs.io/

> Note to Users
>
> Nose has been in maintenance mode for the past several years and will
> likely cease without a new person/team to take over maintainership.
> New projects should consider using Nose2, py.test, or just plain
> unittest/unittest2.

pytest was already configured, so use that instead.

Rename [pytest] to [tool:pytest] to avoid deprecation warning.
@jdufresne
Copy link
Contributor Author

I rebased.

Now that Python 2 has been dropped, the Windows issues has been fixed.

@mgedmin
Copy link
Owner

mgedmin commented Dec 21, 2020

Thank you!

@mgedmin mgedmin merged commit 963dd2d into mgedmin:master Dec 21, 2020
@jdufresne jdufresne deleted the nose branch December 21, 2020 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants