pghalliday / node-BrowserStackTunnel

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

feat: Use new arguments #27

Closed budde377 closed 7 years ago

budde377 commented 7 years ago

When running browser-stack local I get the following errr:

 *** Error: Could not connect to localhost,9877,0!

This seems to be because of the arguments of the binary being updated:

The following command options format will be DEPRECATED. Please use arguments as above!
-version     Displays the version
-force   Kill other running Browserstack Local
-only    Restricts Local Testing access to specified local servers and/or folders
-forcelocal  Route all traffic via local machine
-onlyAutomate    Disable Live Testing and Screenshots, just test Automate
-proxyHost HOST Hostname/IP of proxy, remaining proxy options are ignored if this option is absent
-proxyPort PORT Port for the proxy, defaults to 3128 when -proxyHost is used
-proxyUser USERNAME Username for connecting to proxy (Basic Auth Only)
-proxyPass PASSWORD Password for USERNAME, will be ignored if USERNAME is empty or not specified
-localIdentifier SOME_STRING    If doing simultaneous multiple local testing connections, set this uniquely for different processes

This pull request should update to new args, assuming that hosts are equivalent to the BrowserstackLocal --only argument

pghalliday commented 7 years ago

Thanks for catching this :) Can you update the tests to reflect this change?

budde377 commented 7 years ago

Will do!

budde377 commented 7 years ago

Here you go.

pghalliday commented 7 years ago

thanks :)

pghalliday commented 7 years ago

ok published in version 1.5.0 (possible breaking change if people are still using old binaries)

budde377 commented 7 years ago

Nice

On Mon, Oct 3, 2016, 14:11 Peter Halliday notifications@github.com wrote:

ok published in version 1.5.0 (possible breaking change if people are still using old binaries)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pghalliday/node-BrowserStackTunnel/pull/27#issuecomment-251090686, or mute the thread https://github.com/notifications/unsubscribe-auth/ADVCy8e4Dy8vEAaTktPnsP-LPwmUQknDks5qwPD6gaJpZM4KMXQX .

serhalp commented 7 years ago

This just broke our deploy pipeline and took time to nail down. I strongly encourage you to follow NPM community best practices around semantic versioning, specifically always publishing breaking changes as major version bumps. Thank you.

budde377 commented 7 years ago

@serhalp How did it fail? Was it because of old binaries?

pghalliday commented 7 years ago

@serhalp ah yes, i forgot I was at 1.x and not 0.x - my bad

pghalliday commented 7 years ago

@serhalp - i can delete version 1.5.0 and release 2.0.0. I think that should fix your pipeline (and stop you picking up 1.5.0)

pghalliday commented 7 years ago

@serhalp - ok sorry about that, now released as 2.0.0

serhalp commented 7 years ago

Here's the error we were seeing (makes this PR findable by search engines):

BrowserStackLocal v6.1

 *** Error: Could not connect to --only!

Configuration Options:
-v
     Provides verbose logging
-f
     If you want to test local folder rather internal server
-h
     Prints this help
-version
     Displays the version
-force
     Kill other running Browserstack Local
-only
     Restricts Local Testing access to specified local servers and/or folders
-forcelocal
     Route all traffic via local machine
-onlyAutomate
     Disable Live Testing and Screenshots, just test Automate
-proxyHost HOST
    Hostname/IP of proxy, remaining proxy options are ignored if this option is absent
-proxyPort PORT
    Port for the proxy, defaults to 3128 when -proxyHost is used
-proxyUser USERNAME
    Username for connecting to proxy (Basic Auth Only)
-proxyPass PASSWORD
    Password for USERNAME, will be ignored if USERNAME is empty or not specified
-localIdentifier SOME_STRING
    If doing simultaneous multiple local testing connections, set this uniquely for different processes

To test an internal server, run:
./BrowserStackLocal <KEY>
Example:
./BrowserStackLocal DsVSdoJPBi2z44sbGFx1

To test HTML files, run:
./BrowserStackLocal -f <KEY> <full path to local folder>
Example:
./BrowserStackLocal -f DsVSdoJPBi2z44sbGFx1 /Applications/MAMP/htdocs/example/

View more configuration options at https://www.browserstack.com/local-testing

    at BrowserStackTunnel.exit (node_modules/browserstacktunnel-wrapper/src/BrowserStackTunnel.js:136:28)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:200:12)

I'm not sure how I feel about having unpublished that version... You may have just broken someone else's builds, but at the expense of avoiding breaking countless others? Unclear. It may have been cleaner to npm deprecate 1.5.0 with a message along the lines of "accidentally published breaking change in minor bump" – much easier to debug than a spontaneous 404 from npm. Oh well.

pghalliday commented 7 years ago

@serhalp - ah well it was only up for 8 hours. I'll look into npm deprecate for the future though

budde377 commented 7 years ago

Notice, the newest binaries are version 6.2

budde377 commented 7 years ago

@pghalliday I tried downloading the binaries again and now I'm getting 6.1. It might just be that BrowserStack published the binaries by mistake. This is a mess.

budde377 commented 7 years ago

I have written to BrowserStack asking about this.

pghalliday commented 7 years ago

oh dear

budde377 commented 7 years ago

Okay.. So I got a response from them saying:

Yes, we released a new version of the binary (v6.2) yesterday. Unfortunately, this resulted in Karma tests failing due to the way Karma launches the Local Testing binary. We reverted to the pervious version (v6.1) immediately. Apologies for the inconvenience caused.

They promised to keep me posted, so I'll stay on this and check-in when I hear from them again.

pghalliday commented 7 years ago

ok well i don't expect any more updates before they figure that out, so I think we're safe due to the major version update.

Interestingly, I think Karma uses this library so I hope the problem is not here :s

budde377 commented 7 years ago

Great! It was exactly failing tests in karma that made me create the initial pull request. But I have ensured that they are using v. 1.4.* until this is fixed.

On Tue, Oct 4, 2016 at 2:59 PM Peter Halliday notifications@github.com wrote:

ok well i don't expect any more updates before they figure that out, so I think we're safe due to the major version update.

Interestingly, I think Karma uses this library so I hope the problem is not here :s

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pghalliday/node-BrowserStackTunnel/pull/27#issuecomment-251380608, or mute the thread https://github.com/notifications/unsubscribe-auth/ADVCy4mJiN2oGmB0nBHuCcSokvJ0w4TXks5qwk2ngaJpZM4KMXQX .

pghalliday commented 7 years ago

ok cool

budde377 commented 7 years ago

I got a response from BrowserStack. They are promising that backwards-compatibility will be honoured for the upcoming release. Thus the new binaries should work with version 1.4 and 2.0! I'll expect them to write again when the new binaries are released and then I'll write here again.

pghalliday commented 7 years ago

thanks!

budde377 commented 7 years ago

From BrowserStack

Just an update - we have released a new version of the binary v6.2. As shared earlier, this has backward compatibility and the old commands/parameters should work.

I guess everything is alright now and people can use v1.4 as v2.

pghalliday commented 7 years ago

Awesome, many thanks for coordinating this :)