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

Summary formatter #999

Merged
merged 4 commits into from
Aug 5, 2016
Merged

Summary formatter #999

merged 4 commits into from
Aug 5, 2016

Conversation

mattwynne
Copy link
Member

@mattwynne mattwynne commented Jul 20, 2016

Summary

Introduce a new formatter that provides a summary of scenarios being run

Details, motivation, context

This is a nice simple example of how to write a formatter with the new API. It's helped me drive out or extract some cleaner replacement code for common stuff like the end-of-run stats, and should also prove more reliable than the pretty formatter when run in random mode, because of its simplicity.

I have an idea that this could be made more awesome, with more console-based fanciness to show the details of each step for the currently-running scenario, then collapse back into the scenario title / result once it moved on to the next one. Anyway that can come in another ticket.

How Has This Been Tested?

New feature in docs/formatter. A couple of unit tests for some of the extracted pieces.

Screenshots

screen shot 2016-07-20 at 23 41 27
:

Types of changes

  • New feature
  • Breaking change to API

Checklist:

  • I've added tests for my code

This formatter provides a high-level summary of the scenarios that were
run. It's also A LOT simpler than the pretty formatter, and so more
reliable for testing with.

I'm thinking we can use this to build out alternative re-usable pieces
(like the summary) that need to be re-written anyway.

Work still to do:

- unit tests
- acceptance test for scenario outlines
- finish the summary
- colours?
- some animation as steps run?
Still to do:

- unit tests
- useful output when tests fail
@mattwynne
Copy link
Member Author

Note that I originally called this the 'spec' formatter (in homage to RSpec and Mocha's spec formatters) but I went with summary instead in the end - spec seems a bit overloaded in our world. WDYT?

Failing ✗

2 scenarios (1 passed, 1 failed)
2 steps (1 passed, 1 failed)
Copy link
Member

Choose a reason for hiding this comment

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

Why did you choose to not print the actual error? Are you assuming this is used in conjunction with other formatters?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ooops the push was out of date. Try now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Now that we have Formatter::ConsoleIssues extracted, I'm hoping we can make it print the same output as you're doing for Cucumber-JS @charlierudolph. Where's the best place to get a reference for that behaviour?

@mattwynne
Copy link
Member Author

Note that I also think this might be a useful tool to help finish off #992 since the output is simple.

@@ -80,51 +78,23 @@ def print_element_messages(element_messages, status, kind)
end
end

def print_stats(features, options)
Copy link
Contributor

Choose a reason for hiding this comment

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

The public methods of the Console module are part of the public API (see #893), so this PR introduces breaking changes (which is OK since we are aiming at 3.0.0 - but the PR is miss labeled).

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair point Bjorn. I'd like to get rid of that module altogether really, or at least slim down its interface a lot. Using composition (e.g. ConsoleIssues) is the way forward.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK I've updated the labelling on the PR.

I regret that I didn't tease apart the refactoring from the development of the new formatter, but it was late. I guess I still could if you think that would make it easier to review / merge?

Choose a reason for hiding this comment

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

I would have been nice for this change to be documented in the changelog 🍻

@mattwynne
Copy link
Member Author

I believe this is ready to merge @cucumber/cucumber-ruby

@mattwynne
Copy link
Member Author

OK, no objections so I'm going to merge this.

@lock
Copy link

lock bot commented Oct 24, 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 24, 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.

4 participants