How to migrate from 0.4 to 0.8?

[ydaniv]

I have tests using Intern and Chai that run with 0.4.1 but upgrading to any other version fails badly.

Specifically, in 0.8.0, all WS connections fail.

Couldn't find this covered anywhere.

Thanks for the great work!

[thoov]

@ydaniv Sorry to hear that 0.8 isn't working for you. Do you happen to have any code snippets to share so I can take a look?

[ydaniv]

Sure! Take a look at tests of Backbone.WS. Install dev deps via npm, update mock-socket to 0.8 and run tests.

[ydaniv]

ping > @thoov, does that help? too much work? any way I can help you help me?

[thoov]

@ydaniv Sorry for the delay. Yes that helps and I am able to reproduce the error on my side. Let me take a look at this tomorrow and see if I cant find whats going wrong

[ydaniv]

:+1:

[thoov]

@ydaniv So I was able to track down the issue and it seems like there is a timing problem within the tests. I have updated mock-sockets to 0.8.1 which will help you out. If you get that version and comment out 3 tests: test destroy, construct with retries, test reopen all tests will pass. I believe there are more timing problems but I am not as familiar with your testing environment to dig much farther. Let me know if this helps you out.

[ydaniv]

Thanks @thoov! I got your update and removed the construct with retries and test reopen tests. The test destroy did have a timing issue which I fixed.

The problem with the other 2 seems to be related to calling server.close(). This seems to kill all subsequent tests. Any idea?

[thoov]

@ydaniv Yes calling server.close() will cause the behavior that you are seeing. Normally I would suggest moving the code found here: https://github.com/ydaniv/backbone-ws/blob/master/tests/mocks.js#L4-L28 to your beforeEach: https://github.com/ydaniv/backbone-ws/blob/master/tests/unit/construction.js#L22 (if you do that then in your afterEach you will want to call server.close()). That way you will have a "fresh" server for each test. I could also add something like server.restart().

[ydaniv]

Cool! Let me test that and get back to you. What woud server.restart() desugar to?

[ydaniv]

OK, I see now, the problem is that in 0.4 it would only close connections but now you're removing the server from listening on the given URL so all subsequent connections will fail.

Perhaps a restart is not necessary, what I really need for testing purposes is to simulate a connection being abruptly terminated from server side. Is that possible now?

[ydaniv]

wOOt!

I've updated and followed your advice regarding server restart in afterEach and it works like a charm.

This had 2 interesting consequences:

1. I was forced to continue flow from the onopen handler or else things break. Which is great!
2. I've discovered that when the client sends a message a reply is sent synchronously back, which I think shouldn't be the required behavior. Shouldn't reply be deferred to next task on the event loop? Or at least next tick?

So a server.restart() would definitely be welcome!

Will it help if I attempt at a PR for adding server.restart()/fixing the sync reply issue or both?