-
Notifications
You must be signed in to change notification settings - Fork 10.1k
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
Tests doesn't pass and exit with a timeout exception #519
Comments
are you using linux? |
Yes, I'm using Ubuntu GNU/Linux. |
Fodi69 no, that was not clear because it could be the other unix system aka mac. We have issues with our test suite that they are not working on linux distributions. Which is most likely the issue that you are currently bumping in to. |
Is there anything I can do to help fix the tests on Linux? Any clues to what the problem might be? I'd like to write more tests and a few features that I need for our deployment, and would definitely prefer not starting a second set of tests to work around this. If you can point me to the right direction, I'll do what I can to figure it out. |
@mixu it seems that Node.js 0.6 actually fixed this behavior, so it's not really worth the effort of making it run under 0.4 in my opinion. |
I would prefer to have this on Node 0.4 as well, and I figured it out, the problem is that on OSX, libev seems to use a different backend which results in different behavior. Basically:
Since the tests try to connect immediately without passing control back to the Node event loop, on Linux the server won't be started when the client tries to connect. Then the ECONNREFUSED error gets discarded, so nothing happens Note that according to node.cc (v.0.4.12), if you have OSX 10.6, then EVBACKEND_KQUEUE is used, but otherwise EVFLAG_AUTO is set, which defines the default backend as whatever libev deems appropriate. Can I fix this and send a pull request? I'd love to have the mainline tests work on older Node.js versions, since that's what we're on. The fix involves adding a process.nextTick() wrapper somewhere in the tests. Here is an example for the first test:
Can I add the process.nextTick() to the tests, or would you prefer some other way, e.g. by adding the wrapper to client.handshake() where possible? I'm not particularly familiar with the tests but I think I could just add the wrapper to common.js that delays execution of the client code until nextTick(), at least where the tests start with a handshake. Is that OK? |
I guess the correct solution would be to alter create() to accept a callback to run after is has been started, since then the test won't rely on undefined behavior (e.g. what happens when httpServer.listen() but control has not been passed back to the event loop). That would prevent the problem from reoccurring, though this requires a lot of tests to be restructured. The current tests rely on a platform-specific behavior in the backend which means they can break again if the libev backend is switched. Related: https://gist.github.com/1402427 |
I run 'make test' and I get:
Testing stops, so tests doesn't continue to run after this exception. (but the test suite doesn't exits, just waits and waits)
I followed the guide for contributing (https://github.com/LearnBoost/socket.io/wiki/Contributing)
If I comment this test case out, then the next one fails, and so on.
The text was updated successfully, but these errors were encountered: