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

Raise instead of warn on long-deprecated usages #2849

Merged
merged 3 commits into from
Jan 11, 2021
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
4 changes: 4 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ Breaking Changes:
* Change the default `shared_context_metadata_behavior` to `apply_to_host_groups`
and remove the configuration option. (Phil Pirozhkov, #2834)
* Remove `run_all_when_everything_filtered` configuration option. (Phil Pirozhkov, #2845)
* Raise on unsupported hook scope usage. (Phil Pirozhkov, #2849)
* Raise on usage of metadata on suite-level scopes. (Phil Pirozhkov, #2849)
* Raise an error when `fail_fast` is configured with
an unsupported value. (Phil Pirozhkov, #2849)

Enhancements:

Expand Down
8 changes: 2 additions & 6 deletions lib/rspec/core/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -203,11 +203,9 @@ def fail_fast=(value)
@fail_fast = value.to_i

if value.to_i == 0
# TODO: in RSpec 4, consider raising an error here.
RSpec.warning "Cannot set `RSpec.configuration.fail_fast`" \
raise ArgumentError, "Cannot set `RSpec.configuration.fail_fast`" \
" to `#{value.inspect}`. Only `true`, `false`, `nil` and integers" \
" are valid values."
@fail_fast = true
end
end
end
Expand Down Expand Up @@ -1990,9 +1988,7 @@ def handle_suite_hook(scope, meta)
return nil unless scope == :suite

unless meta.empty?
# TODO: in RSpec 4, consider raising an error here.
# We warn only for backwards compatibility.
RSpec.warn_with "WARNING: `:suite` hooks do not support metadata since " \
raise ArgumentError, "WARNING: `:suite` hooks do not support metadata since " \
"they apply to the suite as a whole rather than " \
"any individual example or example group that has metadata. " \
"The metadata you have provided (#{meta.inspect}) will be ignored."
Expand Down
17 changes: 6 additions & 11 deletions lib/rspec/core/hooks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -442,18 +442,13 @@ def register(prepend_or_append, position, *args, &block)
scope, options = scope_and_options_from(*args)

if scope == :suite
# TODO: consider making this an error in RSpec 4. For SemVer reasons,
# we are only warning in RSpec 3.
RSpec.warn_with "WARNING: `#{position}(:suite)` hooks are only supported on " \
"the RSpec configuration object. This " \
"`#{position}(:suite)` hook, registered on an example " \
"group, will be ignored."
return
raise ArgumentError, "`#{position}(:suite)` hooks are only " \
"supported on the RSpec configuration object. This " \
"`#{position}(:suite)` hook, registered on an example " \
"group, will be ignored."
elsif scope == :context && position == :around
# TODO: consider making this an error in RSpec 4. For SemVer reasons,
# we are only warning in RSpec 3.
RSpec.warn_with "WARNING: `around(:context)` hooks are not supported and " \
"behave like `around(:example)."
raise ArgumentError, "`around(:context)` hooks are not supported " \
"and behave like `around(:example)`."
end

hook = HOOK_TYPES[position][scope].new(block, options)
Expand Down
12 changes: 4 additions & 8 deletions spec/rspec/core/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,14 +110,10 @@ module RSpec::Core
allow(RSpec).to receive(:warning)
end

it 'prints warning' do
config.fail_fast = 'yes'
expect(RSpec).to have_received(:warning).with(/Cannot set `RSpec.configuration.fail_fast`/i)
end

it 'is set to true' do
config.fail_fast = 'yes'
expect(config.fail_fast).to eq true
it 'raises an error' do
expect {
config.fail_fast = 'yes'
}.to raise_error(ArgumentError, /Cannot set `RSpec.configuration.fail_fast`/i)
end
end
end
Expand Down
38 changes: 18 additions & 20 deletions spec/rspec/core/hooks_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -484,32 +484,30 @@ def yielder
])
end

it 'emits a warning for `around(:context)`' do
expect(RSpec).to receive(:warn_with).with(a_string_including(
'`around(:context)` hooks are not supported'
))
RSpec.describe do
around(:context) { }
end
it 'raises an error for `around(:context)`' do
expect {
RSpec.describe do
around(:context) { }
end
}.to raise_error(ArgumentError, a_string_including("`around(:context)` hooks are not supported"))
end

it 'emits a warning for `around(:context)` defined in `configure`' do
expect(RSpec).to receive(:warn_with).with(a_string_including(
'`around(:context)` hooks are not supported'
))
RSpec.configure do |c|
c.around(:context) { }
end
it 'raises an error for `around(:context)` defined in `configure`' do
expect {
RSpec.configure do |c|
c.around(:context) { }
end
}.to raise_error(ArgumentError, a_string_including("`around(:context)` hooks are not supported"))
end

[:before, :around, :after].each do |type|
it "emits a warning for `#{type}(:suite)` hooks" do
expect(RSpec).to receive(:warn_with).with(a_string_including(
"`#{type}(:suite)` hooks are only supported on the RSpec configuration object."
))
RSpec.describe do
send(type, :suite) { }
end
expect {
RSpec.describe do
send(type, :suite) { }
end
}.to raise_error(ArgumentError, a_string_including(
"`#{type}(:suite)` hooks are only supported on the RSpec configuration object"))
end
end
end
Expand Down
27 changes: 6 additions & 21 deletions spec/rspec/core/suite_hooks_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,26 +66,22 @@ module RSpec::Core
end

context "registered on an example group" do
it "is ignored with a clear warning" do
sequence = []

it "raises an error with a clear message" do
expect {
RSpec.describe "Group" do
__send__(registration_method, :suite) { sequence << :suite_hook }
example { sequence << :example }
end.run
}.to change { sequence }.to([:example]).
and output(a_string_including("#{type}(:suite)")).to_stderr
__send__(registration_method, :suite) { }
end
}.to raise_error(a_string_including("#{type}(:suite)"))
end
end

context "registered with metadata" do
it "explicitly warns that the metadata is ignored" do
it "raises an error" do
expect {
RSpec.configure do |c|
c.__send__(registration_method, :suite, :some => :metadata)
end
}.to output(a_string_including(":suite", "metadata")).to_stderr
}.to raise_error(ArgumentError, a_string_including(":suite", "metadata"))
end
end
end
Expand All @@ -112,17 +108,6 @@ def define_and_run_example_group(&block)
runner.run err, out
end

it "still runs :suite hooks with metadata even though the metadata is ignored" do
sequence = []
allow(RSpec).to receive(:warn_with)

config.before(:suite, :foo) { sequence << :before_suite }
config.after(:suite, :foo) { sequence << :after_suite }
define_and_run_example_group { sequence << :example_groups }

expect(sequence).to eq([ :before_suite, :example_groups, :after_suite ])
end

it "runs :suite hooks before and after example groups in the correct order" do
sequence = []

Expand Down