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

Allow chained comparisons #21

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Change log

## [v0.7.0]

### Added
* Change ComparisonMatcher to allow using `at_least` and `at_most` together

## [v0.6.0] - 2020-03-09

### Added
Expand Down
68 changes: 42 additions & 26 deletions lib/rspec/benchmark/comparison_matcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,17 @@ module ComparisonMatcher
#
# @api private
class Matcher
DEFAULT_EXPECTED_COUNT_MATCHERS = { at_least: 1 }.freeze

def initialize(expected, comparison_type, **options)
check_comparison(comparison_type)
@expected = expected
@comparison_type = comparison_type
@count = 1
@count_type = :at_least
@time = options.fetch(:time) { 0.2 }
@warmup = options.fetch(:warmup) { 0.1 }
@bench = ::Benchmark::Perf::Iteration
@expected_count_matchers = {}
end

# Indicates this matcher matches against a block
Expand Down Expand Up @@ -45,14 +47,7 @@ def matches?(block)

@ratio = @actual_ips / @expected_ips.to_f

case @count_type
when :at_most
at_most_comparison
when :exactly
exact_comparison
else
default_comparison
end
expected_count_matchers.all? { |type, count| matches_comparison?(type, count) }

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [89/80]

end

# The time before measurements are taken
Expand Down Expand Up @@ -139,12 +134,11 @@ def failure_message_when_negated

# @api private
def description
if @count == 1
"perform #{@comparison_type} than comparison block"
else
"perform #{@comparison_type} than comparison block " \
"by #{@count_type} #{@count} times"
end
main_description = "perform #{@comparison_type} than comparison block"
description_extra = comparison_description
return main_description if !description_extra || description_extra.empty?

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [83/80]


[main_description, comparison_description].compact.join(' ')

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

end

# @api private
Expand All @@ -162,6 +156,13 @@ def failure_reason

private

def comparison_description
expected_count_matchers

Choose a reason for hiding this comment

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

Style/InverseMethods: Use reject instead of inverting select.

.select { |_, count| count != 1 }

Choose a reason for hiding this comment

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

Layout/DotPosition: Place the . on the previous line, together with the method call receiver.

.map { |type, count| "by #{type} #{count} times" }

Choose a reason for hiding this comment

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

Layout/DotPosition: Place the . on the previous line, together with the method call receiver.

.join(' and ')

Choose a reason for hiding this comment

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

Layout/DotPosition: Place the . on the previous line, together with the method call receiver.
Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

end

def convert_count(n)
case n
when Numeric then n
Expand All @@ -175,8 +176,23 @@ def convert_count(n)
end

def set_expected_times_count(type, n)
@count_type = type
@count = convert_count(n)
@expected_count_matchers ||= {}
@expected_count_matchers[type] = convert_count(n)
end

def expected_count_matchers
@expected_count_matchers.empty? ? DEFAULT_EXPECTED_COUNT_MATCHERS : @expected_count_matchers

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [102/80]

end

def matches_comparison?(count_type, count)
case count_type
when :at_most
at_most_comparison(count)
when :exactly
exact_comparison(count)
else
default_comparison(count)
end
end

# At most comparison
Expand All @@ -201,11 +217,11 @@ def set_expected_times_count(type, n)
# @return [Boolean]
#
# @api private
def at_most_comparison
def at_most_comparison(count)
if @comparison_type == :faster
1 < @ratio && @ratio < @count
1 < @ratio && @ratio < count

Choose a reason for hiding this comment

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

Style/YodaCondition: Reverse the order of the operands 1 < @ratio.

else
@count**-1 < @ratio && @ratio < 1
count**-1 < @ratio && @ratio < 1
end
end

Expand All @@ -224,11 +240,11 @@ def at_most_comparison
# @return [Boolean]
#
# @api private
def exact_comparison
def exact_comparison(count)
if @comparison_type == :faster
@count == @ratio.round
count == @ratio.round
else
@count == (1.0 / @ratio).round
count == (1.0 / @ratio).round
end
end

Expand All @@ -254,11 +270,11 @@ def exact_comparison
# @return [Boolean]
#
# @api private
def default_comparison
def default_comparison(count)
if @comparison_type == :faster
@ratio > @count
@ratio > count
else
@ratio < (1.0 / @count)
@ratio < (1.0 / count)
end
end

Expand Down
22 changes: 22 additions & 0 deletions spec/unit/comparison_matcher_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,28 @@ def clamp_fast(num, min, max)
}.to raise_error(/expected given block to perform faster than comparison block by at_most \d+ times, but performed faster by \d+.\d+ times/)
end
end

context 'with both at_least and at_most count' do
it "passes if the block performance falls in to the range" do
expect { 1 << 1 }.to perform_faster_than { 'x' * 10 * 1024 }.at_least(2).at_most(125)
end

it "fails if the block performance ratio is higher then indicated by at_least" do
expect {
expect { 1 << 1 }
.to perform_faster_than { 'x' * 10 * 1024 }
.at_least(2).at_most(15).times

Choose a reason for hiding this comment

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

Layout/DotPosition: Place the . on the previous line, together with the method call receiver.

}.to raise_error(/expected given block to perform faster than comparison block by at_least \d+ times and by at_most \d+ times, but performed faster by \d+.\d+ times/)

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [174/80]

end

it "fails if the block performance ratio is lower then indicated by at_most" do

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [85/80]

expect {

Choose a reason for hiding this comment

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

Style/BlockDelimiters: Avoid using {...} for multi-line blocks.

expect {

Choose a reason for hiding this comment

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

Style/BlockDelimiters: Avoid using {...} for multi-line blocks.

1 << 1
}.to perform_faster_than { 'x' * 10 * 1024 }.at_least(200).at_most(300).times

Choose a reason for hiding this comment

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

Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
Metrics/LineLength: Line is too long. [87/80]

}.to raise_error(/expected given block to perform faster than comparison block by at_least \d+ times and by at_most \d+ times, but performed faster by \d+.\d+ times/)

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [174/80]

end
end
end

context "expect { ... }.not_to perform_faster_than(...)" do
Expand Down
4 changes: 2 additions & 2 deletions spec/unit/perform_under_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
}.to raise_error(/Repeat value: 0 needs to be greater than 0/)
end

context "expect { ... }.to perfom_under(...).sample" do
context "expect { ... }.to perform_under(...).sample" do
it "passes if the block performs under threshold" do
expect {
'x' * 1024 * 10
Expand All @@ -46,7 +46,7 @@
}.to_not perform_under(0.001).sample(2)
end

it "fails if the block perfoms under threshold" do
it "fails if the block performs under threshold" do
expect {
expect {
'x' * 1024 * 1024 * 10
Expand Down