Skip to content
This repository was archived by the owner on Nov 30, 2024. It is now read-only.

Rename minimal formatter to failure list formatter #2624

Merged
merged 1 commit into from
May 16, 2019
Merged

Rename minimal formatter to failure list formatter #2624

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions lib/rspec/core/formatters.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ module RSpec::Core::Formatters
autoload :JsonFormatter, 'rspec/core/formatters/json_formatter'
autoload :BisectDRbFormatter, 'rspec/core/formatters/bisect_drb_formatter'
autoload :ExceptionPresenter, 'rspec/core/formatters/exception_presenter'
autoload :MinimalFormatter, 'rspec/core/formatters/minimal_formatter'
autoload :FailureListFormatter, 'rspec/core/formatters/failure_list_formatter'

# Register the formatter class
# @param formatter_class [Class] formatter class to register
Expand Down Expand Up @@ -213,8 +213,8 @@ def built_in_formatter(key)
JsonFormatter
when 'bisect-drb'
BisectDRbFormatter
when 'm', 'minimal'
MinimalFormatter
when 'f', 'failure_list'
FailureListFormatter
end
end

Expand Down
6 changes: 3 additions & 3 deletions ...spec/core/formatters/minimal_formatter.rb → ...core/formatters/failure_list_formatter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ module RSpec
module Core
module Formatters
# @private
class MinimalFormatter < BaseFormatter
class FailureListFormatter < BaseFormatter
Formatters.register self, :example_failed, :dump_profile, :message

def example_failed(failure)
Expand All @@ -13,8 +13,8 @@ def example_failed(failure)

# Discard profile and messages
#
# These outputs are not really relevant in the context of this minimal
# formatter.
# These outputs are not really relevant in the context of this failure
# list formatter.
def dump_profile(_profile); end
def message(_message); end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/rspec/core/option_parser.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ def parser(options)
' [d]ocumentation (group and example names)',
' [h]tml',
' [j]son',
' [m]inimal (suitable for editors integration)',
' [f]ailure list (suitable for editors integration)',
Copy link
Member

Choose a reason for hiding this comment

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

The full name here (failure list, on this line) needs to match the option in the case statement in formatters.rb where you used failure_list. Instead of having to decide space vs underscore vs dash or whatever, WDYT of just calling the option failures?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a good point from @myronmarston, and failure list will probably trick users who do not understand how the shell process arguments and don't escape the space char.

I'll add that a hint to the user of the format of the output would help him figure-out what it means without needing to try it:

[f]ailures ("file:line:reason", suitable for editors integration)

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry for the mistake.

Good point. Do you think we should then rename FailureListFormatter to FailuresListFormatter?

Copy link
Member

Choose a reason for hiding this comment

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

No. Grammatically thats looks funny. A list of failures is a failure list.

Copy link
Member

Choose a reason for hiding this comment

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

And don't apologise, I'm to blame for rushing the merge 😂

Copy link
Member Author

@benoittgt benoittgt May 16, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Here we go : #2625

I also added the good suggestion of @smortex. Thanks for your feedbacks

' custom formatter class name') do |o|
options[:formatters] ||= []
options[:formatters] << [o]
Expand Down
6 changes: 3 additions & 3 deletions ...core/formatters/minimal_formatter_spec.rb → ...formatters/failure_list_formatter_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
require 'rspec/core/formatters/minimal_formatter'
require 'rspec/core/formatters/failure_list_formatter'

module RSpec::Core::Formatters
RSpec.describe MinimalFormatter do
RSpec.describe FailureListFormatter do
include FormatterSupport

it 'produces the expected full output' do
output = run_example_specs_with_formatter('minimal')
output = run_example_specs_with_formatter('failure_list')
expect(output).to eq(<<-EOS.gsub(/^\s+\|/, ''))
|./spec/rspec/core/resources/formatter_specs.rb:4:is marked as pending but passes
|./spec/rspec/core/resources/formatter_specs.rb:36:fails
Expand Down