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

custom formatter #309

Closed
wants to merge 5 commits into from
Closed

custom formatter #309

wants to merge 5 commits into from

Conversation

samccone
Copy link
Member

Here we are, custom formatters! rebased on top of master, tests passing and docs. Lets ship this sucker!

Via #90
Via #188
Via #215
Via #257

@samccone
Copy link
Member Author

ping @jbpros

@stjohnjohnson
Copy link

Please yes! :shipit: 🐑 ☕
nod

@afternoon
Copy link

👍

@stjohnjohnson
Copy link

So, who are we waiting for?

@petey
Copy link

petey commented Mar 3, 2015

Yes! :shipit:

mal thumb

@samccone
Copy link
Member Author

samccone commented Mar 3, 2015

Just waiting on some eyes from @jbpros

@mattwynne
Copy link
Member

My one note of caution with this is the experience I've had with having 3rd party code depending on the Ruby Cucumber's original formatter API, which was very, very complex.

I can't see from this PR what the formatter API is, but just bear in mind that once it's out in the wild you'll find it much harder to change it. So make sure you're happy with it now.

@samccone
Copy link
Member Author

samccone commented Mar 3, 2015

heh yeah it is not exactly "simple" to use right now... but..... we can follow semver when we change it so it should not be such a big issue.

@jbpros
Copy link
Member

jbpros commented Mar 4, 2015

This is great stuff! Thanks @samccone.

Also, thanks @mattwynne for the note of caution, it makes sense.

3rd-parties should rely on the listener API, based on events, that allows for pretty good decoupling between the internals of Cucumber and formatters/listeners.

I think this PR lacks two things:

  1. Documentation on how to write good plugins (i.e. do not rely on internals, use events!). Maybe an example?
  2. Some gherkin scenarios testing this e2e.

@mattwynne
Copy link
Member

Could 1&2 not be combined @jbpros? i.e. use the Gherkin scenario as the documentation?

Crazy talk, I know...

@stjohnjohnson
Copy link

@samccone Can I help with the docs or gherkin?

@samccone samccone mentioned this pull request May 13, 2015
@jbpros jbpros modified the milestone: major features Jun 8, 2015
@jbpros jbpros modified the milestones: major features, 1.0 release Oct 10, 2015
@charlierudolph
Copy link
Member

@samccone this looks pretty good as is but is still missing documentation and feature tests. I can help with those if you are okay with me pushing to your branch.

@@ -35,30 +35,25 @@ describe("Cucumber.Cli", function () {
describe("when the help is requested", function () {
beforeEach(function () {
configuration.isHelpRequested.andReturn(true);
cli.run(callback);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revert the changes to this file as they are unrelated

@jbpros
Copy link
Member

jbpros commented Oct 16, 2015

I second @charlierudolph on scenarios and docs for this.

@afternoon
Copy link

@charlierudolph @jbpros I added documentation to --help and to README.md here: 053ddbc

Do we need to expand these, or add other documentation elsewhere?

@charlierudolph
Copy link
Member

The documentation I'm talking about is how to build a formatter.

  • what options are passed to the constructor
  • what methods it can or should implement and the arguments those methods are passed

@afternoon
Copy link

Sounds so obvious when you say it. 😞

Is there a single point of reference for options, or do we need to infer them from the code, e.g. formatter.js, summary_formatter.js and the other listeners?

The API seems to be handlers for the set of events emitted during execution, specified in events.js. All of the stock formatters seem to provide methods like handleAfterFeaturesEvent, but I can't see where those are called (after just a bit of shallow investigation).

@charlierudolph
Copy link
Member

The options for the constructor come from here. The coffeescript snippets option needs to go away.

I think we probably want to suggest inheriting from Cucumber.Listener.Formatter. Might be good to just give people a skeleton for a formatter.

function MyFormatter(options) {
  var Cucumber = require('cucumber');

  if (!options)
    options = {};

  var self = Cucumber.Listener.Formatter(options);

  self.handleBeforeFeaturesEvent = function handleBeforeFeaturesEvent(event, callback) {
    var features = event.getPayloadItem('features');
    callback();
  };

  self.handleBeforeFeatureEvent = function handleBeforeFeatureEvent(event, callback) {
    var feature = event.getPayloadItem('feature');
    callback();
  };

  self.handleBeforeScenarioEvent = function handleBeforeScenarioEvent(event, callback) {
    var scenario = event.getPayloadItem('scenario');
    callback();
  };

  self.handleBeforeStepEvent = function handleBeforeStepEvent(event, callback) {
    var stepResult = event.getPayloadItem('step');
    callback();
  };

  self.handleStepResultEvent = function handleStepResultEvent(event, callback) {
    var stepResult = event.getPayloadItem('stepResult');
    callback();
  };

  self.handleAfterStepEvent = function handleAfterStepEvent(event, callback) {
    var step = event.getPayloadItem('step');
    callback();
  };

  self.handleAfterScenarioEvent = function handleAfterScenarioEvent(event, callback) {
    var scenario = event.getPayloadItem('scenario');
    callback();
  };

  self.handleAfterFeatureEvent = function handleAfterFeatureEvent(event, callback) {
    var feature = event.getPayloadItem('feature');
    callback();
  };

  self.handleAfterFeaturesEvent = function handleAfterFeaturesEvent(event, callback) {
    var features = event.getPayloadItem('features');
    self.finish(callback);
  };

  return self;
}

module.exports = MyFormatter;

Right now the objects are based on the internals of cucumber and probably need to be wrapped up somewhere so we can offer a more consistent interface as the internals are changing in a big way when we update to gherkin3. Probably need something similar to the wrapper around a scenario that is passed to hooks, for each of the objects. @jbpros what are your thoughts?

@jbpros
Copy link
Member

jbpros commented Oct 19, 2015

I agree. Event objects shouldn't be exposed to the outside world (the same holds true for event listeners in userland, btw).

I'm thinking we could use the simpler emitter API for listeners:

function MyFormatter(suite) {
  // suite is an emitter instance injected by Cucumber
  suite.on('beforeFeature', function (feature) {
    // do your stuff
  });

  suite.on('beforeScenario', function (scenario) {
  });

  suite.on('afterScenario', function (scenario, result) {
    // receives the hypothetical result object
  });

  suite.on('beforeStep', function (step) {})
  suite.on('step', function (step, result) {})
  suite.on('afterStep', function (step) {})

  // etc.
}

This is only a listener. Shouldn't we use the name "listener" instead of "formatter" in this new API?

Getting back to the "payload" objects, with such an emitter, each event is accompanied with one or more things and doesn't need a getPayloadItem method anymore.

I'm also tempted to rename the events to something like before-scenario or even before scenario.

@mattwynne
Copy link
Member

FWIW, we've got with before/after_test_case for as opposed to before/after_scenario. We wanted to make a clean break from any confusion with the old AST-based formatter API.

@nemanja-tosic
Copy link

Any updates on this?

@a11smiles
Copy link

when will this be working!?!?

@mattwynne
Copy link
Member

👍

@a11smiles
Copy link

@mattwynne any time expectations around this? I need to create TeamCity and AppVeyor formatters for CI.

@mattwynne
Copy link
Member

What are the next steps @jbpros? Do we still need some docs? Maybe you could help with that @a11smiles?

@a11smiles
Copy link

@mattwynne I'm available to do so...indeed, what are the next steps @jbpros ?

@kchokhawala
Copy link

@jbpros: Is there any update on this ? Is this available to use ? I see there have been some conflicts. I would like to use this, if this is good to (or as and when) use, please provide link to some finalized documentation. Thanks.

@charlierudolph
Copy link
Member

@kchokhawala this isn't ready. I'm working on some sizable changes to the formatters. Once those are done we can circle back to custom formatters. I'm going to go ahead and close this due to its age. Issue held in #516

@charlierudolph charlierudolph deleted the sjs/final-custom-output branch July 1, 2016 23:53
@lock
Copy link

lock bot commented Oct 25, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.