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

switched from nose to pytest #266

Merged
merged 1 commit into from Apr 25, 2018
Merged

switched from nose to pytest #266

merged 1 commit into from Apr 25, 2018

Conversation

meitham
Copy link
Contributor

@meitham meitham commented Aug 20, 2017

This is a rather large commit, and apologies in advance that I didn't discuss the change before making the pull request.

This current test suite uses nosetest which is a project that is no longer being maintained. Nose website is recommending two alternatives to nose, nose2 and pytest. I picked pytest as it is the more popular choice in the community, and for the test fixtures it provided.

I have left the setup function (that runs before every test), but I have commented the lines where we call it. It is no longer needed with pytest, as each test received a new instance of neovim.

The tests are slighly slower on pytest compared to nose, (0.5 seconds difference), but this can be improved if we add the pytest-xdist plugin, where the tests can run in parallel.

@justinmk
Copy link
Member

Nice! Thanks for doing this.

For some reason master is failing CI now, that means something changed externally. Would feel better if CI was green before merging this.

@meitham
Copy link
Contributor Author

meitham commented Aug 20, 2017

I'm not able to reproduce the failures locally. I have python2.7 and pypy, both passing with tox. Could it be something specific to travis-ci?

@justinmk
Copy link
Member

Yes, possibly. That's what needs to be figured out :)

@heliomeiralins
Copy link
Contributor

heliomeiralins commented Aug 23, 2017

Sometimes the tests freeze on Python3.6 when running tox locally.
It happend here too: https://travis-ci.org/meiralins/python-client/jobs/266652469

I will try to work on the issues on the weekend.
edit: found build history on travis.

@meitham
Copy link
Contributor Author

meitham commented Aug 30, 2017

@meiralins have you had the chance to look at the issue past weekend? If not, is there anything I can do to get this moving?

@heliomeiralins
Copy link
Contributor

@meitham sorry, I didn't.

@justinmk
Copy link
Member

I'm not able to reproduce the failures locally. I have python2.7 and pypy, both passing with tox. Could it be something specific to travis-ci?
... is there anything I can do to get this moving?

The two travis builds I looked at, show this error many times:

RuntimeError: Event loop stopped before Future completed.

Next step would be to compare the versions of whatever other packages are involved, with your local environment. Or start a ubuntu trusty 14.04 VM, and try the tests there. Most precise would be to do the same steps that we tell the CI to do.

There's no point in merging this if the tests are failing. I don't have time to investigate this myself.

@justinmk
Copy link
Member

pytest-xdist plugin, where the tests can run in parallel

Just noticed this part. I wonder if that was related to the failures on CI?

Anyway it seems to pass now, great!

]

extras_require = {
'pyuv': ['pyuv>=1.0.0'],
Copy link
Member

Choose a reason for hiding this comment

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

what's this for?

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 thought this is convention in setuptools to define what your tests requirement is, so that the below works.

python setup.py test

@meitham
Copy link
Contributor Author

meitham commented Apr 25, 2018

I think the previous CI failure are related how pytest captures the stdout/stderr which interferes with msgpack, adding the -s flag alone (no capture) did not help, but there were few bugs fixed in latest pytest around capturing so a combination of no capture and pytest upgrade did the trick.

@justinmk
Copy link
Member

lint build is failing:

checkqa runtests: commands[0] | flake8 neovim
neovim/compat.py:42:1: D400 First line should end with a period

Looks like that is also failing on master ...

@bfredl
Copy link
Member

bfredl commented Apr 25, 2018

I have a fix for that in #325

@justinmk
Copy link
Member

Anyone else want to stare at this for a sanity check? Else I will merge.

@bfredl
Copy link
Member

bfredl commented Apr 25, 2018

@justinmk Feel free to merge. I'll fix #325 afterwards.

@justinmk justinmk merged commit b65f62d into neovim:master Apr 25, 2018
bfredl added a commit that referenced this pull request Apr 29, 2018
This release adds support for Python 3.7 which now is in beta. `async`
is a keyword in python3.7, and it should be replaced with `async_` when
used as a keyword argument to API methods. `async` is still supported
with python 2.7 and 3.6 for the moment, but considered deprecated.

Also, integration with the in-process lua interpreter in Nvim is now supported.

Changes since 0.2.4:

* debcde0 clean up remote object implementation (#313)
* e880fe7 Guidelines for local plugin development (#317)
* 1ab98e8 Update the working directory on DirChanged for legacy plugins (#296)
* d53415d Fix SyntaxError in py3.7: use `async_` instead of `async` (#274)
* b65f62d Use `pytest` as `nosetests` are not longer maintained (#266)
* d9aed96 Support using lua functions. `buf.update_higlights` as an internal example (#325)
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