nathanboktae / mocha-phantomjs

:coffee: :ghost: Run client-side mocha tests in the command line through phantomjs
MIT License
954 stars 112 forks source link

config.hooks only accepts module name/path? #170

Closed shellscape closed 9 years ago

shellscape commented 9 years ago

https://github.com/metaskills/mocha-phantomjs/blob/master/lib/mocha-phantomjs.coffee#L185

It seems awfully restrictive that config.hooks only supports a module name or path. I'd like to request this be updated to check the type of the value passed. If it's an object it should pass that along, rather than try to require on an object.

nathanboktae commented 9 years ago

That's the only way it can work. the hooks object needs to have functions hanging off of it, and the config is the JSON serialized object from the command line - JSON doesn't support functions.

shellscape commented 9 years ago

@nathanboktae why so close-happy on tickets? You didn't give any time for discussion at all. Come on now.

Considering:

      .pipe(mocha({
        phantomjs: {
          webSecurityEnabled: false,
          localToRemoteUrlAccessEnabled: true,
          ignoreSslErrors: true,
          hooks: 'mocha-phantomjs-istanbul'
        }
      }))

There's no fundamental difference between:

config.hooks = require(module)

which exports an object with the afterEnd method, and

config.hooks = { afterEnd: function(runner) { } }

So why not allow for this?:

      .pipe(mocha({
        phantomjs: {
          webSecurityEnabled: false,
          localToRemoteUrlAccessEnabled: true,
          ignoreSslErrors: true,
          hooks: { afterEnd: function(runner) { } }
        }
      }))
nathanboktae commented 9 years ago

mocha-phantomjs runs in phantomjs, not node. Also you're giving example code from something not mocha-phantomjs but I'm guessing some wrapper around it for a node.js build system like grunt or gulp. config is hydrated by code running in phantomjs from JSON passed in the command line.