mojotech / pioneer

Integration Testing
MIT License
527 stars 35 forks source link

Create driver with server and capabilities specified in pioneer.json #304

Closed tomhicks closed 9 years ago

tomhicks commented 9 years ago

Just creating an empty PR to discuss this. I'll tidy up the empty commit later.

It's a pain in the ass getting the config from pioneer.json into support/index.coffee, because that file is required and invoked by Cucumber in a way we have no control over. So we're only able to use variables in the global scope (or the local scope of support.index.coffee) that we place there before the cucumber CLI is invoked.

Where can we put these values? I've come up with the following:

1. process.argv

This is how other config gets into the support module, but putting it there is a bit painful and error-prone. process.argv is wiped in pioneer.coffee, rebuilt in config_builder.coffee and then parsed by minimist in the support module. We could ignore the minimist parsing (assuming it will happily parse/ignore non-strings) and just use process.argv directly, but what's the point of jumping through the hoops of rebuilding process.argv when all we're really doing is putting stuff into a global namespace?

Option 2 lets us simplify this.

2. global._pioneerConfig

Once the config file is read, we shove the config into global._pioneerConfig and pick it up from there.

This way we simply get our default values, merge the config JSON values in, and we have that object available when needed. Simpler than the whitelisting/rebuilding thing in config_buider at the moment, unless there's some reason that it has to be in process.argv - does cucumber look there for stuff?

3. Pioneer.config

Same as above but put it into the existing Pioneer namespace. A static/'class' property is still global but it's a little bit better packaged.

4. local scope of support/index.coffee

If the globality is a bit rubbish, we can instead invoke a method hung off the support module, passing in the config, like so:

support/index.coffee

pioneerConfig = { } # whatever the defaults are

support = ->
  # the existing support module code
  @driver = new Driver.Builder().withCapabilities(pioneerConfig.capabilities).build()

support.setPioneerConfig = (config) ->
  pioneerConfig = immutableDeepMerge (pioneerConfig, config)

module.exports = support

Then in pioneer.coffee

  applySpecifications: (obj, libPath, args) ->
    require('./support/index.coffee').setConfig(obj)
    opts = configBuilder.generateOptions(args, obj, libPath)
    this.start(opts) if opts

As @samccone pointed out on gitter, we could do this by creating some driver-building function which is invoked in the support module, which I agree might be better...

@driver = buildDriver(new Driver.Builder())

But we still have the same issue of how to make that building function available in the support module.

I really dislike option 1. For me it's jumping through hoops for the sake of it. We're still ultimately just putting something in the global scope, but we're just making it difficult for ourselves.

Option 2 is better than option 1, but not as good as option 3, in my opinion. It's just a nicer place to put a global value.

Option 4 keeps it out of global scope and moves it into the support module's local scope. I think it's harder to follow than just plopping it on the Pioneer class.

I really don't think there's any other way to do this, because it seems the CLI was intended to be invoked for a specific test run, not a generic, configurable one like pioneer provides.

Thoughts? Have I missed something? Don't care?

samccone commented 9 years ago

resolved with https://github.com/mojotech/pioneer/blob/master/docs/globals.md#driver-configuration