Standard API

[JamesMGreene]

In this issue thread, I would like to focus on standardizing the Reporter API/interface so that we could hopefully share Reporter plugins between testing frameworks.

This would most likely come in the form of:

• an EventEmitter interface (.on(...)/.off(...)) OR an object with particular property names (e.g. runStarted/onRunStart/etc.)
• particular events/properties that would handle configuration of the Reporter itself when it is registered/unregistered with the test framework, e.g. start and stop in Intern's Custom Reporter API
• maybe a standard-ish way to register a Reporter, e.g. MyLib.addReporter(x), MyLib.reporter = x;, etc.

---

Depends on:

• [x] #1: Which events to include in the standard
• [x] #4: How to indicate status of a test

[cjohansen]

I strongly suggest an event emitter interface. This allows for easily adding multiple consumers and creates a loose coupling between the test runner and the reporters. Events is also a model that lends itself better to extension.

[csnover]

I honestly don’t feel strongly about whether we have a reporter being passed an event hub to register events on, or if the reporter objects expose methods that are invoked by the test system. Making up some terms for fun, I’ll just use “event hub” or “direct object”. (EventEmitter is not exactly right as a name since the reporters themselves are consumers of events, not emitters, so don’t want to confuse things using that name.)

The differences that I can come up with in a few minutes are basically:

Event hub:

• Probably requires reporters to be implemented in a more rigid manner (i.e. as constructors)
• Allows reporters to potentially interrogate the event hub for information on what environment they are in/what things are supported
• Design can potentially become much more complex if there are not strict enforcements on things, e.g. reporters are guaranteed to only ever get a hub once

Direct objects:

• Does not give reporters a reference back to a part of the test system, which reduces potential attack surface
• More convenient for two-way message passing than events (just use return value!)
• Slightly easier to author; instead of having to implement a method and registering it with the hub itself, it can just implement the method

Intern uses a pub/sub hub, and the reporter manager just automates the registration of methods with the pub/sub hub, so it is kind of a smooshing together of the two patterns. I actually don’t like this very much and plan to change it in a future release to have all reporters be constructors so that they can be tested multiple times without side-effects. (Right now the reporters end up storing state in closure, which works fine for the most part in real life, but is ugly when you want to test how a reporter responds to multiple possible test system states.) My gut says that the event hub pattern would end up being what I would go with, though I do rather dislike the pattern of sending event objects and then putting the stuff people actually care about on properties of the event object (event.detail? wtfever).

However this API works also likely impacts configurability of reporters; we probably also need to decide whether reporter configuration is in or out of scope of the design.

[cjohansen]

Event hub:

Probably requires reporters to be implemented in a more rigid manner (i.e. as constructors)

Why?

Allows reporters to potentially interrogate the event hub for information on what environment they are in/what things are supported

Again, why? The only interface that would be documented is something you can .on and get standardized messages.

Design can potentially become much more complex if there are not strict enforcements on things, e.g. reporters are guaranteed to only ever get a hub once

Please explain what you mean by this. A reporter listens to messages from the runner. It's in the reporter's control how many sources you have.

Direct objects:

Does not give reporters a reference back to a part of the test system, which reduces potential attack surface

"Attack surface"? We should not take into account that a reporter will be used to hack a test framework.

More convenient for two-way message passing than events (just use return value!)

Why would a reporter pass messages to the test framework?

Slightly easier to author; instead of having to implement a method and registering it with the hub itself, it can just implement the method

We're talking about runner.on("test:start", function () {}); vs reporter.testStart = function () {};. It does not make much sense to compare these in terms of how hard they are to implement.

[JamesMGreene]

I agree with all of @cjohansen's comments.

An example interface for the EventEmitter pattern might be:

MyReporter = {

  register: function( emitter, config ) {
    emitter.on( "runEnd", function( event ) {
      if ( config.isBrowser ) {
        /* ... */
      }
      else {
        /* ... */
      }
    });
    /* ... */
  },

  unregister: function( emitter, config ) {
    // Remove all associated listeners for this Reporter
    emitter.off();
  }

};

QUnit.addReporter( MyReporter );
mocha.reporter( MyReporter );
jasmine.getEnv().addReporter( MyReporter );
/* ... */

[slackersoft]

The event emitter interface feels a little roundabout to me. In order to make a single reporter work with multiple test frameworks, you'd need to pass an object to addReporter that had functions that got called back with an object that emitted events. For a non-cross framework reporter, you would probably just have the reporter directly register for events on the framework. E.G:

QUnit.on('start');
jasmine.getEnv().on('start');

This would obviously tie the reporter to a specific framework, at least for the event registration. It seems like the ability to just pass in an object with methods on it is the most straightforward implementation to reporter writers. (But then maybe I'm biased because that's how jasmine does it). Unless I'm missing something else that would make the event emitter pattern work better than I'm thinking in a case like this.

[JamesMGreene]

The event emitter interface feels a little roundabout to me.

Can't disagree there.

It seems like the ability to just pass in an object with methods on it is the most straightforward implementation to reporter writers.

Can't really disagree there either.

Unless I'm missing something else that would make the event emitter pattern work better than I'm thinking in a case like this.

I see a number of areas where the EventEmitter model seems superior:

1. Easier to extend (add new/optional events) without changing the external contract.
2. Reporter gets more control over when they want to listen, e.g. if a Reporter only started listening to "testStart" events after a test failed.
3. Reporter may choose to add more than one listener for a given event type.
4. Reporter will [presumably] be allowed to emit its own events as well by using an existing mechanism. The best example of this that comes to mind would be a more advanced reporter (e.g. JUnit XML or Coverage) which may want to listen only to common events but also emit its own "reportReady" event as something may need to be done by the end consumer in order to utilize the report (e.g. write the data to a file via PhantomJS).

[csnover]

Event hub:

Probably requires reporters to be implemented in a more rigid manner (i.e. as constructors)

Why?

Because from an architectural perspective it does not feel great to have a single required external object to function, and not provide it immediately (i.e. as an argument to the constructor). In normal circumstances I would certainly not choose a pattern of constructing a reporter object in advance and then passing stuff to one of its methods later to make it do work, in my own software.

Allows reporters to potentially interrogate the event hub for information on what environment they are in/what things are supported

Again, why? The only interface that would be documented is something you can .on and get standardized messages.

Hypothetically, because not all test systems will implement the same functionality, so a reporter may want to be able to know that so it can do something else instead. This is especially the case over time, if a future revision of a hypothetical specification is created; providing a standard mechanism for learning about what is available can help implementers.

Design can potentially become much more complex if there are not strict enforcements on things, e.g. reporters are guaranteed to only ever get a hub once

Please explain what you mean by this. A reporter listens to messages from the runner. It's in the reporter's control how many sources you have.

I mean, given @JamesMGreene’s interface example above, if register is called twice. This would break code that only expects register to be called once. If multiple hubs is required by the spec, this introduces additional complexity to reporters (holding event handlers to multiple event hubs). I would think that multiple event hubs would be an exceptionally unlikely use case and one that could be restricted. In a direct object API, it wouldn’t really matter how many hubs there were, since the test system now controls.

Does not give reporters a reference back to a part of the test system, which reduces potential attack surface

"Attack surface"? We should not take into account that a reporter will be used to hack a test framework.

Raising potential security issues from the outset is important. It’s a bad idea to assume that you can imagine all possible future uses of this API and that nobody would ever use it in an unsafe manner so we don’t have to think about it. If you don’t want to, OK, I will try to do it. :)

More convenient for two-way message passing than events (just use return value!)

Why would a reporter pass messages to the test framework?

Hypothetically, imagine a test suite that starts a server for testing, but then an error occurs during a subsequent part of the setup process such that the teardown part that closes the server is not executed. Now, the test system will be stuck open forever, because there is still an active listening server.

In the same test system, a reporter may perform an asynchronous operation, like a call to a remote server to send the results.

If you blindly process.exit() at the end of the test system, you will successfully prevent the rogue server from hanging the test system but you will also prevent that asynchronous reporter from completing. Communication back from the reporter to indicate whether or not it has finished cleaning up allows the test system to more aggressively clean up any rogue processes/servers/etc. from a failed test run, instead of hanging.

@JamesMGreene also mentioned a separate potential use case for two-way communication.

Slightly easier to author; instead of having to implement a method and registering it with the hub itself, it can just implement the method

We're talking about runner.on("test:start", function () {}); vs reporter.testStart = function () {};. It does not make much sense to compare these in terms of how hard they are to implement.

No, the former also requires additional effort in the form of performing separate registration calls and maintaining handles to clean up later (unless you just want to throw things at the wall and assume it doesn’t matter that they can’t be cleaned) inside the reporter.

[Krinkle]

@JamesMGreene

An example interface for the EventEmitter pattern might be:

MyReporter = {
 register: function( emitter, config ) {
   emitter.on( "runEnd", function( event ) {
   });
   /* ... */
 },
 unregister: function( emitter, config ) {
   emitter.off();
 }
};
jasmine.getEnv().addReporter( MyReporter );

Hm.. this seems a bit obfuscated. This would require the framework to call a method (reporter.un/register), require the reporter to know about and call a method (emitter.on), require the reporter to manually listen for each event, and yet also require the reporter object to have certain properties (register/unregister). That seems like a lot of constraints without the added value or automation I expect a constraint to provide.

A neater interface might be like this:

MyReporter = {
    runEnd: function ( data ) {
    },
    testStart: function ( data ) {
    }
};
jasmine.getEnv().addReporter( MyReporter );

Depending on whether the relevant test framework treats reporters different than regular event listeners, addReporter would just loop over the keys and call .on( key, value ), or if their .on() supports an object (like jQuery), it could even pass it directly.

[JamesMGreene]

@JamesMGreene also mentioned a separate potential use case for two-way communication.

In my use case, the Reporter isn't likely communicating with the test framework but rather trying to send a signal the end user or consuming service through it. That is a one-way outgoing message, not bidirectional communication.

[JamesMGreene]

@Krinkle: As mentioned in my earlier comment https://github.com/js-reporters/js-reporters/issues/3#issuecomment-54768547, I agree that such an interface is more straightforward but also more limiting (see aforementioned comment for reasons).

I am doubtful we will reach unanimous agreement on this particular part of the specification discussion so we'll simply have to arrive at the lesser evil.

[jason0x43]

That is a one-way outgoing message, not bidirectional communication.

It's bidirectional in the sense that the reporter is sending information back to the testing system. For the particular case of handling asynchronous actions in reporters, having the test system call methods on a reporter object actually seems to offer a simpler and more generic solution than events. The reporter methods can just return promises if they're doing something async.

[JamesMGreene]

The reporter methods can just return promises if they're doing something async.

Does the test framework need to care at all if the Reporter is doing something asynchronous if they are appropriately separated by concerns? If so, what would be an example?

[jason0x43]

Does the test framework need to care at all if the Reporter is doing something asynchronous if they are appropriately separated by concerns? If so, what would be an example?

I suppose it depends on how the test framework behaves when testing is complete. Intern, for example, terminates itself with an error code using process.exit after all the tests have finished. Say a reporter was streaming results to a database. With no way of notifying the test framework, the reporter might get cut off in the middle of a transaction when the test framework exits.

[csnover]

Intern, for example, terminates itself with an error code using process.exit after all the tests have finished.

No, it used to do this, and it broke reporters for exactly the reason you mentioned. The only time Intern forcibly quits is if there is an uncaught exception that results in test system instability. (In such a case, we do still want to allow reporters time to perform cleanup with a maybe-async operation, but we don’t right now.)

(I also did already give this same example in https://github.com/js-reporters/js-reporters/issues/3#issuecomment-54890117 :))

[jason0x43]

Ah. I've been looking at broken tests a lot lately (seeing the forcible-exit-on-/error behavior); I forgot about the standard path. :)

[Krinkle]

@JamesMGreene

[..] I agree that such an interface is more straightforward but also more limiting [..]

1. Easier to extend (add new/optional events).
2. Reporter gets more control over when they want to listen.
3. Reporter may choose to add more than one listener for a given event type.
4. Reporter will [presumably] be allowed to emit its own events [..].

There can be an EventEmitter underneath – there should be (framework's decision). Point 1 doesn't seem specific to exposing EventEmitter; providing an object of event handlers or the method to bind event handlers with – both leave the event set extendable, both have all events optional. I question the use cases behind points 2-4.

Having .on() as an interface is great. The problem is that the first-level interface then becomes a callback giving the reporter access to a larger object. And the callback easily becomes a soup of functions and logic that shouldn't have to exist (start listening, stop listening, closure, variables, logic, lots of setup code that need not be). I'm not interested in bootstrapping repeated in every reporter, introducing more potential misuse, confusion and maintenance hazards. It also significantly changes our position on responsibility, separation of concerns, and what reporters and frameworks need to know about each other.

It guess it's best to not get in too much detail of individual reporters here, but I'd like to put the "burden of proof" on adding complexity, not on removing it. I'd like to see a real-world example for those use cases and review the consuming code's quality and whether or not it'd better off without a particular feature. Seems reasonable? We all want the same thing here.

@csnover

[..] if register is called twice. This would break code that only expects register to be called once. If multiple hubs is required by the spec, this introduces additional complexity to reporters (holding event handlers to multiple event hubs). [..]

[cjohansen]

If your reporter needs to communicate back to your test runner, it is not a reporter and your design is conflating concerns. That's literally the definition of software complexity.

The problem is that the first-level interface then becomes a callback giving the reporter access to a larger object. And the callback easily becomes a soup of functions and logic that shouldn't have to exist

What's this "larger object" you're talking about? The hub you call .on() on? If this is your concern, there are ways around it, for instance:

1. The reporter can have methods named after events, the register call will loop properties and call on(propName, method)
2. The register call can receive a proxy

If the reporter needs to maintain state to do its job, then that state belongs in the reporter, NOT in the test runner.

It seems to me that the arguments against an event emitter based design are mainly

1. Wanting to do things in a reporter that does not belong in a reporter
2. Reporter definition syntax

Point 2 is moot because you can easily design a contract without explicit on calls as suggested above. I also think it is an unconvincing argument. Whether a reporter has two or three lines of code for receiving a message has very little to do with "complexity".

How do you support multiple event emitters in the case of the hard contract? Have the reporter know about it? Use an event emitter behind the scenes? ;)

In my opinion, the runner should know exactly nothing about reporters, and the reporters should know nothing about the runner, except how to register, and the message associated with each event.

[JamesMGreene]

In my opinion, the runner should know exactly nothing about reporters, and the reporters should know nothing about the runner, except how to register, and the message associated with each event.

Hear hear! :beers:

QUnit already has these types of events (though not currently exposed via a proper EventEmitter... there is an open PR (jquery/qunit#644) for that waiting on the results of js-reporters/js-reporters discussions/decisions) which are leveraged by many 3rd party plugins, not just reporters. It would be ideal if the reporters could fit into that mold and only need the test runner to provide the proxied EventEmitter that they add their listeners/callbacks with upon registration rather than having to make the all of our frameworks' test runners intimately aware of a Reporter interface with statically defined method names.

[dkl-ppi]

@csnover

If multiple hubs is required by the spec, this introduces additional complexity to reporters (holding event handlers to multiple event hubs). I would think that multiple event hubs would be an exceptionally unlikely use case and one that could be restricted. In a direct object API, it wouldn’t really matter how many hubs there were, since the test system now controls.

I don't understand what you mean with "multiple hubs". Would be nice to have example code here in the repo for a better understanding and to play with.

@cjohansen

If your reporter needs to communicate back to your test runner, it is not a reporter and your design is conflating concerns. That's literally the definition of software complexity.

You are absolutely right, but if we specify, that a reporter also has to emit specific events, as @JamesMGreene already suggested, it is up to the testing framework, what it does with the information. Sure, this doesn't allow the reporter to ask the runner for state or something else, but this is in any case a no-go.

[JamesMGreene]

@dwittner

...if we specify, that a reporter also has to emit specific events, as @JamesMGreene already suggested, it is up to the testing framework, what it does with the information. Sure, this doesn't allow the reporter to ask the runner for state or something else, but this is in any case a no-go.

I don't believe I've ever said that Reporters must emit their own events, just that I think it is a practical use case if the EventEmitter approach is taken. I would not expect the test framework to react to such an event but rather only rebroadcast it to potential consumers if the test framework itself allows consumers to add event listeners — if not, then there isn't any practical reason to allow Reporters to emit.

Theoretical example:

// Reporter setup/connection
QUnit.addReporter({
  register: function(emitter, config) {
    /*
      Emitter structure:
      { on: QUnit.on, off: QUnit._off, emit: QUnit._emit }
    */

    // Reporter
    emitter.on("runEnd", function(e) {
      /* calculate coverage... */
      emitter.emit("coverage", coverageData);
    });
  }
});

// Consumer
QUnit.on("coverage", function(e) {
  /* consume coverage data... */
});

[dkl-ppi]

@JamesMGreene

I don't believe I've ever said that Reporters must emit their own events...

You are right, you didn't say must, you said

1. Reporter will [presumably] be allowed to emit its own events

I only wanted to discuss, if it could be useful to specifiy events a reporter must emit, to handle situations @jason0x43 has described:

I suppose it depends on how the test framework behaves when testing is complete. Intern, for example, terminates itself with an error code using process.exit after all the tests have finished. Say a reporter was streaming results to a database. With no way of notifying the test framework, the reporter might get cut off in the middle of a transaction when the test framework exits.

For example:

    ...
    // Reporter
    emitter.on("runEnd", function(e) {
      /* calculate coverage... */
      emitter.emit("coverage", coverageData);
      emitter.emit("reportEnd", ...); // <- must be emitted
    });
  }
});

// Consumer
QUnit.on("reportEnd", function(e) {
  /* say goodbye ;) */
});

It is meant as a compromise for the testing frameworks, which rely on such informations.

[jzaefferer]

It would be useful to see a few usage examples of the reporters themselves. That should help answer the question if they need to use an existing API to export their results or can just provide their own.

[cjohansen]

A reporter that emits messages back to the test framework is not a reporter, it is an extension. That's outside the scope of this effort in my opinion.

[jason0x43]

Whether or not you call it an extension, a reporter is initialized by the test framework and runs in the context of the test framework (i.e., not in separate processes), so it would benefit from having some way of informing the runner whether it was busy when the the runner wants to shut down.

[cjohansen]

Benefit how? I think it is fairly uncontroversial to say that if the test framework is going to act on the reporter's command, it's doing more than reporting results.

[jzaefferer]

As mentioned before, a few actual code samples could help a lot here. For example, this JUnit-reporter for QUnit has its own API for reporting the result: https://github.com/jquery/qunit-reporter-junit#usage - it uses the currently existing callbacks in QUnit (easy to switch to even emitter) and provides a callback for the result. There might be better approaches to solve that, if you know of any, please share them.

[cjohansen]

Well, there are a few approaches to this kind of problem. What you want here is to give the reporter a dedicated output channel. The test runner itself should not know anything about this detail. Where the reporter prints/sends data is the reporter's business.

In Buster, this is solved by having a step that configures the reporter. It looks at things like command-line options etc. It then builds an output object, initializes the reporter with it, and connects the reporter to the test-runner. The test runner is blissfully ignorant of this. It just emits messages, which the reporter consumes.

The limitation of this model is that the reporter most likely cannot write to an asynchronous output. I personally find this to not be a problem, especially given the role of the reporter, and it's position in the data flow.

[flore77]

There is one more thing that I guess it should be settle in this issue, which is:

maybe a standard-ish way to register a Reporter

then I think it can be closed.

Since we have adopted the standard with the EventEmitter interface, I think there should be a method, which can add an infinite number of reporters:

testFramework.addReporter(reporter);

and there should be also a method in which we set the default reporter (better said overwrite it):

testFramework.setDefaultReporter(reporter);

I am thinking if it would be good to have also a second param called options:

testFramework.addReporter(reporter, options);

so that reporters can be customized by receiving different options. I know Mocha has this feature.

What do you think about it?

/cc @boneskull @searls

[jzaefferer]

How does the option argument work? Is that passed to a constructor, implying the reporter argument to be the constructor? Or is that taking a reporter instance?

[flore77]

Is that passed to a constructor, implying the reporter argument to be the constructor

Yes.

[jzaefferer]

Okay, sounds good to me.

[flore77]

@boneskull are you ok with this ?