Don't use fork to start the notebook in js tests #5339
Conversation
| if os.path.exists(self.server_info_file): | ||
| self._load_server_info() | ||
| return | ||
| time.sleep(0.1) |
There was a problem hiding this comment.
Just for curiosity, why you are using 0.1 resolution in time.sleep?
1 sec is too much?
There was a problem hiding this comment.
No reason to wait a whole second each time, that just makes things unnecessarily slow.
|
Hmm, I am seeing this error in a freshly cleaned git tree:
Also occurs in master though. |
|
@ellisonbg - the test suite walks the tree in IPython/html/tests/, that way we don't manually have to add the files and directories to run as tests. However, |
|
Cleaned out my git tree and tests are passing fine. This code looks good, once #5326 is merged and this is rebased, it can be merged. |
|
OK #5326 is merged so this can be rebased. |
|
rebased. |
It can encounter a weird segfault on OS X with sqlite when Qt is present (?!) The main reason to use the fork was to get the port number, but this is easy now that notebooks write a server-info file. Further advantage is that the symptom of a failed server start is no longer silence and hanging tests, but an actual failure with the server's log output.
| self.server.join() | ||
| self.stream_capturer.halt() | ||
| try: | ||
| self.server.terminate() |
There was a problem hiding this comment.
I think we should stop the server before the stream capturer - once the stream capturer has stopped, it's theoretically possible for the server to fill up its stdout/stderr buffer and hang.
There was a problem hiding this comment.
moved halt after terminate.
|
I think this is looking good. The JSController class is getting long enough that I might be inclined to pull it out into a separate module, but that doesn't need to be done in this PR. |
Don't use fork to start the notebook in js tests
Don't use fork to start the notebook in js tests
It can encounter a weird segfault on OS X with sqlite when Qt is present (?!)
The main reason to use the fork was to get the port number, but this is easy now that notebooks write a server-info file.
Further advantage is that the symptom of a failed server start is no longer silence and hanging tests, but an actual failure with the server's log output.
This will need to be rebased after #5326 is merged