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

Plagiarise Cucumber-JS's formatter summary #1030

Closed
mattwynne opened this issue Sep 23, 2016 · 10 comments
Closed

Plagiarise Cucumber-JS's formatter summary #1030

mattwynne opened this issue Sep 23, 2016 · 10 comments
Assignees
Labels
⌛ stale Will soon be closed by stalebot unless there is activity

Comments

@mattwynne
Copy link
Member

Summary

Cucumber-JS recently added a very nice summary output that lists all the issues found during a test run. Most other testing tools (mocha, rspec) do something similar.

I'd like to converge on what Cucumber-JS is doing, and print this summary at the bottom of all of our console formatters.

Expected Behavior

@charlierudolph who wrote this feature into Cucumber-JS tells me that it's not all that well documented in their tests, but there are clues:

Current Behaviour

The formatters are inconsistent at the moment. This is a chance to bring all the console formatters into line.

Possible Solution

Historically, the console formatters used inheritance, using the Cucumber::Formatter::Console module to bring in this kind of functionality. That module got big and unwieldy, and I've recently done some refactoring to use composition instead, extracting out the Cucumber::Formatter::ConsoleCounts and Cucumber::Formatter::ConsoleIssues classes.

A suggested solution would be to rename ConsoleIssues to ConsoleRerunSuggestions or similar (because that's actually what it does) and build a new ConsoleIssues that prints the issues list in the same way as Cucumber-JS does.

Context & Motivation

Although the long-term plan is to build a central library of cross-platform formatters, that's a while off. This should be a relatively easy / quick implementation, and should improve the user experience.

@mattwynne
Copy link
Member Author

@nodo I think this would be more useful than #401 (and probably easier too!)

@nodo
Copy link
Member

nodo commented Dec 7, 2016

@mattwynne Thanks! sounds good I will pick this up

@nodo nodo self-assigned this Dec 7, 2016
@mattwynne
Copy link
Member Author

👍

@brasmusson
Copy link
Contributor

A question here is whether if it really is the job of the formatter to print the summary, snippets etc., not the least when considering it from the perspective of the future cross-platform formatters.

In Cucumber-JVM the summary, snippets etc. is not printed by formatters, but by a separate summary plugin. Currently there are two built in summary plugins, the default summary and the null summary - the latter not printing anything at all (when using the JUnit-integration of Cucumber-JVM you might want to have nothing at all printed to the console - that is accomplished by using the null formatter as console formatter and the null summary as summary plugin).

The differentiating question here is, if someone were to (with the --out option) write the pretty formatter output to a file, what should end up in the file? Only the output related to the "event bus events" the the pretty formatter outputs, or that and the summary, snippets etc.?

The printing of the summary in Cucumber-JVM was actually one of the things a did in the Cucumber-JVM code base, with the strict instruction that it "should not be part of the formatter, but rather be implemented close to where snippets are printed".

@mattwynne
Copy link
Member Author

I've started extracting a couple of things a bit like you describe here @brasmusson, see https://github.com/cucumber/cucumber-ruby/blob/master/lib/cucumber/formatter/console_counts.rb for example.

I'm ambivalent about whether they're part of the formatters or not in the end, but I do think we should break these things up, and use composition rather than inheritance. The Formatter::Console mixin gives me the willies.

@nodo
Copy link
Member

nodo commented Dec 10, 2016

It's a good point @brasmusson. I have just started working on this and I think that separating the logic which prints the summary and the formatter will bring more flexibility and clean code.

@mattwynne if you agree I would try to refactor this as part of this PR. What do you think?

@mattwynne
Copy link
Member Author

@nodo I think that refactoring the console formatter code to make it more modular would be an excellent plan. You might want to do it separately as a "pre-factoring" and then do this work on top of that.

@stale
Copy link

stale bot commented Nov 8, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week if no further activity occurs.

@stale stale bot added the ⌛ stale Will soon be closed by stalebot unless there is activity label Nov 8, 2017
@stale
Copy link

stale bot commented Nov 15, 2017

This issue has been automatically closed because of inactivity. You can support the Cucumber core team on opencollective.

@stale stale bot closed this as completed Nov 15, 2017
@lock
Copy link

lock bot commented Nov 15, 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 Nov 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
⌛ stale Will soon be closed by stalebot unless there is activity
Projects
None yet
Development

No branches or pull requests

3 participants