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

Timeout tests when they hang. #75

Merged
merged 12 commits into from
Aug 11, 2017
Merged

Timeout tests when they hang. #75

merged 12 commits into from
Aug 11, 2017

Conversation

Korijn
Copy link
Contributor

@Korijn Korijn commented Aug 10, 2017

Hopefully this will snuff out the test that times out on AppVeyor.

  • Added pytest-timeout
  • Configured test timeout to 10s
  • Added .vscode to gitignore

Continuation of #74

@Korijn
Copy link
Contributor Author

Korijn commented Aug 10, 2017

Looks like the test suite at least completes now, instead of timing out the AppVeyor build... now to figure out which one it is!

@Korijn
Copy link
Contributor Author

Korijn commented Aug 10, 2017

It's test_can_execute_subprocess_primitive and a few others...

@Korijn
Copy link
Contributor Author

Korijn commented Aug 10, 2017

Oddly enough there seem to be race conditions at work:

image

One passed, two failed, and one is hanging indefinitely, even with pytest-timeout...

@Korijn
Copy link
Contributor Author

Korijn commented Aug 10, 2017

I'm not seeing any failing tests anymore. But AppVeyor still returns with a nonzero exit code. What could be the root cause?

Also, in one of the passing builds, we still see this in the console output:

Exception ignored in: <bound method _ProactorSocketTransport.__del__ of <_ProactorSocketTransport closing fd=-1>>
Traceback (most recent call last):
  File "c:\python34\lib\asyncio\proactor_events.py", line 90, in __del__
    self.close()
  File "c:\python34\lib\asyncio\proactor_events.py", line 78, in close
    self._loop.call_soon(self._call_connection_lost, None)
  File "C:\projects\quamash\quamash\__init__.py", line 353, in call_soon
    return self.call_later(0, callback, *args)
  File "C:\projects\quamash\quamash\__init__.py", line 330, in call_later
    return self._add_callback(asyncio.Handle(callback, args, self), delay)
  File "C:\projects\quamash\quamash\__init__.py", line 347, in _add_callback
    self.__timers.append(timer)
AttributeError: 'NoneType' object has no attribute 'append'
Build success

@Korijn
Copy link
Contributor Author

Korijn commented Aug 10, 2017

Perhaps we need to use pytest-xdist's option --boxed...

@Korijn
Copy link
Contributor Author

Korijn commented Aug 11, 2017

I've configured pytest-xdist to box all tests in isolated subprocesses, and I've removed the skip markers. Let's see if that will give us a decent overview of test failures...

Ugh, it's not supported on Windows...

@harvimt
Copy link
Owner

harvimt commented Aug 11, 2017 via email

@Korijn
Copy link
Contributor Author

Korijn commented Aug 11, 2017

Alright, so, what I've learned in this process is that:

  • All tests randomly fail on Windows, so test skip decorators don't make any sense. Therefore I've removed the decorator I added in a previous PR.
  • Boxing tests to subprocesses with pytest-xdist is not supported in Windows, so that doesn't help either.
  • I added pytest-timeout to make sure AppVeyor doesn't hang indefinitely.

This PR won't help getting AppVeyor builds to pass. Someone really needs to dig into the internals of the library and the tests to figure out what is going on.

I've updated #73 to reflect the findings here.

This PR can be merged as is.

@Korijn Korijn changed the title pytest timeouts Timeout tests when they hang. Aug 11, 2017
@Korijn
Copy link
Contributor Author

Korijn commented Aug 11, 2017

@harvimt Can this be merged?

@harvimt harvimt merged commit 5c8bc08 into harvimt:master Aug 11, 2017
@Korijn Korijn deleted the test_timeout branch August 11, 2017 07:25
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.

2 participants