Scoping and filtering methods, and more helpful UnexpectedResponse #9

Closed
wants to merge 4 commits into
from
@@ -82,7 +82,7 @@ def check_response_status!(request_type, response, options)
verb, default_status = verb_map[request_type]
expected_status = options[:expected_response_status] || default_status
unless expected_status == response.status
- raise UnexpectedResponse, "#{verb} to #{response.url} responded with " \
+ raise UnexpectedResponse.new(response.status), "#{verb} to #{response.url} responded with " \
@jwilger
jwilger Apr 23, 2012 Owner

I was under the impression that in raise Exception, message the "message" part was simply passed into Exception.new, so not sure what is happening to message on lines where this is raised now. Maybe I'm wrong?

@ravigadad
ravigadad Apr 23, 2012 Contributor

Yeah, this is due to Ruby's wacky implementation of Kernel.raise. If you give it an exception class, the initializer is called with the message as the first argument, but the message is also stored on the exception. If you have an #initialize for your exception, you may not want the message as a param - in which case you explicitly create the instance of the Exception. The message is still stored on the exception somehow (possibly using a separate setter method), though according to what I read here: http://www.ruby-forum.com/topic/207448, the message and backtrace (optional third param to raise) are tucked away in inaccessible instance variables. When you call #message on the exception, the message is returned even if it wasn't part of the initialization, since #message uses #to_s which uses the internal variables. Magic! And yes, this has been raised as an issue, and debated: http://bugs.ruby-lang.org/issues/5898

@ravigadad
ravigadad Apr 23, 2012 Contributor

(which isn't to say I agree with the issue as it was raised - it just seems to be another instance where the confusing dichotomy of Exception.new and Kernel.raise cause problems).

@jwilger
jwilger Apr 23, 2012 Owner

I wonder if the best thing to do would be to just pass in method, URL and status and let the exception class construct its own message from that? (i.e. raise UnexpectedResponse.new(:post, 'http://foo.com/bar', 404)).

Regards,
John Wilger

ph: 971-678-0999
http://johnwilger.com

@ravigadad
ravigadad Apr 23, 2012 Contributor

That could work - in which case, I think we should use a different class of exception for the detect_server_error! exception. Unless we assume that a detected server error should always show up as a 500. I think UnexpectedResponse should always be expected to indicate the actual response.

@ravigadad
ravigadad Apr 23, 2012 Contributor

At any rate, that sounds like a refactoring - the message is received the same way whether the exception constructor creates it or it's set by the raise call.

+ "#{response.status} status, not #{expected_status} as expected\n\n" \
+ response.body
end
@@ -4,7 +4,13 @@ class UnknownKeyError < ArgumentError; end
# @private
class ConfigurationError < StandardError; end
# @private
- class UnexpectedResponse < RuntimeError; end
+ class UnexpectedResponse < RuntimeError
+ attr_reader :status_code
+
+ def initialize(status_code = nil)
+ @status_code = status_code
+ end
+ end
# @private
class AssertionFailed < RuntimeError; end
# @private
@@ -1,4 +1,5 @@
require 'kookaburra/mental_model'
+require 'capybara/util/timeout'
class Kookaburra
class MentalModel
@@ -7,12 +8,24 @@ class MentalModel
# @see Kookaburra::TestHelpers#match_mental_model_of
# @see Kookaburra::TestHelpers#assert_mental_model_of
class Matcher
- def initialize(mental_model, collection_key)
+ # Creates a new matcher for the given MentalModel on the given
+ # collection_key.
+ #
+ # @param [MentalModel] mental_model The MentalModel instance to
+ # request the expected collection from.
+ # @param collection_key The name of the collection that contains
+ # the expected elements.
+ # @param [Integer] wait_for The number of seconds during which to
+ # continually retry failures, before reporting the failure. Not
+ # used when comparing against an array (see #matches?).
+ # @return [self]
+ def initialize(mental_model, collection_key, wait_for = 2)
@collection_key = collection_key
+ @wait_for = wait_for
mental_model.send(collection_key).tap do |collection|
- @expected = collection
- @unexpected = collection.deleted
+ @expected = collection.dup
+ @unexpected = collection.deleted.dup
end
end
@@ -23,8 +36,14 @@ def initialize(mental_model, collection_key)
# model contains elements { A, B, C }, but you only expect to see element
# A.
#
- # @param [Array] collection_keys The keys used in your mental model to
- # reference the data
+ # @example
+ # matcher.only?(:foo, :bar, :baz)
+ # @example With an array of keys
+ # keys = [:foo, :bar, :baz]
+ # matcher.only?(*keys)
+ #
+ # @param collection_keys The keys used in your mental model to reference
+ # the data - if you have an array, splat it.
# @return [self]
def only(*collection_keys)
keepers = @expected.slice(*collection_keys)
@@ -36,24 +55,98 @@ def only(*collection_keys)
self
end
+ # Specifies that members of the expected collection should be mapped by
+ # the given block before attempting to match.
+ #
+ # Useful if the result represents a modified version of what's on the
+ # mental model.
+ #
+ # @yield [val] map function, run once for each member of the collection
+ # @return [self]
+ def mapped_by(&block)
+ validate_block_arguments 'mapped_by', &block
+ @expected = Hash[@expected.map { |key, val| [key, block.call(val)] }]
+ @unexpected = Hash[@unexpected.map { |key, val| [key, block.call(val)] }]
+ self
+ end
+
+ # Specifies that result should be filtered by a given block.
+ #
+ # Useful if you are looking at a filtered result based on given criteria.
+ # That is, you only expect to see elements for which a given block
+ # returns true.
+ #
+ # @yield [val] function used to select specific results from collection
+ # @return [self]
+ def where(&block)
+ validate_block_arguments 'where', &block
+ valid_keys = @expected.select { |key, val| block.call(val) }.map { |key, val| key }
+ only *valid_keys
+ end
+
# Reads better than {#only} with no args
#
# @return [self]
def expecting_nothing
only
end
+ # Specifies the method to try calling on "actual," which is expected to
+ # return the observed result for comparison against the mental model.
+ #
+ # @param [Symbol] method The method to call on "actual"
+ # @return [self]
+ def using(collection_method)
+ @collection_method = collection_method
+ self
+ end
+
# The result contains everything that was expected to be found and nothing
# that was unexpected.
#
+ # The MentalModel::Matcher can be used with two types of objects.
+ #
+ # 1) If the matcher is called on an Array, a direct comparison will be
+ # done between the Array and the MentalModel collection (after any
+ # specified scoping or mapping methods have) been applied.
+ #
+ # 2) If the matcher is called on an object that responds to the
+ # collection_method (which defaults to the collection_key but can be
+ # overridden with "#using"), the result of actual#collection_method will
+ # be used for the comparison.
+ #
+ # When using method #2, if the match is unsuccessful, failure won't be
+ # reported immediately; instead, the comparison will be retried
+ # continuously for the number of seconds specified by wait_for (see
+ # "#initialize"). This is the recommended usage, and is useful for taking
+ # into account possible rendering delays.
+ #
# (Part of the RSpec protocol for custom matchers.)
#
- # @param [Array] actual This is the data observed that you are attempting
- # to match against the mental model.
+ # @param [Array, #collection_method] actual This is the data observed
+ # (or an object that returns the data observed when called with
+ # collection_method) that you you are attempting to match against the
+ # mental model.
# @return Boolean
def matches?(actual)
- @actual = actual
- expected_items_not_found.empty? && unexpected_items_found.empty?
+ @collection_method ||= @collection_key
+ @proc_for_actual = if actual.respond_to?(@collection_method.to_sym)
+ proc { actual.send(@collection_method.to_sym) }
+ else
+ @wait_for = 0
+ proc { actual }
+ end
+ if @wait_for > 0
+ Capybara.timeout(@wait_for) do
+ begin
+ check_expectations
+ rescue Capybara::TimeoutError
+ false
+ end
+ end
+ else
+ check_expectations
+ end
end
# Message to be printed when observed reality does not conform to
@@ -108,6 +201,11 @@ def unexpected_items_found
difference_between_arrays(unexpected_items, unexpected_items_not_found)
end
+ def check_expectations
+ @actual = @proc_for_actual.call
+ expected_items_not_found.empty? && unexpected_items_found.empty?
+ end
+
# (Swiped from RSpec's array matcher)
# Returns the difference of arrays, accounting for duplicates.
# e.g., difference_between_arrays([1, 2, 3, 3], [1, 2, 3]) # => [3]
@@ -125,6 +223,11 @@ def pp_array(array)
array = array.sort if array.all? { |e| e.respond_to?(:<=>) }
array.inspect
end
+
+ def validate_block_arguments(method, &block)
+ raise "Must supply a block to ##{method}" unless block_given?
+ raise "Block supplied to ##{method} must take one argument (the value)" unless block.arity == 1
+ end
end
end
end
@@ -98,8 +98,40 @@ def k
# Delegates to {#k}
delegate :ui, :to => :k
- # RSpec-style custom matcher that compares a given array with
- # the current state of one named collection in the mental model
+ # RSpec-style custom matcher that compares an observed result with
+ # the current state of one named collection in the mental model.
+ #
+ # This matcher can be used in two different ways.
+ #
+ # 1) If the matcher is called on an Array, a direct comparison will be
+ # done between the Array and the MentalModel collection (after any
+ # specified scoping or mapping methods have) been applied.
+ #
+ # 2) If the matcher is called on an object that responds to the
+ # collection_method (which defaults to the collection_key but can be
+ # overridden with "#using"), the result of actual#collection_method will
+ # be used for the comparison.
+ #
+ # When using method #2, if the match is unsuccessful, failure won't be
+ # reported immediately; instead, the comparison will be retried
+ # continuously for the number of seconds specified by wait_for (see
+ # "#initialize"). This is the recommended usage, and is useful for taking
+ # into account possible rendering delays.
+ #
+ # @example Comparing against an array
+ # mental_model.widgets = { :foo => foo, :bar => bar }
+ # [foo, bar].should match_mental_model_of(:widgets)
+ # @example Comparing against an object that responds to the collection_key
+ # mental_model.widgets = { :foo => foo, :bar => bar }
+ # widget_index.widgets = [foo, bar]
+ # widget_index.should match_mental_model_of(:widgets)
+ # @example Comparing against an object that responds to an alternate key
+ # mental_model.widgets = { :foo => foo, :bar => bar }
+ # widget_index.visible_widgets = [foo, bar]
+ # widget_index.should match_mental_model_of(:widgets).using(:visible_widgets)
+ #
+ # @param [Symbol] collection_key The key of the collection on the
+ # mental model that represents the observed result we want.
#
# @see Kookaburra::MentalModel::Matcher
def match_mental_model_of(collection_key)
@@ -109,9 +141,26 @@ def match_mental_model_of(collection_key)
# Custom assertion for Test::Unit-style tests
# (really, anything that uses #assert(predicate, message = nil))
#
+ # This is essentially a wrapper of match_mental_model_of.
+ #
+ # @param [Symbol] collection_key The key of the collection on the
+ # mental model that represents the observed result we want.
+ # @param [Array, #collection_method] actual This is the data observed
+ # (or an object that returns the data observed when called with
+ # collection_method) that you you are attempting to match against the
+ # mental model.
+ # @param [message] message Message to return in case of failure; if not
+ # specified, will return message describing differences between expected
+ # and actual.
+ # @param [options] options Hash of scoping/filtering blocks to call on
+ # matcher (for limiting collection data to compare against) before
+ # attempting match. See the Matcher for details.
# @see Kookaburra::MentalModel::Matcher
- def assert_mental_model_matches(collection_key, actual, message = nil)
+ def assert_mental_model_matches(collection_key, actual, message = nil, options = {})
matcher = match_mental_model_of(collection_key)
+ options.each_pair do |key, val|
+ matcher = matcher.send(key.to_sym, &val)
+ end
result = matcher.matches?(actual)
return if !!result # don't even bother
@@ -149,7 +149,7 @@ def component_locator
# function returns true
def detect_server_error!
if @server_error_detection.try(:call, browser)
- raise UnexpectedResponse, "Your server error detection function detected a server error. Looks like your applications is busted. :-("
+ raise UnexpectedResponse.new, "Your server error detection function detected a server error. Looks like your application is busted. :-("
@ravigadad
ravigadad Apr 23, 2012 Contributor

Note this change, related to your comment on the call to Kernel.raise in APIDriver. Here I had to pass an instance of the exception, rather than the class - otherwise, the new exception would have the message set as the status_code (as well as the return from #message, even though they're completely unrelated), because the message is sent in as the first argument to initialize, when raise is given a class.

end
end
end
@@ -144,11 +144,19 @@ def parse_json_req_body
content << <<-EOF
<ul>
EOF
+ #
+ # Note that we're simulating a rendering delay for the "name" attribute
+ # of the widgets. They're being placed into a temporary data-attribute
+ # and then, 300ms later (see the setTimeout below), the value in that
+ # attribute is moved to the text content of the element, where it's to
+ # be expected. This is being done so we can test that the matcher won't
+ # fail immediately, and that it will retry for a period of time.
+ #
@@widgets.each do |w|
content << <<-EOF
<li class="widget_summary">
<span class="id">#{w[:id]}</span>
- <span class="name">#{w[:name]}</span>
+ <span class="name" data-name="#{w[:name]}"></span>
<form id="delete_#{w[:id]}" action="/widgets/#{w[:id]}" method="POST">
<button type="submit" value="Delete" />
</form>
@@ -159,6 +167,14 @@ def parse_json_req_body
</ul>
<a href="/widgets/new">New Widget</a>
</div>
+ <script>
+ setTimeout(function(){
+ nameElements = document.querySelectorAll('.widget_summary .name');
+ for (i = 0; i < nameElements.length; i++) {
+ nameElements[i].innerHTML = nameElements[i].getAttribute('data-name');
+ }
+ }, 300);
+ </script>
</body>
</html>
EOF
@@ -336,23 +352,43 @@ def delete_widget(name)
ui.sign_in(:bob)
ui.view_widget_list
- # The following two lines are two different ways to shave the yak, but
- # the second one does more to match against the full state of the mental
- # model, provides better failure messages, and is shorter.
+ # We're going to test presence of the expected elements in two ways here.
+ # They're both similar, but the first technique (match_mental_model_of)
+ # does more to match against the full state of the mental model, provides
+ # better failure messages, and is shorter. Also, the simple method of
+ # testing for equality of arrays will fail if there are any extraneous
+ # widgets in the list, while the MentalModelMatcher technique will take
+ # into account what is expected and what is explicitly not expected (through
+ # deletion); this is important in the case of a multi-client testing server.
+ # Finally, the array equality method will fail if the items are presented in
+ # a different order, which is usually irrelevant; this can be corrected for
+ # by using the unfortunately-named `=~` matcher.
+
+ # First we'll use the MentalModelMatcher; this is important because the
+ # MentalModelMatcher will actually wait until a given timeout (using
+ # Capybara's timeout) before failing, if items are not found. Since
+ # we're simulating a rendering delay in the widget index, a direct
+ # comparison of arrays would outright fail without artificially delaying
+ # within the test (e.g. using `sleep`).
+ ui.widget_list.should match_mental_model_of(:widgets)
+
+ # Now that we know the rendering is done (since match_mental_model_of
+ # waited until it found them), we can compare the arrays.
ui.widget_list.widgets.should == k.get_data(:widgets).values_at(:widget_a, :widget_b)
- ui.widget_list.widgets.should match_mental_model_of(:widgets)
ui.create_new_widget(:widget_c, :name => 'Bar')
- # As above, these are equivalent, but the second line is preferred.
+ # As above, these are mostly equivalent in this simple case, but the
+ # match_mental_model_of technique is preferred.
+ ui.widget_list.should match_mental_model_of(:widgets)
ui.widget_list.widgets.should == k.get_data(:widgets).values_at(:widget_a, :widget_b, :widget_c)
- ui.widget_list.widgets.should match_mental_model_of(:widgets)
ui.delete_widget(:widget_b)
- # As above, these are equivalent, but the second line is preferred.
+ # As above, these are mostly equivalent in this simple case, but the
+ # match_mental_model_of technique is preferred.
+ ui.widget_list.should match_mental_model_of(:widgets)
ui.widget_list.widgets.should == k.get_data(:widgets).values_at(:widget_a, :widget_c)
- ui.widget_list.widgets.should match_mental_model_of(:widgets)
end
end
end
Oops, something went wrong.