appfolio / ae_page_objects

Page Objects for Capybara
MIT License
28 stars 9 forks source link

Dsl#element should change its block behavior #112

Closed dvicory closed 9 years ago

dvicory commented 9 years ago
def element(name, options = {}, &block)
  ...
  define_method name do |&block|
    ElementProxy.new(klass, self, options, &block)
  end
  ...
end

Dsl#element defines a method that gives you an ElementProxy. The way this is written is misleading as to its behavior. If you pass a block to the defined method, it is never called or used. For instance, the below code will not throw:

element :some_element do
  element :nested
end

some_element do { throw Exception }

My proposal is that we make this less misleading as we have seen an instance of passing a block thinking that assertions within the block would be ran and the parameter passed would be the page object:

element :page_object do
  element :other_element
  def is_awesome?
    false
  end
end

some.really.nested.page_object do |page_object|
  assert page_object.other_element.present?
  assert page_object.is_awesome?
end

The block will never be called and the test will silently pass.

There are two thoughts to this fixing this: either define_method creates a method that does not take a parameter which will hopefully not give the impression that giving a block will do something useful, or allow passing a block and call that block with an instance of that ElementProxy.

The latter option might look something like this:

define_method name do |&block|
  elem = ElementProxy.new(klass, self, options, &block)
  if block
    block.call(elem)
  else
    elem
  end
end
dtognazzini commented 9 years ago

The block should handling should be removed.