Skip to content
This repository was archived by the owner on Nov 30, 2024. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 21 additions & 1 deletion lib/rspec/core/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,27 @@ def pattern= value

# When a block passed to pending fails (as expected), display the failure
# without reporting it as a failure (default: false).
add_setting :show_failures_in_pending_blocks
def show_failures_in_pending_blocks=(value)
RSpec.warn_deprecation(<<-EOS.gsub(/^\s+\|/, ''))
|RSpec.configuration.show_failures_in_pending_blocks is being removed
|with no replacement. Called from #{CallerFilter.first_non_rspec_line}.
EOS

@show_failures_in_pending_blocks = value
end

def show_failures_in_pending_blocks
RSpec.warn_deprecation(<<-EOS.gsub(/^\s+\|/, ''))
|RSpec.configuration.show_failures_in_pending_blocks is being removed
|with no replacement. Called from #{CallerFilter.first_non_rspec_line}.
EOS

@show_failures_in_pending_blocks
end

Copy link
Member

Choose a reason for hiding this comment

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

There's also the predicate form show_failures_in_pending_blocks? to deprecate.

Also, "with no replacement" isn't entirely true -- the replacement is to use a custom formatter that prints the failure details from pending examples. Not sure if that's worth mentioning, though.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, one slight complication with show_failures_in_pending_blocks? is that it's the method called from base text formatter but we don't want it issuing a deprecation. You could probably change it to use RSpec.configuration.instance_variable_get(:@show_failures_in_pending_blocks) which is a hack but bypasses the deprecation warning and I'm OK with since it's a temporary thing just in 2.99.

def show_failures_in_pending_blocks?
!!show_failures_in_pending_blocks
end

# Convert symbols to hashes with the symbol as a key with a value of
# `true` (default: false).
Expand Down
20 changes: 20 additions & 0 deletions lib/rspec/core/example_group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,22 @@ def description
def self.define_example_method(name, extra_options={})
module_eval(<<-END_RUBY, __FILE__, __LINE__)
def #{name}(desc=nil, *args, &block)
if #{name.inspect} == :pending
RSpec.warn_deprecation(<<-EOS.gsub(/^\s+\|/, ''))
|The semantics of `RSpec::Core::ExampleGroup#pending` are changing in RSpec 3.
|In RSpec 2.x, it caused the example to be skipped. In RSpec 3, the example will
|still be run but is expected to fail, and will be marked as a failure (rather
|than as pending) if the example passes, just like how `pending` with a block
|from within an example already works.
|
|To keep the same skip semantics, change `pending` to `skip`. Otherwise, if you
|want the new RSpec 3 behavior, you can safely ignore this warning and continue
|to upgrade to RSpec 3 without addressing it.
|
|Called from \#{CallerFilter.first_non_rspec_line}.
|
EOS
Copy link
Member

Choose a reason for hiding this comment

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

FWIW, I've become a fan of this technique for doc strings:

RSpec.warn_deprecation(<<-EOS.gsub(/^\s+\|/, ''))
  |Some text
  |Some more text
  |blah blah
EOS

I like how it keeps the indentation flow rather than forcing the string to be fully left-aligned.

Not worth holding up this PR for, though; more something to think about in the future. (Or not, if you don't prefer it...I hold this preference quite loosely).

end
options = build_metadata_hash_from(args)
options.update(:pending => RSpec::Core::Pending::NOT_YET_IMPLEMENTED) unless block
options.update(#{extra_options.inspect})
Expand Down Expand Up @@ -108,6 +124,10 @@ def #{name}(desc=nil, *args, &block)
# Shortcut to define an example with :pending => true
# @see example
define_example_method :pending, :pending => true
# Shortcut to define an example with :pending => true
# Backported from RSpec 3 to aid migration.
# @see example
define_example_method :skip, :pending => true
# Shortcut to define an example with :pending => 'Temporarily disabled with xexample'
# @see example
define_example_method :xexample, :pending => 'Temporarily disabled with xexample'
Expand Down
4 changes: 3 additions & 1 deletion lib/rspec/core/formatters/base_text_formatter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,10 @@ def dump_pending
output.puts pending_color(" #{pending_example.full_description}")
output.puts detail_color(" # #{pending_example.execution_result[:pending_message]}")
output.puts detail_color(" # #{format_caller(pending_example.location)}")
# instance_variable_get is a hack to avoid a deprecation warning,
# it's only for 2.99.
if pending_example.execution_result[:exception] \
&& RSpec.configuration.show_failures_in_pending_blocks?
&& RSpec.configuration.instance_variable_get(:@show_failures_in_pending_blocks)
dump_failure_info(pending_example)
dump_backtrace(pending_example)
end
Expand Down
30 changes: 29 additions & 1 deletion lib/rspec/core/pending.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,32 @@ class PendingExampleFixedError < StandardError; end
# it "does something", :pending => "something else getting finished" do
# # ...
# end
def pending(*args)
def pending(*args, &block)
RSpec.warn_deprecation(<<-EOS.gsub(/^\s+\|/, ''))
|The semantics of `RSpec::Core::Pending#pending` are changing in
|RSpec 3. In RSpec 2.x, it caused the example to be skipped. In
|RSpec 3, the rest of the example will still be run but is expected
|to fail, and will be marked as a failure (rather than as pending)
|if the example passes.
|
|Any passed block will no longer be executed. This feature is being
|removed since it was semantically inconsistent, and the behaviour it
|offered is being made available with the other ways of marking an
|example pending.
Copy link
Member

Choose a reason for hiding this comment

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

👍 This wording is very clear and well done. Nice work!

|
|To keep the same skip semantics, change `pending` to `skip`.
|Otherwise, if you want the new RSpec 3 behavior, you can safely
|ignore this warning and continue to upgrade to RSpec 3 without
|addressing it.
|
|Called from #{CallerFilter.first_non_rspec_line}.
|
EOS

pending_no_warning(*args, &block)
end

def pending_no_warning(*args)
return self.class.before(:each) { pending(*args) } unless RSpec.current_example

options = args.last.is_a?(Hash) ? args.pop : {}
Expand Down Expand Up @@ -103,6 +128,9 @@ def pending(*args)
end
raise PendingDeclaredInExample.new(message)
end

# Backport from RSpec 3 to aid in upgrading.
alias_method :skip, :pending_no_warning
end

# Alias the error for compatibility with extension gems (e.g. formatters)
Expand Down
23 changes: 23 additions & 0 deletions spec/rspec/core/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1859,5 +1859,28 @@ def metadata_hash(*args)
end
end

describe '#show_failures_in_pending_blocks' do
specify 'reader is deprecated' do
expect_warn_deprecation_with_call_site(__FILE__, __LINE__ + 3,
/show_failures_in_pending_blocks/)

config.show_failures_in_pending_blocks
end

specify 'predicate is deprecated' do
expect_warn_deprecation_with_call_site(__FILE__, __LINE__ + 3,
/show_failures_in_pending_blocks/)

config.show_failures_in_pending_blocks?
end

specify 'writer is deprecated' do
expect_warn_deprecation_with_call_site(__FILE__, __LINE__ + 3,
/show_failures_in_pending_blocks/)

config.show_failures_in_pending_blocks = true
end
end

end
end
50 changes: 36 additions & 14 deletions spec/rspec/core/example_group_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -693,30 +693,52 @@ def define_and_run_group(define_outer_example = false)
end
end

%w[pending xit xspecify xexample].each do |method_name|
describe "::#{method_name}" do
before do
@group = ExampleGroup.describe
@group.send(method_name, "is pending") { }
end
%w[pending skip].each do |method_name|
describe ".#{method_name}" do
let(:group) { ExampleGroup.describe.tap {|x|
x.send(method_name, "is pending") { }
}}

it "generates a pending example" do
@group.run
expect(@group.examples.first).to be_pending
group.run
expect(group.examples.first).to be_pending
end

it "sets the pending message" do
group.run
expect(group.examples.first.metadata[:execution_result][:pending_message]).to eq(RSpec::Core::Pending::NO_REASON_GIVEN)
end
end
end

it "sets the pending message", :if => method_name == 'pending' do
@group.run
expect(@group.examples.first.metadata[:execution_result][:pending_message]).to eq(RSpec::Core::Pending::NO_REASON_GIVEN)
%w[xit xspecify xexample].each do |method_name|
describe ".#{method_name}" do
let(:group) { ExampleGroup.describe.tap {|x|
x.send(method_name, "is pending") { }
}}

it "generates a pending example" do
group.run
expect(group.examples.first).to be_pending
end

it "sets the pending message", :unless => method_name == 'pending' do
@group.run
expect(@group.examples.first.metadata[:execution_result][:pending_message]).to eq("Temporarily disabled with #{method_name}")
it "sets the pending message" do
group.run
expect(group.examples.first.metadata[:execution_result][:pending_message]).to eq("Temporarily disabled with #{method_name}")
end
end
end

describe '::pending' do
Copy link
Member

Choose a reason for hiding this comment

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

I've never really liked using :: for message sends (or in doc strings to indicate a method). I've never asked David about it but I think he liked it quite a bit since there are a fair number of places in rspec that use it. I always prefer a . for a message send though; to me, :: is a constant scoping operator and I don't see a benefit for using it for something else even though ruby allows it.

Are there situations where you prefer it?

(Don't bother changing it in this PR -- since it's in 2.99 and not master it's not sticking around and I don't really care. I'm just bringing this up because I'm curious how others on the team feel about it).

Copy link
Member Author

Choose a reason for hiding this comment

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

I prefer ., I used :: here to be consistent with the other specs around it.

it 'shows upgrade warning' do
expect_warn_deprecation_with_call_site(
"example_group_spec.rb", __LINE__ + 3
)
group = ExampleGroup.describe
group.send(:pending, "is pending") { }
end
end

describe "adding examples" do

it "allows adding an example using 'it'" do
Expand Down
2 changes: 1 addition & 1 deletion spec/rspec/core/formatters/base_text_formatter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ def run_all_and_dump_pending
end

context "with show_failures_in_pending_blocks setting enabled" do
before { RSpec.configuration.stub(:show_failures_in_pending_blocks?) { true } }
before { RSpec.configuration.instance_variable_set(:@show_failures_in_pending_blocks, true) }

it "preserves formatting" do
group.example("example name") { pending { expect("this").to eq("that") } }
Expand Down
5 changes: 4 additions & 1 deletion spec/rspec/core/formatters/html_formatted.html
Original file line number Diff line number Diff line change
Expand Up @@ -392,4 +392,7 @@ <h1>RSpec Code Examples</h1>
</div>
</div>
</body>
</html>
</html><html><p>

2 deprecation warnings total
</p></html>
5 changes: 4 additions & 1 deletion spec/rspec/core/formatters/text_mate_formatted.html
Original file line number Diff line number Diff line change
Expand Up @@ -392,4 +392,7 @@ <h1>RSpec Code Examples</h1>
</div>
</div>
</body>
</html>
</html><html><p>

2 deprecation warnings total
</p></html>
26 changes: 26 additions & 0 deletions spec/rspec/core/pending_example_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,19 @@
example.run(group.new, double.as_null_object)
expect(example).to be_pending_with(RSpec::Core::Pending::NO_REASON_GIVEN)
end

it 'shows upgrade warning' do
expect_warn_deprecation_with_call_site(
"pending_example_spec.rb", __LINE__ + 4
)
group = RSpec::Core::ExampleGroup.describe('group') do
it "does something" do
pending
end
end
example = group.examples.first
example.run(group.new, double.as_null_object)
end
end

context "with no docstring" do
Expand Down Expand Up @@ -116,6 +129,19 @@ def run_example(*pending_args, &block)
example
end

it 'shows upgrade warning' do
expect_warn_deprecation_with_call_site(
"pending_example_spec.rb", __LINE__ + 4
)
group = RSpec::Core::ExampleGroup.describe('group') do
it "does something" do
pending {}
end
end
example = group.examples.first
example.run(group.new, double.as_null_object)
end

context "that fails" do
def run_example(*pending_args)
super(*pending_args) { raise ArgumentError.new }
Expand Down