testdouble / teenytest

A very simple, zero-config test runner for Node.js
MIT License
97 stars 14 forks source link

Add reporting hooks that conform to js-reporters #15

Open searls opened 8 years ago

searls commented 8 years ago

One half of our extensibility story is getting information out of the tests as they run (or afterward). It'd be nice if we had this, and we may as well implement it to the emerging spec since it's greenfield and there's no reason not to: https://github.com/js-reporters/js-reporters

flore77 commented 8 years ago

Hi, I am currently taking care of the js-reporters project and I want to help implementing the spec into teenytest.

I am not sure how the suite that is defined in the spec:

Suite: A suite is a collection of tests and potentially other suites.

can be represented for teenytest.

I was thinking all objects should be considered suites and only functions should be considered tests.

For example:

module.exports = {
  test1: function() {},
  ...
  testn: function() {}
}

all this type of tests, that we call global tests, should be part, only theoretically, of a global suite, which must not be emitted.

module.exports = {
  suite1: {
    test1: function() {},
    ...
    testn: function() {}
  },
  suite2: {
    suite21 {
      test1: function() {},
      ...
      testn: function() {}
    },

    test1: function() {},
    ...
    testn: function() {}
  }
  ...
  suiten: {
    ...
  }
}

So only nested tests will really contain suites.

searls commented 8 years ago

Please bear in mind I'm not familiar with the js-reporter spec yet other than a cursory review. A few pieces of background:

As a result, I would define this as a global teenytest suite:

// $ teenytest **/*.test.js

// foo.test.js
module.exports = {
  test1: function(){},
  sub: {
    test2: function(){}
  }
}

// bar.test.js
module.exports = function test3 () {}

Conceptually, this seems pretty solid to me. Could you explain the nature of the issue with this? Is this just a case the spec didn't consider?

flore77 commented 8 years ago

Please bear in mind I'm not familiar with the js-reporter spec yet other than a cursory review.

Sure, neither do I with teenytest :smile:

I was talking about that teenytest does not define the term suite explicitly, how it is defined in other frameworks like Jasmine, Mocha etc. and in the js-reporter spec. What resembles most to the suite term definition in teenytest are the objects that are grouping more tests.

The above definition of the global suite is perfect, the global suite must contain all the tests and groups of tests (suites). This suite must be emitted on the runtStart event, which is triggered only once.

I am not sure about the foo and bar suites, the other frameworks do not name a suite after the file name and neither the js-reporter spec does not mention something about this, I would see something like:

this would be done in js-reporters hooks:

Maybe some people will get confused if they see an extra suite in the reporter's output that they have not defined. Also now teenytest does not ouput a test name composed with the file name, for the test in bar.test.js file it would output ok 3 - test3 - ....

searls commented 8 years ago

@flore77 big news—I've been busy making teenytest more extensible, in part inspired by this discussion. I just released teenytest@5.0.0 which has a plugin architecture that may allow for a relatively painless implementation of a js-reporters compliant reporter plugin to be created (whether it's published separately or shipped as part of the main repo)

flore77 commented 8 years ago

Great! Indeed, you have done a lot of work. I will look over and try to write a reporter plugin and then I think we can discuss more.

flore77 commented 8 years ago

@searls it is not quite I was expecting. I was thinking to implement the js-reporter spec native into teenytest. So that teenytest could be connected to any reporter that is respecting the standard.

The goal is that for example Karma that currently needs a reporter for any testing framework, will only need only one reporter to collect the data from any testing framework if all frameworks would respect the standard.

That's why I am thinking it would be good for teenytest to have it built in, it would work out of the box with high-level consumers like Karma, BrowserStack, SauceLabs etc.

What do you think about it?

jzaefferer commented 8 years ago

Without having reviewed the plugin architecture, so maybe I'm missing something important there: To expand on what @flore77 said, we were hoping that the plugin interface of teenytest would use the event emitter specced in js-reporters, using the same events and properties. That way js-reporters compatible reporters would work immediately, instead of having to implement teenytest specific plugins for each reporter format.

searls commented 8 years ago

@jzaefferer understood, but the plugin system I needed required far more granularity (and an ability to mutate outcomes) that the js-reporter spec itself wouldn't allow. Since js-reporter is, I think, a pretty easy-to-circumscribe subset of what this plugin design will allow, whether the plugin is shipped internally and enabled by default or shipped externally and a prerequisite for other js-reporter pluggability would, I think, be a social decision we could make.

searls commented 8 years ago

If you guys aren't feeling sure about an approach here, I'm happy to take a stab as a POC.

In order to proceed, though, I need some idea on how a js-reporters compliant test runner would be configured?

Does someone register to an EventEmitter? If so, we could use a teenytest configurator and teenytest.jsReporters.addListener(myThing) could register your thing

Want to check on that before I proceed because it feels like more often than not the communication will need to be interprocess and I'm not sure how to deal with that

On Jun 24, 2016, at 19:16, Jörn Zaefferer notifications@github.com wrote:

Without having reviewed the plugin architecture, so maybe I'm missing something important there: To expand on what @flore77 said, we were hoping that the plugin interface of teenytest would use the event emitter specced in js-reporters, using the same events and properties. That way js-reporters compatible reporters would work immediately, instead of having to implement teenytest specific plugins for each reporter format.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

jzaefferer commented 8 years ago

Does someone register to an EventEmitter? If so, we could use a teenytest configurator and teenytest.jsReporters.addListener(myThing) could register your thing

That looks pretty reasonable. We're currently working on integrating js-reporters into browserstack-runner, and should have a reference implementation there soon. Once that is in place, we could extend that to also support teenytest (once it has native support for js-reporters). That would be an even better reference implementation.

Want to check on that before I proceed because it feels like more often than not the communication will need to be interprocess and I'm not sure how to deal with that

The main reason we held back on responding here was this comment. Both @flore77 and me weren't clear what the usecase would be. Do you have an example that could clarify the issue?

flore77 commented 8 years ago

@searls a little update, here is the integration into browserstack-runner. If teenytest would come with the js-reporter spec natively it would work directly with the new reporter as also it would skip the registering part, because there is no need for an adapter.

searls commented 8 years ago

Very cool. Implementing js-reporter as an EventEmitter using the reporters plugin interface is still on my list