reactioncommerce / reaction-cli

A command line tool for working with Reaction Commerce.
33 stars 20 forks source link

allow --port #7

Closed mikemurray closed 8 years ago

mikemurray commented 8 years ago
> reaction test --port 3040
Setting up plugin imports...

Running custom test command:
 meteor test --port 3040

/Users/mikemurray/.meteor/packages/meteor-tool/.1.4.0-1.1vx5e87++os.osx.x86_64+web.browser+web.cordova/mt-os.osx.x86_64/dev_bundle/lib/node_modules/meteor-promise/promise_server.js:165
      throw error;
      ^

Error: You must specify a driver package with --driver-package
    at doTestCommand (/tools/cli/commands.js:1624:13)
    at Command.func (/tools/cli/commands.js:1496:10)
    at /tools/cli/main.js:1410:23
jshimko commented 8 years ago

That's technically the expected behavior. I wanted users to be able to run any custom test commands and still have the reaction-cli plugin loader functionality. So reaction test and reaction test unit each have default args (which are shown when you run them), but adding any new args allows you to override all of the defaults and define your own command. The benefit of doing that over just writting meteor test ... is that the plugin loader will still run before building the app.

I suppose I could add an exception for -p and --port, but that might make the override functionality more ambiguous because those args would append while anything else overrides.

brent-hoover commented 8 years ago

We should probably add --port 6000 (or whatever port) to the default command so that you can run tests alongside a running reaction. I've meant to mention that before but just kept forgetting.

mikemurray commented 8 years ago

And its also confusing when you try to run the test along side a running reaction instance and you get the following error. Then when you do --port everything breaks.

> reaction test
Setting up plugin imports...

Running full-app test command:
 SERVER_TEST_REPORTER="dot" meteor test --once --full-app --headless --driver-package dispatch:mocha

Can't listen on port 3000. Perhaps another Meteor is running?

Running two copies of Meteor in the same application directory
will not work. If something else is using port 3000, you can
specify an alternative port with --port <port>.

Also reaction test is just an alias for SERVER_TEST_REPORTER="dot" meteor test --once --full-app --headless --driver-package dispatch:mocha. So if they really wanted to overwrite those params they could just copy-paste-change that. Which is what I'm doing at the moment.

tl;dr ¯_(ツ)_/¯ maybe we change it, or maybe we don't. I think @zenweasel's suggestion a good compromise though.

jshimko commented 8 years ago

Wish granted in 0.4.14. You can now override the port with -p or --port and it'll append to the default test commands. Any other args will overwrite the whole test command (the former behavior).