Name collision - how would you handle this?

crucoba · August 7, 2010, 11:11pm

Hey all,

It turns out that if you have

Rails (2 or 3)
Ruby-1.9
a model named Message
let(:message) or def message in an example group
a Rails assertion in an example in that group
- note that rspec-rails’ matchers delegate to Rails’ assertions

You’ll get an error saying “wrong number of arguments (1 for 0)”

This is because the rails assertion, which, when running with Ruby-1.9,
delegates to Minitest::Assertions#assert_block, which delegates to a
message() method that it defines. So the message() method defined by
let() overrides the message() method in the Assertions module, and
results in unexpected and undesirable outcomes.

So - what should we do? I don’t think changing Minitest is really an
option, as too many assertion libraries already wrap Minitest
assertions. I don’t think RSpec should be in the business of monitoring
methods end-users define to make sure they’re not overriding
pre-existing methods (what if you override a method intentionally?). The
only thing I’m left with is document this particular case and hope for
the best, but that feels unsatisfactory as well.

Recommendations? Words of wisdom?

Cheers,
David

crucoba · August 7, 2010, 11:38pm

On 7 Aug 2010, at 22:10, David C. wrote:

So - what should we do? I don’t think changing Minitest is really an option, as too many assertion libraries already wrap Minitest assertions. I don’t think RSpec should be in the business of monitoring methods end-users define to make sure they’re not overriding pre-existing methods (what if you override a method intentionally?). The only thing I’m left with is document this particular case and hope for the best, but that feels unsatisfactory as well.

While I fully agree if you def a method that already exists, you
should be expected to deal with it yourself (that’s just the way things
are in Ruby), does the same apply to let? I can actually see an
argument that you should only be able to let a method that doesn’t
already exist, and also only do it once (which is just a consequence of
not being able to override a method, given the current implementation).

Can you think of any downsides of preventing RSpec users from overriding
existing methods with let? Are there any popular names already taken?
Or other problems?

To me, let is magic. I don’t think of it, first and foremost, of
defining a method. I see the things it creates as more like local
variables, and just remind myself that they’re methods if I wonder why
it works.

I’m not sold either way on this, but I think it’s one worth a debate.

Ash

–
http://www.patchspace.co.uk/
http://www.linkedin.com/in/ashleymoran

crucoba · August 8, 2010, 12:24am

On Aug 7, 2010, at 4:37 PM, Ashley M. wrote:

On 7 Aug 2010, at 22:10, David C. wrote:

So - what should we do? I don’t think changing Minitest is really an option, as too many assertion libraries already wrap Minitest assertions. I don’t think RSpec should be in the business of monitoring methods end-users define to make sure they’re not overriding pre-existing methods (what if you override a method intentionally?). The only thing I’m left with is document this particular case and hope for the best, but that feels unsatisfactory as well.

While I fully agree if you def a method that already exists, you should be expected to deal with it yourself (that’s just the way things are in Ruby), does the same apply to let? I can actually see an argument that you should only be able to let a method that doesn’t already exist, and also only do it once (which is just a consequence of not being able to override a method, given the current implementation).

Can you think of any downsides of preventing RSpec users from overriding existing methods with let?

Yes. Let’s say I write a shared example group with this:

def foo
raise “you need to define a foo method in the block passed to
it_should_behave_like”
end

And you override it using let(:foo), which would be a perfectly
reasonable way to handle it. In fact, it would be the way I would handle
in instinctively, because now I don’t have to wrote my own memoization
handling into the method.

Are there any popular names already taken? Or other problems?

To me, let is magic. I don’t think of it, first and foremost, of defining a method.

From the RDoc: Generates a method whose return value is memoized after the first call.

I see the things it creates as more like local variables, and just remind myself that they’re methods if I wonder why it works.

I’m not sold either way on this, but I think it’s one worth a debate.

The debate is on! Any other opinions out there?

crucoba · August 8, 2010, 1:10pm

On 7 Aug 2010, at 23:18, David C. wrote:

Yes. Let’s say I write a shared example group with this:

def foo
raise “you need to define a foo method in the block passed to it_should_behave_like”
end

And you override it using let(:foo), which would be a perfectly reasonable way to handle it. In fact, it would be the way I would handle in instinctively, because now I don’t have to wrote my own memoization handling into the method.

I instinctively agree with ashley, but I see your point too David.

Would it be awful to make let even more magic, and do something with
#caller to forward the message to MiniTest if it didn’t come from the
code in your example block? Maybe the method defined by let could even
have a __hidden name, and then RSpec can forward the message to that
__hidden method if the message was sent from within the example block.

Sounds pretty horrible, doesn’t it?

The debate is on! Any other opinions out there?

Ash

rspec-users mailing list
[email protected]
http://rubyforge.org/mailman/listinfo/rspec-users

cheers,
Matt

http://blog.mattwynne.net
+44(0)7974 430184

crucoba · August 8, 2010, 3:36pm

On 8 Aug 2010, at 12:05, Matt W. wrote:

And you override it using let(:foo), which would be a perfectly reasonable way to handle it. In fact, it would be the way I would handle in instinctively, because now I don’t have to wrote my own memoization handling into the method.

I instinctively agree with ashley, but I see your point too David.

Would it be awful to make let even more magic, and do something with #caller to forward the message to MiniTest if it didn’t come from the code in your example block? Maybe the method defined by let could even have a __hidden name, and then RSpec can forward the message to that __hidden method if the message was sent from within the example block.

Sounds pretty horrible, doesn’t it?

Hmmm, sounds like it could create a pretty nasty coupling to MiniTest.
Maybe there’s a general solution like:

define_example_method do # or maybe “define_helper” ?
# some stuff that only gets called from examples
end

I’m not sure I’d be keen on let embedding this magic, but maybe as a
general concept it makes more sense, as a way of isolating helper code
in example groups.

There’s another side to the debate, which is that in shared example
groups, I prefer the precondition-check style to the templatemethod-fail
style, ie rather than:

def foo
raise “you need to define a foo method …”
end

I’d prefer to write:

unless respond_to?(:foo)
raise “you need to define a foo method …”
end

But that would involve evaluating the configuration block first >:)

Ash

–
http://www.patchspace.co.uk/
http://www.linkedin.com/in/ashleymoran

crucoba · August 8, 2010, 5:39pm

On Aug 7, 2010, at 4:10 PM, David C. wrote:

You’ll get an error saying “wrong number of arguments (1 for 0)”

This is because the rails assertion, which, when running with Ruby-1.9, delegates to Minitest::Assertions#assert_block, which delegates to a message() method that it defines. So the message() method defined by let() overrides the message() method in the Assertions module, and results in unexpected and undesirable outcomes.

So - what should we do? I don’t think changing Minitest is really an option, as too many assertion libraries already wrap Minitest assertions. I don’t think RSpec should be in the business of monitoring methods end-users define to make sure they’re not overriding pre-existing methods (what if you override a method intentionally?). The only thing I’m left with is document this particular case and hope for the best, but that feels unsatisfactory as well.

Recommendations? Words of wisdom?

FYI - here’s the issue that spawned this thread:

github.com/rspec/rspec-rails

let(:message) overrides the message method in Minitest::Assertions

opened 09:48PM - 07 Aug 10 UTC

closed 03:58AM - 10 Aug 10 UTC

ericbock

If I define let(:message) { mock_model(Message).as_null_object } and have an exa…mple that asserts my controller should redirect_to then I receive a wrong number of arguments (1 for 0) error. I'm using ruby 1.9.1p429, rails 3.0.0.rc and rspec-rails 2.0.0.beta.19 [According to David Chelimsky](http://forums.pragprog.com/forums/95/topics/4911): > RSpec delegates redirect_to to Rails’ assert_template. In Ruby-1.9, we rely on Minitest instead > of Test::Unit, and its Assertions module is mixed in, and that module has a message method in > it. The let method defines a method, whose name is derived from the name passed to it: in this > case, message. That message method overrides the message method in Minitest::Assertions, > and thus, the error you see is created. Here's a failing example: ``` require 'spec_helper' describe MessagesController do let(:message) { mock_model(Message).as_null_object } describe "POST create" do it "redirects to the Messages index" do Message.stub(:new).and_return(message) post :create response.should redirect_to(:action => "index") end end end ```

crucoba · August 8, 2010, 5:42pm

On 8 Aug 2010, at 16:38, David C. wrote:

a Rails assertion in an example in that group
FYI - here’s the issue that spawned this thread: let(:message) overrides the message method in Minitest::Assertions · Issue #152 · rspec/rspec-rails · GitHub
Can you use the Assertions module some other way than mixing it into the
example (thereby polluting it with the Assertions module’s methods?)

rspec-users mailing list
[email protected]
http://rubyforge.org/mailman/listinfo/rspec-users

cheers,
Matt

http://blog.mattwynne.net
+44(0)7974 430184

crucoba · August 8, 2010, 5:52pm

On Aug 8, 2010, at 8:05 AM, Ashley M. wrote:

On 8 Aug 2010, at 12:05, Matt W. wrote:

And you override it using let(:foo), which would be a perfectly reasonable way to handle it. In fact, it would be the way I would handle in instinctively, because now I don’t have to wrote my own memoization handling into the method.

I instinctively agree with ashley, but I see your point too David.

Would it be awful to make let even more magic, and do something with #caller to forward the message to MiniTest if it didn’t come from the code in your example block? Maybe the method defined by let could even have a __hidden name, and then RSpec can forward the message to that __hidden method if the message was sent from within the example block.

Sounds pretty horrible, doesn’t it?

Yes! It seems crazy to me to put this burden on RSpec, or any downstream
library. The problem here stems from the fact that Minitest consumed a
non-testing-domain-specific name here. Test::Unit uses build_message,
which is still sort of generic, but at least it’s a verb. I submitted a
request to change message to build_message [1], but even if Ryan agrees
to do it, we wouldn’t likely see it in ruby-1.9.2, so we’d still have
the conflict for some time.

Also, let’s say he does agree - now people who like to prefix their
test-data-building-helper-methods with build_xxx will be screwed too

I wonder if the right answer is for RSpec to implement its own base
assertion library, matching the Test::Unit/Minitest assertion APIs. That
would have an interesting side effect in that domain-specific
assertion-libraries could be used in RSpec without having to depend on
Test::Unit or Minitest. It would also allow folks who like RSpec’s
structure (example groups and examples) but prefer assert_xxx over
RSpec’s matcher to use RSpec the way they’d like without the additional
dependency.

Thoughts on this? Really just an idea. I’m fairly convinced it’s a bad
one, but my head is sort of spinning over this issue right now (or maybe
it’s the cocktails at http://thedrchicago.com/), so I figured I’d throw
it against the wall and see if it sticks (think spaghetti, or Jackson
Pollock).

Hmmm, sounds like it could create a pretty nasty coupling to MiniTest. Maybe there’s a general solution like:

define_example_method do # or maybe “define_helper” ?

some stuff that only gets called from examples

end

I’m not sure I’d be keen on let embedding this magic, but maybe as a general concept it makes more sense, as a way of isolating helper code in example groups.

This suggests, to me, “don’t use Ruby if you want things to work right.”

end

But that would involve evaluating the configuration block first >:)

Keep in mind that it_should_behave_like generates a nested group, so
shared group authors can do this:

shared_examples_for “bar” do
unless respond_to?(:foo)
raise “gotta have foo”
end
end

and host group authors can do this:

describe “thing” do
let(:foo) { Foo.new }
it_should_behave_like “bar”
end

That’s part of the beauty of eval’ing the conditions block last - we get
the best of both worlds.

Cheers,
David

[1]
http://rubyforge.org/tracker/index.php?func=detail&aid=28457&group_id=1040&atid=4097

crucoba · August 8, 2010, 6:13pm

On 8 Aug 2010, at 16:53, David C. wrote:

a model named Message
Recommendations? Words of wisdom?

FYI - here’s the issue that spawned this thread: let(:message) overrides the message method in Minitest::Assertions · Issue #152 · rspec/rspec-rails · GitHub

Can you use the Assertions module some other way than mixing it into the example (thereby polluting it with the Assertions module’s methods?)

I like the idea in the abstract, but most of the rails assertions rely on some state that is local to the example (@response, @controller, @request, etc, etc). RSpec could gather up all those instance variables and pass them into an assertion-wrapper object, but then it would be highly coupled to that implementation and would lead us down a familiar and unfriendly path of forcing rspec-rails releases for every rails release. That’s a world I hope to leave behind with Rails 3

So leave the rails assertions mixed into the example, but forward all
the calls to the MiniTest::Assertions methods to some other object that
has them mixed in. Won’t that work?

rspec-users mailing list
[email protected]
http://rubyforge.org/mailman/listinfo/rspec-users

cheers,
Matt

http://blog.mattwynne.net
+44(0)7974 430184

crucoba · August 8, 2010, 6:00pm

On Aug 8, 2010, at 10:40 AM, Matt W. wrote:

let(:message) or def message in an example group

FYI - here’s the issue that spawned this thread: let(:message) overrides the message method in Minitest::Assertions · Issue #152 · rspec/rspec-rails · GitHub

Can you use the Assertions module some other way than mixing it into the example (thereby polluting it with the Assertions module’s methods?)

I like the idea in the abstract, but most of the rails assertions rely
on some state that is local to the example (@response, @controller,
@request, etc, etc). RSpec could gather up all those instance
variables and pass them into an assertion-wrapper object, but then it
would be highly coupled to that implementation and would lead us down a
familiar and unfriendly path of forcing rspec-rails releases for every
rails release. That’s a world I hope to leave behind with Rails 3

It would also eliminate the option to use the Rails assertions directly
in examples.

Oh, well

crucoba · August 9, 2010, 3:00am

On Aug 8, 2010, at 11:13 AM, Matt W. wrote:

So leave the rails assertions mixed into the example, but forward all the calls to the MiniTest::Assertions methods to some other object that has them mixed in. Won’t that work?
Here’s a prototype implementation:
http://github.com/rspec/rspec-rails/commit/0cd384536cf532435ec8f290a9c357b60872acd7

It’s on a branch
(http://github.com/rspec/rspec-rails/tree/assertion-delegate) because
I’m not convinced this is the right way to go yet, but I’d like some
feedback from anyone who wishes to peruse and comment.

Thanks,
David

crucoba · August 8, 2010, 6:58pm

Good error messages are important to a library’s usability. Could we
find away to give the user a good error message when they override a
“reserved method”?

I’m thinking this could be accomplished with 2 simple pieces:

A method_added hook in Rspec-core that gives a warning or error
when a reserved method is overridden.
An API that allows libraries to add to the reserved methods list.

That way, rspec doesn’t have to have knowledge of other libraries; it
would be the responsibility of other libraries to add their reserved
methods to the list.

Myron

crucoba · August 9, 2010, 3:31am

On Aug 8, 2010, at 11:58 AM, Myron M. wrote:

That way, rspec doesn’t have to have knowledge of other libraries; it
would be the responsibility of other libraries to add their reserved
methods to the list.

Yehuda K. made a similar suggestion to me, referencing some code from
merb:

github.com

merb/merb/blob/master/merb-core/lib/merb-core/controller/merb_controller.rb#L508


      
          class Merb::Controller < Merb::AbstractController
          
          
  class_inheritable_accessor :_hidden_actions, :_shown_actions, 
                                       :_overridable, :_override_bang
          
          
  self._hidden_actions ||= []
            self._shown_actions  ||= []
            self._overridable    ||= []
            self._override_bang  ||= []
          
          
  cattr_accessor :_subclasses
            self._subclasses = Set.new
          
          
  # @api private
            def self.subclasses_list() _subclasses end
          
          
  include Merb::ResponderMixin
            include Merb::ControllerMixin
            include Merb::AuthenticationMixin
            include Merb::ConditionalGetMixin
          
          
  # @param [Merb::Controller] klass The Merb::Controller inheriting from
            #   the base class.
            #
            # @api private
            def self.inherited(klass)
              _subclasses << klass.to_s
              super
              klass._template_root = Merb.dir_for(:view) unless self._template_root
            end
          
          
  # @param [Array<Symbol>] *names Array of method names that should be
            #   overridable in application controllers.
            #
            # @return [Array] The list of methods that are overridable
            #
            # @api plugin
            def self.overridable(*names)
              self._overridable.push(*names)
            end
          
          
  # Allow overriding methods.
            #
            # In an application controller, call override! before a method to indicate
            # that you want to override a method in Merb::Controller that is not
            # normally overridable.
            #
            # Doing this may potentially break your app in a future release of Merb,
            # and this is provided for users who are willing to take that risk.
            # Without using override!, Merb will raise an error if you attempt to
            # override a method defined on Merb::Controller.
            #
            # This is to help users avoid a common mistake of defining an action
            # that overrides a core method on Merb::Controller.
            #
            #     class Kontroller < Application
            #       def status
            #         render
            #       end
            #     end
            #
            # will raise a Merb::ReservedError, because {#status} is a method on
            # Merb::Controller.
            #
            #     class Kontroller < Application
            #       override! :status
            #       def status
            #         some_code || super
            #       end
            #     end
            #
            # will not raise a Merb::ReservedError, because the user specifically
            # decided to override the status method.
            #
            # @param [Array<Symbol>] *names An Array of methods that will override
            #   Merb core classes on purpose
            #
            # @api public
            def self.override!(*names)
              self._override_bang.push(*names)
            end
          
          
  # Hide each of the given methods from being callable as actions.
            #
            # @param [Array<#to_s>] *names Actions that should be added to the list.
            #
            # @return [Array<String>] Actions that should not be possible
            #   to dispatch to.
            #
            # @api public
            def self.hide_action(*names)
              self._hidden_actions = self._hidden_actions | names.map { |n| n.to_s }
            end
          
          
  # Makes each of the given methods being callable as actions. You can use
            # this to make methods included from modules callable as actions.
            #
            # @param [Array<#to_s>] *names Actions that should be added to the list.
            #
            # @return [Array<String>] Actions that should be dispatched to even if
            #   they would not otherwise be.
            #
            # @example Use like:
            #     module Foo
            #       def self.included(base)
            #         base.show_action(:foo)
            #       end
            #
            #       def foo
            #        # some actiony stuff
            #       end
            #
            #       def foo_helper
            #         # this should not be an action
            #       end
            #     end
            #
            # @api public
            def self.show_action(*names)
              self._shown_actions = self._shown_actions | names.map {|n| n.to_s}
            end
          
          
  # The list of actions that are callable, after taking defaults,
            # _hidden_actions and _shown_actions into consideration. It is calculated
            # once, the first time an action is dispatched for this controller.
            #
            # @return [SimpleSet<String>] Actions that should be callable.
            #
            # @api public
            def self.callable_actions
              @callable_actions ||= Extlib::SimpleSet.new(_callable_methods)
            end
          
          
  # Stub method so plugins can implement param filtering if they want.
            #
            # @param [Hash<Symbol => String>] params Parameters
            #
            # @return [Hash<Symbol => String>] A new list of params, filtered as desired
            #
            # @api plugin
            # @overridable
            def self._filter_params(params)
              params
            end
            overridable :_filter_params
          
          
  # All methods that are callable as actions.
            #
            # @return [Array] A list of method names that are also actions.
            #
            # @api private
            def self._callable_methods
              callables = []
              klass = self
              begin
                callables << (klass.public_instance_methods(false) + klass._shown_actions).map{|m| m.to_s} - klass._hidden_actions
                klass = klass.superclass
              end until klass == Merb::AbstractController || klass == Object
              callables.flatten.reject{|action| action =~ /^_.*/}.map {|x| x.to_s}
            end
          
          
  # MIME-type aware template locations.
            #
            # This is overridden from AbstractController, which defines a version
            # that does not involve mime-types.
            #
            # @see AbstractController#_template_location
            #
            # #### Notes
            # By default, this renders ":controller/:action.:type". To change this,
            # override it in your application class or in individual controllers.
            #
            # @api public
            # @overridable
            def _template_location(context, type, controller)
              _conditionally_append_extension(controller ? "#{controller}/#{context}" : "#{context}", type)
            end
            overridable :_template_location
          
          
  # MIME-type aware template locations.
            #
            # This is overridden from AbstractController, which defines a version
            # that does not involve mime-types.
            #
            # @see AbstractController#_template_location
            #
            # @param [String] template The absolute path to a template, without
            #   mime type and template extension. The mime-type extension is optional
            #   and will be appended from the current content type if it hasn't been
            #   added already.
            # @param [#to_s] type The mime-type of the template that will be rendered.
            #   Defaults to nil.
            #
            # @api public
            def _absolute_template_location(template, type)
              _conditionally_append_extension(template, type)
            end
          
          
  # Build a new controller.
            #
            # Sets the variables that came in through the dispatch as available to
            # the controller.
            #
            # @param [Merb::Request] request The Merb::Request that came in from Rack.
            # @param [Integer] status An integer code for the status.
            # @param [Hash<header => value>] headers A hash of headers to start the
            #   controller with. These headers can be overridden later by the #headers
            #   method.
            #
            # @api plugin
            # @overridable
            def initialize(request, status=200, headers={'Content-Type' => 'text/html; charset=utf-8'})
              super()
              @request, @_status, @headers = request, status, headers
            end
            overridable :initialize
          
          
  # Call the controller as a Rack endpoint.
            #
            # Expects:
            # * **`env["merb.status"]`:** the default status code to be returned
            # * **`env["merb.action_name"]`:** the action name to dispatch
            # * **`env["merb.request_start"]`:** a `Time` object representing the
            #   start of the request.
            #
            # @param [Hash] env A rack environment
            #
            # @return [Array<Integer, Hash, #each>] A standard Rack response
            #
            # @api public
            def self.call(env)
              new(Merb::Request.new(env), env["merb.status"])._call
            end
          
          
  # Dispatches the action and records benchmarks
            #
            # @return [Array<Integer, Hash, #each>] A standard Rack response
            # 
            # @api private
            def _call
              _dispatch(request.env["merb.action_name"])
              _benchmarks[:dispatch_time] = Time.now - request.env["merb.request_start"]
              Merb.logger.info { _benchmarks.inspect }
              Merb.logger.flush
              rack_response        
            end
          
          
  # Dispatch the action.
            #
            # Extends {AbstractController#_dispatch} with logging, error handling,
            # and benchmarking.
            #
            # @param [#to_s] action The action to dispatch to.
            #
            # @return [Merb::Controller] self
            # @todo See {AbstractController#_dispatch} and ticket #1335 about the
            #   return type.
            #
            # @raise [ActionNotFound] The requested action was not found in class.
            #
            # @api plugin
            def _dispatch(action=:index)
              Merb.logger.info { "Params: #{self.class._filter_params(request.params).inspect}" }
              start = Time.now
              if self.class.callable_actions.include?(action.to_s)
                super(action)
              else
                raise ActionNotFound, "Action '#{action}' was not found in #{self.class}"
              end
              @_benchmarks[:action_time] = Time.now - start
              self
            end
          
          
  # @api public
            attr_reader :request, :headers
          
          
  # Response status code.
            #
            # @return [Fixnum]
            #
            # @api public
            def status
              @_status
            end
          
          
  # Set the response status code.
            #
            # @param [Fixnum, Symbol] s A status code or named HTTP status
            #
            # @api public
            def status=(s)
              if s.is_a?(Symbol) && STATUS_CODES.key?(s)
                @_status = STATUS_CODES[s]
              elsif s.is_a?(Fixnum)
                @_status = s
              else
                raise ArgumentError, "Status should be of type Fixnum or Symbol, was #{s.class}"
              end
            end
          
          
  # The parameters from the request object.
            #
            # @return [Hash]
            #
            # @api public
            def params()  request.params  end
          
          
  # Generate URLs.
            #
            # Same as {Merb::Router.url}, but allows to pass `:this` as a name
            # to use the name of the current {Request}. All parameters of the current
            # request are also added to the arguments.
            #
            # @see Merb::Router.url
            #
            # @api public
            def url(name, *args)
              args << params
              name = request.route if name == :this
              Merb::Router.url(name, *args)
            end
          
          
  # Generates a URL for a single or nested resource.
            #
            # Same as {Merb::Router.resource}, but all parameters of the current
            # request are also added to the arguments.
            #
            # @see Merb::Router.resource
            #
            # @api public
            def resource(*args)
              args << params
              Merb::Router.resource(*args)
            end
          
          
  alias_method :relative_url, :url
            
            # Returns the absolute URL including the passed protocol and host.
            #
            # Calls {AbstractController#absolute_url} with the protocol and host
            # options pre-populated from the current request unless explicitly
            # specified.
            #
            # @see AbstractController#absolute_url
            #
            # @api public
            def absolute_url(*args)
              options  = extract_options_from_args!(args) || {}
              options[:protocol] ||= request.protocol
              options[:host] ||= request.host
              args << options
              super(*args)
            end
          
          
  # The results of the controller's render, to be returned to Rack.
            #
            # @return [Array<Integer, Hash, String>] The controller's status code,
            #   headers, and body
            #
            # @api private
            def rack_response
              [status, headers, Merb::Rack::StreamWrapper.new(body)]
            end
          
          
  # Sets a controller to be "abstract".
            #
            # This controller will not be able to be routed to and is used for super
            # classing only.
            #
            # @api public
            def self.abstract!
              @_abstract = true
            end
          
          
  # Asks a controller if it is abstract
            #
            # @return [Boolean] True if the controller has been set as abstract.
            #
            # @api public
            def self.abstract?
              !!@_abstract 
            end
          
          
  # Hide any methods that may have been exposed as actions before.
            hide_action(*_callable_methods)
          
          
  private
          
          
  # If not already added, add the proper mime extension to the template path.
            #
            # @param [#to_s] template The template path to append the mime type to.
            # @param [#to_s] type The extension to append to the template path
            #   conditionally.
            #
            # @api private
            def _conditionally_append_extension(template, type)
              type && !template.match(/\.#{type.to_s.escape_regexp}$/) ? "#{template}.#{type}" : template
            end
            
            # When a method is added to a subclass of Merb::Controller (i.e. an app controller) that
            # is defined on Merb::Controller, raise a Merb::ReservedError. An error will not be raised
            # if the method is defined as overridable in the Merb API.
            #
            # This behavior can be overridden by using `override! method_name` before attempting to
            # override the method.
            #
            # @param [#to_sym] meth The method that is being added
            #
            # @raise [Merb::ReservedError] If the method being added is in a subclass
            #   of Merb::Controller, the method is defined on Merb::Controller, it is
            #   not defined as overridable in the Merb API, and the user has not
            #   specified that it can be overridden.
            #
            # @return [nil]
            #
            # @api private
            def self.method_added(meth)
              if self < Merb::Controller && Merb::Controller.method_defined?(meth) && 
                !self._overridable.include?(meth.to_sym) && !self._override_bang.include?(meth.to_sym)
          
          
      raise Merb::ReservedError, "You tried to define #{meth} on " \
                  "#{self.name} but it was already defined on Merb::Controller. " \
                  "If you meant to override a core method, use override!"
              end
            end
          end

This file has been truncated. show original

Merb also has an override! method that end users can use to override the
registered reserved methods, which I agree would be a necessary part of
this. The idea being that any user that does that does so explicitly and
knowingly.

The blacklist comment probably wouldn’t work for upstream libs like
Rails, Test::Unit or MiniUnit. It would be up to RSpec to define those
lists. But maybe that’s an acceptable tradeoff. WDYT?

crucoba · August 9, 2010, 1:44pm

On 9 Aug 2010, at 01:54, David C. wrote:

Hey all,
You’ll get an error saying “wrong number of arguments (1 for 0)”

I like the idea in the abstract, but most of the rails assertions rely on some state that is local to the example (@response, @controller, @request, etc, etc). RSpec could gather up all those instance variables and pass them into an assertion-wrapper object, but then it would be highly coupled to that implementation and would lead us down a familiar and unfriendly path of forcing rspec-rails releases for every rails release. That’s a world I hope to leave behind with Rails 3

So leave the rails assertions mixed into the example, but forward all the calls to the MiniTest::Assertions methods to some other object that has them mixed in. Won’t that work?

Here’s a prototype implementation: http://github.com/rspec/rspec-rails/commit/0cd384536cf532435ec8f290a9c357b60872acd7

It’s on a branch (http://github.com/rspec/rspec-rails/tree/assertion-delegate) because I’m not convinced this is the right way to go yet, but I’d like some feedback from anyone who wishes to peruse and comment.

Yeah, that’s what I was talking about. Couple of thoughts / questions:

I’m still not clear why you need to copy the instance variable over
though - do the rails assertions get monkey-patched into the
Test::Unit::Assertions module then?
Also, how come there’s nothing in the specs about the #message method
that caused all this?

[email protected]
http://rubyforge.org/mailman/listinfo/rspec-users

cheers,
Matt

http://blog.mattwynne.net
+44(0)7974 430184

crucoba · August 9, 2010, 2:21pm

On Aug 9, 2010, at 6:37 AM, Matt W. wrote:

On 8 Aug 2010, at 16:38, David C. wrote:

a Rails assertion in an example in that group
FYI - here’s the issue that spawned this thread: let(:message) overrides the message method in Minitest::Assertions · Issue #152 · rspec/rspec-rails · GitHub

Yeah, that’s what I was talking about. Couple of thoughts / questions:

I’m still not clear why you need to copy the instance variable over though - do the rails assertions get monkey-patched into the Test::Unit::Assertions module then?

No - holdover from exploratory session.

Also, how come there’s nothing in the specs about the #message method that caused all this?

Good point.

New patch:

crucoba · August 9, 2010, 2:44pm

On 9 Aug 2010, at 13:04, David C. wrote:

On 8 Aug 2010, at 16:53, David C. wrote:

Ruby-1.9

Here’s a prototype implementation: http://github.com/rspec/rspec-rails/commit/0cd384536cf532435ec8f290a9c357b60872acd7

Good point.

New patch: Wrap T/U and/or MiniTest assertions in a separate scope from that of the · rspec/rspec-rails@8660031 · GitHub

This is like an extremely sluggish kind of pair programming!

What do you think of it now? Is it growing on you? What worries you
about it?

[email protected]
[email protected]
http://rubyforge.org/mailman/listinfo/rspec-users

rspec-users mailing list
[email protected]
http://rubyforge.org/mailman/listinfo/rspec-users

cheers,
Matt

http://blog.mattwynne.net
+44(0)7974 430184

crucoba · August 9, 2010, 3:25pm

On Aug 9, 2010, at 7:42 AM, Matt W. wrote:

On Aug 8, 2010, at 11:13 AM, Matt W. wrote:

It turns out that if you have
This is because the rails assertion, which, when running with Ruby-1.9, delegates to Minitest::Assertions#assert_block, which delegates to a message() method that it defines. So the message() method defined by let() overrides the message() method in the Assertions module, and results in unexpected and undesirable outcomes.

No - holdover from exploratory session.

Also, how come there’s nothing in the specs about the #message method that caused all this?

Good point.

New patch: Wrap T/U and/or MiniTest assertions in a separate scope from that of the · rspec/rspec-rails@8660031 · GitHub

This is like an extremely sluggish kind of pair programming!

What do you think of it now? Is it growing on you? What worries you about it?

Not sure yet. On one hand it feels sort of unnecessary, but on the
other, it does solve this particular anomaly. I’m going to sit on it for
a day or two and see if any specific arguments against come to mind. In
the mean time, other opinions would be welcome.

Cheers,
David

crucoba · August 11, 2010, 10:15pm

Sorry it’s taken me so long to respond–I have considerably less time
on weekdays than the weekend to take care of things like this.

Yehuda K. made a similar suggestion to me, referencing some code from merb:http://github.com/merb/merb/blob/master/merb-core/lib/merb-core/contr…

Merb also has an override! method that end users can use to override the registered reserved methods, which I agree would be a necessary part of this. The idea being that any user that does that does so explicitly and knowingly.

Merb’s implementation is very similar to what I had in mind. It’s
nice to see I’m not in left field with my idea :).

I agree that having something like override! is important, although I
think I slightly prefer an API like this:

allow_reserved_overrides do
def reserved_method
end
end

Or maybe I like blocks too much…

The blacklist comment probably wouldn’t work for upstream libs like Rails, Test::Unit or MiniUnit. It would be up to RSpec to define those lists. But maybe that’s an acceptable tradeoff. WDYT?

RSpec is pretty high-profile in the Ruby community, so we could
hopefully get most libraries to add their reserved methods using
something like:

if defined?(RSpec::Core.add_reserved_methods)
RSpec::Core.add_reserved_methods :foo, :bar, :bazz
end

As far as Rails goes, rspec-rails seems like a natural point to
register these reserved methods. For libraries that are distributed
with ruby like Test::Unit, I think it’s acceptable to register their
reserved methods in rspec-core itself.

What do others think of this idea? I’m willing to take a stab at
implementing this if there is support for it.

Myron

crucoba · August 12, 2010, 8:32pm

On 12 Aug 2010, at 04:30, David C. wrote:

I think they should all be registered in the same place, in rspec-core. Or are you saying that rspec-rails would take the responsibility of registering the names for rspec-rails, rails, test/unit and minitest? That makes sense to me, as long as they’re all using RSpec::Core::register_reserved_name (or whatever).

What do others think of this idea? I’m willing to take a stab at
implementing this if there is support for it.

I’m still not sold on this idea yet. Seems like a lot of complexity for not much benefit. I’ve already taken care of the message issue by encapsulating the assertion libs in a separate scope.

Anybody else have opinions on this?

The idea sounds interesting, but sounds like it could lead to a nasty
dependency down the line. Is there any way of doing the core of this as
an external gem, that all interested projects could use?

I’m only following the edge of this thread, but by dependency radar went
off - it’s a bit oversensitive.

–
http://www.patchspace.co.uk/
http://www.linkedin.com/in/ashleymoran

crucoba · August 12, 2010, 5:48am

On Aug 11, 2010, at 3:05 PM, Myron M. wrote:

I agree that having something like override! is important, although I

with ruby like Test::Unit, I think it’s acceptable to register their
reserved methods in rspec-core itself.

I think they should all be registered in the same place, in rspec-core.
Or are you saying that rspec-rails would take the responsibility of
registering the names for rspec-rails, rails, test/unit and minitest?
That makes sense to me, as long as they’re all using
RSpec::Core::register_reserved_name (or whatever).

What do others think of this idea? I’m willing to take a stab at
implementing this if there is support for it.

I’m still not sold on this idea yet. Seems like a lot of complexity for
not much benefit. I’ve already taken care of the message issue by
encapsulating the assertion libs in a separate scope.

Anybody else have opinions on this?