Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from

Conversation

bcaudan
Copy link
Contributor

@bcaudan bcaudan commented Mar 24, 2020

Motivation

Following jasmine/jasmine#1027, it could be useful to support passing reporter to the command line when:

  • reporter constructor is not the default export of the reporter file
  • reporter constructor can take a configuration parameter

cf bcaudan/jasmine-spec-reporter#392

Changes

Add new command line arguments:

  • --reporter-constructor= to specify the field to find the constructor in the reporter file exports
  • --reporter-configuration= to specify a json configuration file path

Let me know what you think.

@sgravrock
Copy link
Member

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
Copy link
Member

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.

@sgravrock sgravrock closed this Jul 23, 2022
sgravrock added a commit that referenced this pull request Oct 22, 2022
This supports more complex scenarios than the --reporter= CLI flag
(multiple reporters, reporters that need configuration, reporters
that aren't default exports, etc) without pushing the complexity of
all of those scenarios into Jasmine or requiring the user to move
to programmatic usage.

See #77 and #159.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants