search

metarhia / jstp

Fast RPC for browser and Node.js based on TCP, WebSocket, and MDSF
https://metarhia.github.io/jstp
Other
142 stars 10 forks source link

test: simplify resendable call tests #368

Closed nechaido closed 6 years ago

nechaido commented 6 years ago

This should fix the #361.

aqrln commented 6 years ago

I don't frankly think it is a good idea generally, but as I don't see any other solution, I'm not being opposed to it. Perhaps, we should create a separate directory for long-running tests and run only those test cases with -T?

nechaido commented 6 years ago

@aqrln, unfortunately, increasing timeout won't fix the problem.

nechaido commented 6 years ago

This PR should be landed after #369 and #370 (to avoid code coverage regression).

lundibundi commented 6 years ago

Why would we land it if https://github.com/metarhia/jstp/pull/368#issuecomment-413880631 or is it no longer true? If that's how it is then I'd like to have a separate directory for tests without timeout (I assume configuring per-test timeout is not possible in tab), wdyt? Also, is 30s really not enough for this test?

aqrln commented 6 years ago

@lundibundi it took me a moment to realize what's happening, but the PR title was changed and the branch was replaced with completely different commits. In hindsight, opening a new PR would have been a lot more appropriate to avoid confusion, but at this point it's fine as it is I guess.

nechaido commented 6 years ago

@lundibundi I've forgotten to update description in the PR. Increasing timeout doesn't help because the test may not end in some situation.

The current solution is to rewrite the test in a different and much simpler way.

lundibundi commented 6 years ago

@aqrln yeah, didn't notice such dramatic change. Edited the comment.

belochub commented 6 years ago

Landed in https://github.com/metarhia/jstp/commit/a1a0c0ea70699590429f574dd11040183b6b6e4d.