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

add at_least_until to the maintain expectation #167

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
58 changes: 48 additions & 10 deletions lib/roby/test/execution_expectations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -114,17 +114,36 @@ def not_become_unreachable(*generators, backtrace: caller(1))
#
# @param [Float] at_least_during the minimum duration in seconds. If
# zero, the expectations will run at least one execution cycle. The
# exact duration depends on the other expectations.
# exact duration depends on the other expectations. Cannot be used
# with at_least_until.
# @param [Float] at_least_until the expected generator. The expectations
# will run until the generator is emitted. Cannot be used with
# at_least_during.
# @yieldparam [ExecutionEngine::PropagationInfo]
# all_propagation_info all that happened during the propagations
# since the beginning of expect_execution block. It contains event
# emissions and raised/caught errors.
# @yieldreturn [Boolean] expected to be true over duration seconds
def maintain(
at_least_during: 0, description: nil, backtrace: caller(1), &block
at_least_during: nil,
at_least_until: nil,
description: nil,
backtrace: caller(1),
&block
)
if at_least_during && at_least_until
raise ArgumentError, "at_least_until and at_least_during "\
"are mutually exclusive"
end

at_least_during = 0 unless at_least_during || at_least_until
add_expectation(
Maintain.new(at_least_during, block, description, backtrace)
Maintain.new(
at_least_during || at_least_until,
block,
description,
backtrace
)
)
end

Expand Down Expand Up @@ -1145,30 +1164,49 @@ def to_s
end

class Maintain < Expectation
def initialize(at_least_during, block, description, backtrace)
def initialize(at_least, block, description, backtrace)
Copy link
Member

Choose a reason for hiding this comment

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

You're making the difference between at_least and until in the user API (which is good) and then stop making it in this constructor to finally re-split on type...

Just propagate the same two arguments to the constructor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't want to break the API

Copy link
Member

Choose a reason for hiding this comment

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

Please do, these are meant to be private. The public interface are the expectation API calls.

(and add a # @private documentation line with the Maintain class)

super(backtrace)
@at_least_during = at_least_during

if at_least.kind_of? EventGenerator
@at_least_until = at_least
else
@at_least_during = at_least
@deadline = Time.now + at_least
end

@description = description || @backtrace[0].to_s
@block = block
@deadline = Time.now + at_least_during
@failed = false
end

def emitted?(propagation_info)
return false unless @at_least_until

@emitted_events =
propagation_info
.emitted_events
.find_all { |ev| ev.generator == @at_least_until }
!@emitted_events.empty?
end

def update_match(propagation_info)
if !@block.call(propagation_info)
@failed = true
false
elsif Time.now > @deadline
true
else
emitted?(propagation_info) ||
(@at_least_during && Time.now > @deadline)
Comment on lines +1197 to +1198
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this never match if neither @at_least* are set?

I would check on @deadline here since the condition is using it.

Copy link
Contributor Author

@g-arjones g-arjones May 31, 2020

Choose a reason for hiding this comment

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

Doesn't this never match if neither @at_least* are set?

The user level API enforces one of them to be set (just as it did before this patch)

Copy link
Member

Choose a reason for hiding this comment

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

OK. Please add a comment explaining that, it will make things easier on the understanding.

end
end

def unachievable?(_propagation_info)
@failed
@failed || @at_least_until&.unreachable?
end

def explain_unachievable(_propagation_info)
"#{self} returned false"
return "#{self} returned false" if @failed

@at_least_until&.unreachability_reason
end

def to_s
Expand Down
37 changes: 37 additions & 0 deletions test/test/test_execution_expectations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,43 @@ def return_object

assert_equal "10", expectation.to_s
end
it "validates when the event is emitted" do
plan.add(generator = EventGenerator.new)
expect_execution { generator.emit }
.to { maintain(at_least_until: generator) { true } }
Comment on lines +667 to +669
Copy link
Member

Choose a reason for hiding this comment

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

I would assume the caller would want the predicate to at least match once. Not sure what's your use-case though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Will fix.

end
it "fails if the event is not emitted within the given timeout" do
plan.add(generator = EventGenerator.new)
assert_raises(ExecutionExpectations::Unmet) do
expect_execution
.timeout(0.2)
.to { maintain(at_least_until: generator) { true } }
end
end
it "fails if the block evaluates to false" do
plan.add(generator = EventGenerator.new)
assert_raises(ExecutionExpectations::Unmet) do
expect_execution
.to { maintain(at_least_until: generator) { false } }
end
end
it "fails if at_least_during and at_least_until are used together" do
plan.add(generator = EventGenerator.new)
assert_raises(ArgumentError) do
expect_execution.to do
maintain(at_least_until: generator,
at_least_during: 1) { true }
end
end
end
it "fails if the generator becomes unreachable" do
plan.add(generator = EventGenerator.new)
assert_raises(ExecutionExpectations::Unmet) do
expect_execution { generator.unreachable! }
.timeout(0)
.to { maintain(at_least_until: generator) { true } }
end
end
end

describe "#achieve" do
Expand Down