Issues to many xhr

[yosiat]

Hi,

I am using tape-run to run mobx tests and I saw that in safari and chrome not all tests results are sent back to the node http server, therefore tests never finishes.

Taken from https://github.com/mobxjs/mobx/pull/590

After some researching, I found out that for each console.log we issue POST request and it might be that we issue lots of requests and browser resources are exhausted.

I can submit a pull request that will use websocket-stream (or socket-io so we can support old browsers?) to fix that.

[juliangruber]

hmm yeah that's definitely a problem. I haven't seen it personally but I also haven't run suites with that many tests. I'll debug this with >11k tests per suite next...We're using POST requests here as they usually just work™️ everywhere.

[yosiat]

@juliangruber I see two options for fixing it:

1. Batch console messages
2. Use socket.io in order to leverage websocket.

What do you think?

[juliangruber]

I'd prefer 1) over 2) because of simplicity. But 2) would be a viable option too I guess.

[yosiat]

I can submit pull request for both options, If you want PR please choose which option you prefer.

[juliangruber]

awesome! please do 1) batch console messages then :)

that's very much appreciated!

[yosiat]

@juliangruber Great! I will work it.. it next few days I will submit PR ~

Edit: I think I will use this package - https://www.npmjs.com/package/buffered-queue for batching.

[yosiat]

@juliangruber I am trying to run the tests on master (before any change) and the "test/spawn.js" are failing - it's known issue?

λ ~/projects/browser-run/ master make test
test/close.js ......................................... 2/2 1s
test/empty.js ......................................... 1/1 441ms
test/error.js ......................................... 1/1 1s
test/phantom.js ....................................... 2/2 3s
test/spawn.js ../Users/yosi/projects/browser-run/node_modules/tree-kill/index.js:78
    ps.stdout.on('data', function (data) {
             ^

TypeError: Cannot read property 'on' of undefined
    at buildProcessTree (/Users/yosi/projects/browser-run/node_modules/tree-kill/index.js:78:14)
    at /Users/yosi/projects/browser-run/node_modules/tree-kill/index.js:99:11
    at Array.forEach (native)
    at ChildProcess.onClose (/Users/yosi/projects/browser-run/node_modules/tree-kill/index.js:94:31)
    at emitTwo (events.js:106:13)
    at ChildProcess.emit (events.js:191:7)
    at maybeClose (internal/child_process.js:877:16)
    at Socket.<anonymous> (internal/child_process.js:334:11)
    at emitOne (events.js:96:13)
    at Socket.emit (events.js:188:7)
    at Pipe._handle.close [as _onclose] (net.js:493:12)
test/spawn.js ......................................... 1/2 4s
  not ok TypeError: Cannot read property 'on' of undefined
    at:
      line: 78
      column: 14
      file: node_modules/tree-kill/index.js
      function: buildProcessTree
    stack: |
      buildProcessTree (node_modules/tree-kill/index.js:78:14)
      node_modules/tree-kill/index.js:99:11
      ChildProcess.onClose (node_modules/tree-kill/index.js:94:31)
      Pipe._handle.close [as _onclose] (net.js:493:12)
    test: TAP
    message: 'TypeError: Cannot read property ''on'' of undefined'
    source: |
      ps.stdout.on('data', function (data) {

test/stream.js ........................................ 1/1 1s
total ................................................. 8/9

  8 passing (12s)
  1 failing

make: *** [test] Error 1

[juliangruber]

hm, I don't get that error locally. This error looks very weird...which node version are you on?

[yosiat]

@juliangruber 6.7.0, by the way - I am trying very simple case -

console.log("hello");
window.close();

and I am trying to run it using:

> cat app.js | ./bin/bin.js --browser chrome

and I get no output and the launched browser is not redirecting to localhost..

[juliangruber]

hm, tests pass for me on 6.7.0. which osx version are you running?

I can reproduce the bug with chrome quitting before the results have been submitted. It should be fixed in 3.2.1, can you try again?

[yosiat]

@juliangruber I on macos sierra

The bug is actually that chrome dosen't quit and don't redirect to localhost.

[juliangruber]

yeah, chrome support is currently broken. oh, and also, your bug shouldn't appear with the default electron browser, right?

[yosiat]

@juliangruber yes it works, I want to run tests like phantomjs tests but to use the reporter.js, how do we want to do this? add only tests to firefox?

[juliangruber]

if you do cat app.js | ./bin/bin.js, you use electronjs. is there a particular reason this doesn't work for you? electronjs is basically chrome.

[yosiat]

@juliangruber I want to test the reporter.js since I am going to change it's implementation. The electronjs doesn't handle reporter.js code it gets the console.log's from the pipe.