Add reporting hooks that conform to js-reporters #15

Open
searls opened this Issue Jun 9, 2016 · 12 comments

Comments

Projects
None yet
3 participants
@searls
Member

searls commented Jun 9, 2016

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

This comment has been minimized.

Show comment
Hide comment
@flore77

flore77 Jun 9, 2016

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.

flore77 commented Jun 9, 2016

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

This comment has been minimized.

Show comment
Hide comment
@searls

searls Jun 9, 2016

Member

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

  • A teenytest global suite is, I suppose, defined as the set of all modules that a certain glob matches, less any name or line number filters applied (e.g. teenytest **/*.test.js#foo is all tests named foo in files matching that glob)
  • teenytest is only for Node.js testing, so the concept of global state insofar as it's assumed "because browsers" and script tags and concatenation and so forth may represent an impedance mismatch.

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 () {}
  • global suite
    • foo suite
      • test1 test
      • sub suite
        • test2 test
    • bar suite
      • test3 test

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?

Member

searls commented Jun 9, 2016

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

  • A teenytest global suite is, I suppose, defined as the set of all modules that a certain glob matches, less any name or line number filters applied (e.g. teenytest **/*.test.js#foo is all tests named foo in files matching that glob)
  • teenytest is only for Node.js testing, so the concept of global state insofar as it's assumed "because browsers" and script tags and concatenation and so forth may represent an impedance mismatch.

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 () {}
  • global suite
    • foo suite
      • test1 test
      • sub suite
        • test2 test
    • bar suite
      • test3 test

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

This comment has been minimized.

Show comment
Hide comment
@flore77

flore77 Jun 10, 2016

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 😄

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:

  • global suite
    • test1 test
    • sub suite
      • test2 test
    • test3 test

this would be done in js-reporters hooks:

  • emit(runStart, global suite)
  • emit(testStart, test1)
  • emit(testEnd, test1)
  • emit(suiteStart, sub suite)
  • emit(testStart, test2)
  • emit(testEnd, test2)
  • emit(suiteEnd, sub suite)
  • emit(testStart, test3)
  • emit(testEnd, test3)
  • emit(runEnd, global suite)

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 - ....

flore77 commented Jun 10, 2016

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 😄

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:

  • global suite
    • test1 test
    • sub suite
      • test2 test
    • test3 test

this would be done in js-reporters hooks:

  • emit(runStart, global suite)
  • emit(testStart, test1)
  • emit(testEnd, test1)
  • emit(suiteStart, sub suite)
  • emit(testStart, test2)
  • emit(testEnd, test2)
  • emit(suiteEnd, sub suite)
  • emit(testStart, test3)
  • emit(testEnd, test3)
  • emit(runEnd, global suite)

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 searls referenced this issue Jun 19, 2016

Merged

Plugins #17

10 of 10 tasks complete
@searls

This comment has been minimized.

Show comment
Hide comment
@searls

searls Jun 24, 2016

Member

@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)

Member

searls commented Jun 24, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@flore77

flore77 Jun 24, 2016

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 Jun 24, 2016

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

This comment has been minimized.

Show comment
Hide comment
@flore77

flore77 Jun 24, 2016

@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?

flore77 commented Jun 24, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@jzaefferer

jzaefferer Jun 24, 2016

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.

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

This comment has been minimized.

Show comment
Hide comment
@searls

searls Jun 24, 2016

Member

@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.

Member

searls commented Jun 24, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@searls

searls Jun 25, 2016

Member

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.

Member

searls commented Jun 25, 2016

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

This comment has been minimized.

Show comment
Hide comment
@jzaefferer

jzaefferer Jul 4, 2016

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?

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

This comment has been minimized.

Show comment
Hide comment
@flore77

flore77 Jul 18, 2016

@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.

flore77 commented Jul 18, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@searls

searls Jul 18, 2016

Member

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

Member

searls commented Jul 18, 2016

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment