Skip to content

Commit

Permalink
Merge pull request #1277 from rspec/use-strict_predicate_matchers-by-…
Browse files Browse the repository at this point in the history
…default

Turn strict_predicate_matchers on by default
  • Loading branch information
pirj committed Jan 8, 2021
2 parents ebab7e2 + ccb246c commit 1515f2b
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 27 deletions.
1 change: 1 addition & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ Breaking Changes:

* Ruby < 2.3 is no longer supported. (Phil Pirozhkov, #1231)
* Remove `should` and `should_not` syntax (including one-liners). (Phil Pirozhkov, #1245)
* Turn `strict_predicate_matchers` on by default. (Phil Pirozhkov, #1277)

Enhancements:

Expand Down
24 changes: 12 additions & 12 deletions features/built_in_matchers/predicates.feature
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Feature: Predicate matchers

In either case, RSpec provides nice, clear error messages, such as:

`expected zero? to be truthy, got false`
`expected zero? to return true, got false`

Calling private methods will also fail:

Expand All @@ -60,7 +60,7 @@ Feature: Predicate matchers
"""
When I run `rspec be_zero_spec.rb`
Then the output should contain "2 examples, 1 failure"
And the output should contain "expected `7.zero?` to be truthy, got false"
And the output should contain "expected `7.zero?` to return true, got false"

Scenario: is_expected.not_to be_empty (based on Array#empty?)
Given a file named "not_to_be_empty_spec.rb" with:
Expand All @@ -75,7 +75,7 @@ Feature: Predicate matchers
"""
When I run `rspec not_to_be_empty_spec.rb`
Then the output should contain "2 examples, 1 failure"
And the output should contain "expected `[].empty?` to be falsey, got true"
And the output should contain "expected `[].empty?` to return false, got true"

Scenario: is_expected.to have_key (based on Hash#has_key?)
Given a file named "have_key_spec.rb" with:
Expand All @@ -88,7 +88,7 @@ Feature: Predicate matchers
"""
When I run `rspec have_key_spec.rb`
Then the output should contain "2 examples, 1 failure"
And the output should contain "expected `{:foo=>7}.has_key?(:bar)` to be truthy, got false"
And the output should contain "expected `{:foo=>7}.has_key?(:bar)` to return true, got false"

Scenario: is_expected.to have_decimals (based on custom #have_decimals? method)
Given a file named "have_decimals_spec.rb" with:
Expand All @@ -114,7 +114,7 @@ Feature: Predicate matchers
"""
When I run `rspec have_decimals_spec.rb`
Then the output should contain "2 examples, 1 failure"
And the output should contain "expected `42.0.has_decimals?` to be truthy, got false"
And the output should contain "expected `42.0.has_decimals?` to return true, got false"

Scenario: matcher arguments are passed on to the predicate method
Given a file named "predicate_matcher_argument_spec.rb" with:
Expand All @@ -136,8 +136,8 @@ Feature: Predicate matchers
"""
When I run `rspec predicate_matcher_argument_spec.rb`
Then the output should contain "4 examples, 2 failures"
And the output should contain "expected `12.multiple_of?(4)` to be falsey, got true"
And the output should contain "expected `12.multiple_of?(5)` to be truthy, got false"
And the output should contain "expected `12.multiple_of?(4)` to return false, got true"
And the output should contain "expected `12.multiple_of?(5)` to return true, got false"

Scenario: the config `strict_predicate_matchers` impacts matching of results other than `true` and `false`
Given a file named "strict_or_not.rb" with:
Expand All @@ -159,14 +159,14 @@ Feature: Predicate matchers
end
end
context 'with non-strict matchers (default)' do
let(:strict) { false }
context 'with strict matchers (default)' do
let(:strict) { true }
# deliberate failure
it { is_expected.to have_strange_result }
end
context 'with strict matchers' do
let(:strict) { true }
# deliberate failure
context 'with non-strict matchers' do
let(:strict) { false }
it { is_expected.to have_strange_result }
end
end
Expand Down
4 changes: 2 additions & 2 deletions features/test_frameworks/minitest.feature
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ Feature: Minitest integration
"""
When I run `ruby rspec_expectations_test.rb`
Then the output should contain "4 runs, 5 assertions, 2 failures, 0 errors"
And the output should contain "expected `[1, 2].empty?` to be truthy, got false"
And the output should contain "expected `[1, 2].empty?` to return true, got false"
And the output should contain "be_an_int is deprecated"
And the output should contain "Got 2 failures from failure aggregation block"

Expand Down Expand Up @@ -105,7 +105,7 @@ Feature: Minitest integration
"""
When I run `ruby rspec_expectations_spec.rb`
Then the output should contain "9 runs, 10 assertions, 5 failures, 0 errors"
And the output should contain "expected `[1, 2].empty?` to be truthy, got false"
And the output should contain "expected `[1, 2].empty?` to return true, got false"
And the output should contain "expected ZeroDivisionError but nothing was raised"
And the output should contain "Got 2 failures from failure aggregation block"
And the output should contain "Expected [1, 2] to be empty?"
8 changes: 4 additions & 4 deletions lib/rspec/expectations/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class Configuration

def initialize
@on_potential_false_positives = :warn
@strict_predicate_matchers = false
@strict_predicate_matchers = true
end

# Configures the maximum character length that RSpec will print while
Expand Down Expand Up @@ -126,9 +126,9 @@ def on_potential_false_positives=(behavior)
@on_potential_false_positives = behavior
end

# Configures RSpec to check predicate matchers to `be(true)` / `be(false)` (strict),
# or `be_truthy` / `be_falsey` (not strict).
# Historically, the default was `false`, but `true` is recommended.
# Configures RSpec to check the values returned by predicate matchers
# to be exactly true/false (strict), or truthy/falsey (not strict).
# Before RSpec 4, the default was `false`.
def strict_predicate_matchers=(flag)
raise ArgumentError, "Pass `true` or `false`" unless flag == true || flag == false
@strict_predicate_matchers = flag
Expand Down
17 changes: 17 additions & 0 deletions spec/rspec/expectations/configuration_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,23 @@ class << rspec_dup; undef configuration; end
end
end

describe "#strict_predicate_matchers?" do
it "is true by default" do
expect(config.strict_predicate_matchers?).to be(true)
end

it "can be set to false" do
config.strict_predicate_matchers = false
expect(config.strict_predicate_matchers?).to be(false)
end

it "can be set back to true" do
config.strict_predicate_matchers = false
config.strict_predicate_matchers = true
expect(config.strict_predicate_matchers?).to be(true)
end
end

describe "#max_formatted_output_length=" do
before do
@orig_max_formatted_output_length = RSpec::Support::ObjectFormatter.default_instance.max_formatted_output_length
Expand Down
10 changes: 2 additions & 8 deletions spec/rspec/matchers/built_in/has_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ def o.has_sym?(sym); sym == :foo; end
expect({ :a => "A" }).not_to have_key(:b)
end

context "when strict_predicate_matchers is set to true" do
context "when strict_predicate_matchers is set to true", with_configuration: {strict_predicate_matchers: true} do
it "fails when #has_sym? returns nil" do
actual = double("actual", :has_foo? => nil)
expect {
Expand All @@ -140,13 +140,7 @@ def o.has_sym?(sym); sym == :foo; end
end
end

context "when strict_predicate_matchers is set to false" do
around do |example|
RSpec::Expectations.configuration.strict_predicate_matchers = false
example.run
RSpec::Expectations.configuration.strict_predicate_matchers = true
end

context "when strict_predicate_matchers is set to false", with_configuration: {strict_predicate_matchers: false} do
it "passes if #has_sym?(*args) returns nil" do
actual = double("actual", :has_foo? => nil)
expect(actual).not_to have_foo
Expand Down
18 changes: 17 additions & 1 deletion spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ def hash_inspect(hash)

config.expect_with :rspec do |expectations|
expectations.include_chain_clauses_in_custom_matcher_descriptions = true
expectations.strict_predicate_matchers = true
end

config.mock_with :rspec do |mocks|
Expand Down Expand Up @@ -70,6 +69,23 @@ def hash_inspect(hash)
end
RSpec.configuration.include_context "with warn_about_potential_false_positives set to false", :warn_about_potential_false_positives

RSpec.shared_context "with modified configuration" do
around do |example|
configuration = example.metadata[:with_configuration]
original = configuration.keys.each.with_object({}) do |key, config|
config[key] = RSpec::Expectations.configuration.public_send(key)
end
configuration.each do |key, value|
RSpec::Expectations.configuration.public_send("#{key}=", value)
end
example.run
original.each do |key, value|
RSpec::Expectations.configuration.public_send("#{key}=", value)
end
end
end
RSpec.configuration.include_context "with modified configuration", :with_configuration

module MinitestIntegration
include ::RSpec::Support::InSubProcess

Expand Down

0 comments on commit 1515f2b

Please sign in to comment.