jasmine / jasmine-npm

A jasmine runner for node projects.
MIT License
376 stars 145 forks source link

Add ability to pass reporter constructor and configuration on the cli #159

Closed bcaudan closed 2 years ago

bcaudan commented 4 years ago

Motivation

Following https://github.com/jasmine/jasmine/issues/1027, it could be useful to support passing reporter to the command line when:

cf https://github.com/bcaudan/jasmine-spec-reporter/issues/392

Changes

Add new command line arguments:

Let me know what you think.

sgravrock commented 3 years ago

Thanks for the PR, and I'm sorry for not responding sooner.

I'm reluctant to add this kind of complexity to the CLI. It feels like we'd be using CLI arguments to do a job that would be better done by JavaScript code. The addition of ES module support to jasmine-npm since you submitted this PR complicates matters further. Using require to load the reporter means it can't be in an ES module. Using dynamic import to load the reporter would solve that problem at the expense of making users who might not be familiar with ES modules deal with ES/CommonJS module interop. (I'm not sure how big a problem that would be in practice, but my gut feeling is that doing that along with making the constructor path configurable greatly increases the risk of Jasmine showing users errors that they don't know how to solve.)

Do you have a use case for this that wouldn't be served by either of the following options?

  1. Create an adapter module whose default export returns a properly configured reporter, and use that with --reporter, e.g.:
$ cat myreporter.js
const otherReporter = require('other-reporter');
export default function() {
    return new otherReporter.OtherReporter({some: "configuration"});
}
$ jasmine --reporter=./myreporter.js
  1. Create a helper that instantiates, configures, and adds the reporter.

  2. Provide an instantiated reporter in a jasmine.js (not .json) config file:

    const otherReporter = require('other-reporter');
    module.exports = {
    reporters: [new otherReporter.OtherReporter({some: "configuration"})]
    // ... the rest of the Jasmine configuration (specs, helpers, etc)
    };

(The first two options should work today, but the third would require some changes in jasmine-npm.)

I'd like to avoid getting Jasmine deeper into the business of loading and instantiating reporters, especially since the JS module landscape is becoming less uniform. I think we need to keep the ability to specify a reporter on the command line, for ad hoc use cases such as IdeaJasmine adding its reporter to a user-controlled configuration, but the bar for adding complexity there should be pretty high at this point.

sgravrock commented 2 years ago

Closing since this has been overtaken by changes in Jasmine. Feel free to open a new issue if you'd like to continue discussing reporter configuration.