search

pghalliday / node-BrowserStackTunnel

https://www.npmjs.com/package/browserstacktunnel-wrapper
MIT License
17 stars 24 forks source link

Add tunnel identifier #4

Closed vojtajina closed 10 years ago

pghalliday commented 10 years ago

ok i merged this but had to add quite a few tests and in doing so noticed that you broke the multiple hosts support, i fixed it now though.

However I also noticed a problem with the mocks (I think) as I now get this warning when running tests:

(node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit.
Trace
    at process.EventEmitter.addListener (events.js:160:15)
    at process.on.process.addListener (node.js:769:26)
    at BrowserStackTunnel.startTunnel (src/BrowserStackTunnel.js:125:13)
    at BrowserStackTunnel.start (src/BrowserStackTunnel.js:133:12)
    at Context.<anonymous> (/vagrant/test/src/BrowserStackTunnel.js:297:24)
    at Test.Runnable.run (/vagrant/node_modules/grunt-mocha-test/node_modules/mocha/lib/runnable.js:194:15)
    at Runner.runTest (/vagrant/node_modules/grunt-mocha-test/node_modules/mocha/lib/runner.js:355:10)
    at /vagrant/node_modules/grunt-mocha-test/node_modules/mocha/lib/runner.js:401:12
    at next (/vagrant/node_modules/grunt-mocha-test/node_modules/mocha/lib/runner.js:281:14)
    at /vagrant/node_modules/grunt-mocha-test/node_modules/mocha/lib/runner.js:290:7

I think they are getting leaked from the mocks, as that is where the listeners are added and removed, but I haven't been able to quickly assess what might be the root cause. If you could take a look at that it would be much appreciated. I'd like to get that resolved before I publish the changes.

Many thanks for the input though :)

pghalliday commented 10 years ago

hmm, I don't think it's the mock spawn that's leaking listeners, more likely the tunnel code itself. I'm gonna put this to one side for now and publish as is seeing as how it hasn't been causing problems up to now. I will add a new issue as a reminder to get back to it

pghalliday commented 10 years ago

Also I just realised that these leaks have nothing to do with the changes you added :o

pghalliday commented 10 years ago

version 1.1.2 published

vojtajina commented 10 years ago

Thanks!

On Sun, Jan 12, 2014 at 4:06 AM, Peter Halliday notifications@github.comwrote:

version 1.1.2 published

— Reply to this email directly or view it on GitHubhttps://github.com/pghalliday/node-BrowserStackTunnel/pull/4#issuecomment-32120949 .