Add `--reporters` argument

[Julien-R44]

So let's imagine that the user defines several reporters :

configure({
  reporters: [specReporter(), dotReporter()]
})

In this case, we would like to have the output of a single reporter. But to also be able to choose the output of another reporter in a different context. For example, the SpecReporter would be for local dev, and the DotReporter for the CI.

---

Update ReporterContract

The first thing to do would be to define names for the reporters so that they can be selected later via a --reporter argument. My suggestion is this:

// Instead of 
export type ReporterContract = (runner: Runner<any>, emitter: Emitter) => void | Promise<void>;

// The reporter factory would return an object like this. Name and manager
export type ReporterContract = {
   name: string // Would be 'dot', or 'spec', for the above example
   handler: (runner: Runner<any>, emitter: Emitter) => void | Promise<void>
}

However, this would consist of a breaking change. Which is pretty annoying for a small feature like this. My other suggestion, so that it does not cause a breaking change :

• Modify the ReporterContract as follows :

type ReporterHandler = (runner: Runner<any>, emitter: Emitter) => void | Promise<void>
type ReporterContract = 
 | { name: string; handler: ReporterHandler } 
 | ReporterHandler

And in case, just the handler is defined, without any name, maybe we could just assign a random one. When a user tries to use only his reporter via the --reporters argument, and it is not found, we could display an error like "Reporter not found. Make sure you define a name for it".

---

Select a reporter

To activate reporters, what I suggest :

• Just a flag --reporters which accepts a list of names of reporters. node bin/test.ts --reporters=dot,json,spec or node bin/test.ts --reporters=dot

Now the thing that's a bit boring. If I go back to the configuration that I put at the top of the issue. If I run my tests with node bin/test.ts without specifying any reporter, then both (dot and spec) will be enabled. So always having to specify a reporter could be a bit of a pain.

What I propose here is to add an enabled property in the contract of each reporter :

// The reporter factory would return an object like this. Name and manager
export type ReporterContract = 
 | { 
     name: string; 
     handler: ReporterHandler 
     enabled: boolean
   } 
 | ReporterHandler

By default, spec and dot would be set to enabled = false. Passing the name of the reporter via the flag would enable it. And if ever, when notifying the reporters in the runner, we realize that none of the reporters are enabled (which means that no --reporters argument has been passed) then we'd default to enabling the spec reporter.

I'm not really convinced by this solution, but it's the only one I could see

---

In closing, there's a question I've been asking myself:

Why allow multiple reporters? Will the user really need to run several reporters at the same time? If this is not the case, what I would suggest is maybe something like this :

// Specifiy one reporter
configure({
  reporter: 'dot'
})

// Specify custom reporter
configure({
  reporter: './path/to/reporter.ts' // or '@org/reporter'
})

And then we would import, directly from the runner, the reporter. Exactly like pino does with the transporters for example: https://github.com/Julien-R44/pino-loki#as-module

( For example, when you want to use pino-pretty, it tries to import it on its own, and if it doesn't find it, we get a nice error that invites us to install it. )

Let me know what you think about it !

[thetutlage]

Great proposal. I see two things here.

• We need the reporters array to be a collection of reporters, where all of the reporters are not enabled by default. Or in other words, they are disabled by default.
• Next, we can activate one or more reporters with the --reporters flag.
• Finally, we need at least one default enabled reporter. Otherwise, it becomes tiring to always pass the --reporter flag.

I was looking at Mocha, and they have the spec reporter enabled by default. However, the spec reporter is bundled within mocha, so they always have it available.

Have you looked at Vitest or Jest, how they approach this? I have some ideas in mind, but let's see if there is an established pattern

[Julien-R44]

Yes, so Vitest proposes a bit the same thing as my last suggestion. Except that all the reporters are built-in and directly integrated in it. No additional packages to install : https://vitest.dev/config/#reporters

Jest proposes this : https://jestjs.io/fr/docs/configuration#reporters-arraymodulename--modulename-options

I am personally not a fan of Jest's tuple syntax, and I find Vitest's approach much more clear and concise. However, the problem with Vitest's approach is that it is impossible to change reporter options. We could imagine a reporterOptions property, but this would not be typesafe :

configure({
   reporter: 'dot',
   reporterOptions: {
      dotOption: true
   }
})

[thetutlage]

How about making reporters an object with following properties.

reporters: {
  list: [],
  default: 'spec',
},

The default property can be type safe based upon what's in their in the list.

[Julien-R44]

The problem with this solution is that you can't activate several reporters at the same time unless you use the --reporters flag. But I ask again: do we really need to activate several reporters at the same time?

Or maybe we can just make it an array :

reporters: {
  list: [],
  default: ['spec','another'],
},

[Julien-R44]

Actually yes, having multiple reporters can be handy. Let's imagine that the user wants to have a json reporter that saves the result of the tests in a file but doesn't display anything in the console, but also to have the spec reporter. This is a bit of a stretch, but it could make sense

[thetutlage]

Yeah, we can allow the default one to be an array and rename the property to be defaults.

So basically, you have a hardcoded set of defaults in the config. However, you can overwrite them using the --reporters flag

[Julien-R44]

Okay, that sounds nice. The only thing that bothers me a little is that it results in a breaking change. But at this point, I don't think we have much choice if we want to offer an API that's not too ugly

If it sounds okay for you, I'm willing to implement it this weekend

[Julien-R44]

Let me just recap:

• We forget about the non breaking change attempt on the ReporterContract knowing that we have another breaking change, so not much interest except being error prone. This means that the contract of the reporter becomes the following :

type ReporterContract = 
   | { name: string; handler: ReporterHandler } 
   | ReporterHandler

• Then, as you said, we keep that API for the configure function :

reporters: {
  list: [
     specReporter(),
     anotherReporter()
   ],
  defaults: ['spec','another'],
},

As simple as that !

[Julien-R44]

Actually, no, sorry. It is perfectly possible to make the thing non-breaking.

• We keep my suggestion about the ReporterContract.
• And finally, we do the same thing for the reporters configuration property. Basically: if reporters is an array, like in the current API, then we just enable all reporters. If it's an object, then we use the new system.

[thetutlage]

Right now we do not have many 3rd party reporters. So, we can break the ReporterContract and ship them as major server. People won't have to change a line in their config.

For the config reporters property, yes, it can be backwards compatible.

[Julien-R44]

Yeah, I was mostly thinking of people that would potentially write an inline reporter. Or who would have just kept their reporter private. Very unlikely to be the case honestly, but possible.

[thetutlage]

Yeah, but I think 0.1% chance of that. Also, we will mark it as a major release. Its just I do not want others to change their config too after the upgrade.

[Julien-R44]

Okay great. Thanks a lot for the feedback dude. I'll get right on it quickly!

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.