johanneswuerbach / saucie

This library allows you to integrate results of your frontend JavaScript tests into a [Sauce jobs results page](https://saucelabs.com/docs/javascript-unit-tests-integration).
26 stars 18 forks source link

switch to commander #59

Closed marcoow closed 7 years ago

marcoow commented 7 years ago

…as optimist is deprecated and the underlying minimist seems poorly maintained.

I also renamed the xxxSL options to just xxx (e.g. --sessionNameSL to --sessionName) for a nicer command line API.

Fixes #58

marcoow commented 7 years ago

Hm, not sure why the tests are failing actually… @johanneswuerbach: any idea?

johanneswuerbach commented 7 years ago

Great work 👏

Hm, not sure why the tests are failing actually… @johanneswuerbach: any idea?

Looks like the last runs https://saucelabs.com/u/ember-cli-sauce-ci didn't start connect, maybe the connect default true isn't working?

marcoow commented 7 years ago

Yeah, boolean defaults don't seem to be applied at all…

marcoow commented 7 years ago

@johanneswuerbach: tests are fixed for everything but Node 4. I can't reproduce the error though… Could you restart the Node 4 job on Travis maybe?

marcoow commented 7 years ago

I have no idea why the tests fail at this point… 😞

Turbo87 commented 7 years ago

@johanneswuerbach the tests on this PR seem to be passing now. I've added this.retries(2) to the test suite to let Mocha retry failed tests to reduce the risk of network issues failing the tests.

johanneswuerbach commented 7 years ago

Nice, lgtm. Could you cleanup the history and then I would be ready to merge :-)

Turbo87 commented 7 years ago

@johanneswuerbach @marcoow I've updated and rebased this PR now and everything appears to be 🍏

I got one question about the --url argument though: should the url be set to an empty string by using --url without any arguments? (is that what https://github.com/johanneswuerbach/ember-cli-sauce/blob/master/lib/commands/sauce.js#L146-L163 is doing?)

marcoow commented 7 years ago

@Turbo87: hm, somehow the change that sets connect to false if noConnect is true is gone.

I got one question about the --url argument though: should the url be set to an empty string by using --url without any arguments? (is that what https://github.com/johanneswuerbach/ember-cli-sauce/blob/master/lib/commands/sauce.js#L146-L163 is doing?)

The testem runner will add the actual URL so something like -b Safari -p iOS --url will effectively result in something like -b Safari -p iOS --url http://localhost:7135/…/tests/index.html.

Turbo87 commented 7 years ago

the change that sets connect to false if noConnect is gone

that is done automatically by commander if an option starts with --no-

The testem runner will add the actual URL so something like -b Safari -p iOS --url will effectively result in something like -b Safari -p iOS --url http://localhost:7135/…/tests/index.html.

Correct me if I'm wrong but that mean --url will not be called without an URL, right? Because that's how it's currently implemented: it requires you to give it an URL if you use the --url option.

marcoow commented 7 years ago

the change that sets connect to false if noConnect is gone

that is done automatically by commander if an option starts with --no-

👍

Correct me if I'm wrong but that mean --url will not be called without an URL, right? Because that's how it's currently implemented: it requires you to give it an URL if you use the --url option.

Yes, the command line will effectively be sth. like sauce -b Safari -p iOS --url http://localhost:7135/…/tests/index.html and if the URL was missing it should actually error out.

johanneswuerbach commented 7 years ago

Looks really good 👍

marcoow commented 7 years ago

🎉