-
Notifications
You must be signed in to change notification settings - Fork 126
Fix tests on Travis #347
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
Fix tests on Travis #347
Conversation
Can somebody please stop the jobs at https://travis-ci.org/neovim/python-client/builds/403972108 ? Would be good if I could do this in the future myself, but as for this PR I am only running one job from now on, so it should be fine then. |
The problem is The log:
Running it locally adds the following at the end, which is missing on Travis:
It passed when adding logging the first time though:
https://travis-ci.org/neovim/python-client/jobs/403974893 So it is maybe only flaky and logging fixes it somehow? |
dead-locking is mentioned in the test: https://github.com/neovim/python-client/blob/ebeacd017abc195fd82cf00f0f85c3720b7713a6/test/test_client_rpc.py#L54 - but I do not know how it's meant. @bfredl The last two jobs succeeded and failed, without code changes. Does c224007 (on top of the mentioned build) makes sense? |
Stopping here for now - somebody with more insight should take a look now :) |
Fixed by adding a sleep after |
.travis.yml
Outdated
@@ -23,7 +24,7 @@ python: | |||
before_install: | |||
- if [ $CI_TARGET = tests ]; then | |||
eval "$(curl -Ss https://raw.githubusercontent.com/neovim/bot-ci/master/scripts/travis-setup.sh) nightly-x64"; | |||
pip install -q scrutinizer-ocular tox-travis; |
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.
What is this change for?
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.
It is not really necessary, will remove it.
test/test_client_rpc.py
Outdated
@@ -41,6 +42,11 @@ def request_cb(name, args): | |||
# this would have dead-locked if not async | |||
vim.funcs.rpcrequest(vim.channel_id, "test-event", async_=True) | |||
vim.run_loop(request_cb, None, None) | |||
|
|||
# This sleep is required on Travis, where it hangs with |
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.
This shouldn't really be needed, but let's merge this for now. Let's mark it with TODO
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.
Done.
@@ -6,6 +6,7 @@ env: | |||
- NOSE_VERBOSE=2 | |||
- NOSE_WITH_COVERAGE=true | |||
- NOSE_COVER_PACKAGE=neovim |
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.
we don't use nose anymore, so we can remove these envs I guess?
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.
Likely, yes. Should be a seperate PR then though.
Very nice, thanks. |
No description provided.