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

Fix delegate_method when used with shoulda-context #591

Conversation

mcmire
Copy link
Collaborator

@mcmire mcmire commented Sep 26, 2014

This is a fix for #581.


When specifying a custom subject, a straightforward usage of
delegate_method, such as the following

class UserPresenter
  delegate :id, to: :user

  attr_reader :user

  def initialize(user)
    @user = user
  end
end

class UserPresenterTest < ActiveSupport::TestCase
  subject { UserPresenter.new(User.new) }
  should delegate_method(:id).to(:user)
end

would error with something like:

ArgumentError: wrong number of arguments (0 for 1)

This happens because the should method in shoulda-context asks the
DelegateMethodMatcher object for its description so that it can use it
to give the test a name and create a method that Test::Unit can run.
Now, the matcher's description needs a subject in order to determine
whether a class or an instance is being tested here -- if a class is
being tested the description will be "should delegate #id to #user
object", if a class then "should delegate .id to .user object".
Unfortunately the matcher doesn't know what the subject is before
its #description method is called -- it only knows about this when it
gets evaluated.

Within the matcher we do have access to the current context class, so we
could read the subject block off of it and evaluate it. However, in
order to properly do this we also need access to the instance of the
test itself, which we do not have until the matcher is evaluated (by
which point it's too late).

Since there's really no way to solve this problem apart from rewriting a
lot of shoulda-context, and since often times your subject is an
instance and not a class, just assume it's an instance in this case.

@mcmire mcmire added this to the 2.7.1 milestone Sep 26, 2014
@mcmire mcmire force-pushed the ew-fix-delegate-method-with-shoulda-context branch from 8a3c2bd to 4bf829a Compare October 8, 2014 04:11
The minitest dependency is necessary in order to write Cucumber tests
that depend on Minitest.
When specifying a custom subject, a straightforward usage of
`delegate_method`, such as the following

    class UserPresenter
      delegate :id, to: :user

      attr_reader :user

      def initialize(user)
        @user = user
      end
    end

    class UserPresenterTest < ActiveSupport::TestCase
      subject { UserPresenter.new(User.new) }
      should delegate_method(:id).to(:user)
    end

would error with something like:

    ArgumentError: wrong number of arguments (0 for 1)

This happens because the `should` method in shoulda-context asks the
DelegateMethodMatcher object for its description so that it can use it
to give the test a name and create a method that Test::Unit can run.
Now, the matcher's description needs a subject in order to determine
whether a class or an instance is being tested here -- if a class is
being tested the description will be "should delegate #id to \#user
object", if a class then "should delegate .id to .user object".
Unfortunately the matcher doesn't know what the subject is before
its #description method is called -- it only knows about this when it
gets evaluated.

Within the matcher we do have access to the current context class, so we
could read the subject block off of it and evaluate it. However, in
order to properly do this we also need access to the instance of the
test itself, which we do not have until the matcher is evaluated (by
which point it's too late).

Since there's really no way to solve this problem apart from rewriting a
lot of shoulda-context, and since often times your subject is an
instance and not a class, just assume it's an instance in this case.
@mcmire mcmire force-pushed the ew-fix-delegate-method-with-shoulda-context branch from 9d34b95 to c683aed Compare October 8, 2014 07:33
@mcmire
Copy link
Collaborator Author

mcmire commented Oct 9, 2014

Merged in caa87ae...c683aed.

@mcmire mcmire closed this Oct 9, 2014
@mcmire mcmire deleted the ew-fix-delegate-method-with-shoulda-context branch October 9, 2014 04:40
mcmire added a commit that referenced this pull request Oct 9, 2014
jsonn pushed a commit to jsonn/pkgsrc that referenced this pull request Jun 12, 2015
pkgsrc change: allow build on Ruby 2.2.

# 2.8.0

### Deprecations

* `ensure_length_of` has been renamed to `validate_length_of`.
  `ensure_length_of` is deprecated and will be removed in 3.0.0.

* `set_the_flash` has been renamed to `set_flash`. `set_the_flash` is
  deprecated and will be removed in 3.0.0.

* `set_session(:foo)` is deprecated in favor of `set_session[:foo]`.
  `set_session(:foo)` will be invalid syntax in 3.0.0.

* Using `should set_session[:key].to(nil)` to assert that that a value has not
  been set is deprecated. Please use `should_not set_session[:key]` instead.
  In 3.0.0, `should set_session[:key].to(nil)` will only pass if the value is
  truly nil.

### Bug fixes

* Fix `delegate_method` so that it works again with shoulda-context. ([#591])

* Fix `validate_uniqueness_of` when used with `scoped_to` so that when one of
  the scope attributes is a polymorphic `*_type` attribute and the model has
  another validation on the same attribute, the matcher does not fail with an
  error. ([#592])

* Fix `has_many` used with `through` so that when the association does not
  exist, and the matcher fails, it does not raise an error when producing the
  failure message. ([#588])

* Fix `have_and_belong_to_many` used with `join_table` so that it does not fail
  when `foreign_key` and/or `association_foreign_key` was specified on the
  association as a symbol instead of a string. ([#584])

* Fix `allow_value` when an i18n translation key is passed to `with_message` and
  the `:against` option is used to specify an alternate attribute. A bug here
  also happened to affect `validate_confirmation_of` when an i18n translation
  key is passed to `with_message`. ([#593])

* Fix `class_name` qualifier for association matchers so that if the model being
  referenced is namespaced, the matcher will correctly resolve the class before
  checking it against the association's `class_name`. ([#537])

* Fix `validate_inclusion_of` used with `with_message` so that it fails if given
  a message that does not match the message on the validation. ([#598])

* Fix `route` matcher so that when controller and action are specified in hash
  notation (e.g. `posts#show`), route parameters such as `id` do not need to be
  specified as a string but may be specified as a number as well. ([#602])

* Fix `allow_value`, `validate_numericality_of` and `validate_inclusion_of` so
  that they handle RangeErrors emitted from ActiveRecord 4.2. These exceptions
  arise whenever we attempt to set an attribute using a value that lies outside
  the range of the column (assuming the column is an integer). RangeError is now
  treated specially, failing the test instead of bubbling up as an error.
  ([#634], [#637], [#642])

### Features

* Add ability to test `:primary_key` option on associations. ([#597])

* Add `allow_blank` qualifier to `validate_uniqueness_of` to complement
  the `allow_blank` option. ([#543])

* Change `set_session` so that #[] and #to qualifiers are optional, similar to
  `set_flash`. That is, you can now say `should set_session` to assert that any
  flash value has been set, or `should set_session.to('value')` to assert that
  any value in the session is 'value'.

* Change `set_session` so that its #to qualifier supports regexps, similar to
  `set_flash`.

* Add `with_prefix` qualifier to `delegate_method` to correspond to the `prefix`
  option for Rails's `delegate` macro. ([#622])

* Add support for Rails 4.2, especially fixing `serialize` matcher to remove
  warning about `serialized_attributes` being deprecated. ([#627])

* Update `dependent` qualifier on association matchers to support `:destroy`,
  `:delete`, `:nullify`, `:restrict`, `:restrict_with_exception`, and
  `:restrict_with_error`. You can also pass `true` or `false` to assert that
  the association has (or has not) been declared with *any* dependent option.
  ([#631])

### Improvements

* Tweak `allow_value` failure message so that it reads a bit nicer when listing
  existing errors.

[#591]: thoughtbot/shoulda-matchers#591
[#592]: thoughtbot/shoulda-matchers#592
[#588]: thoughtbot/shoulda-matchers#588
[#584]: thoughtbot/shoulda-matchers#584
[#593]: thoughtbot/shoulda-matchers#593
[#597]: thoughtbot/shoulda-matchers#597
[#537]: thoughtbot/shoulda-matchers#537
[#598]: thoughtbot/shoulda-matchers#598
[#602]: thoughtbot/shoulda-matchers#602
[#543]: thoughtbot/shoulda-matchers#543
[#622]: thoughtbot/shoulda-matchers#622
[#627]: thoughtbot/shoulda-matchers#627
[#631]: thoughtbot/shoulda-matchers#631
[#634]: thoughtbot/shoulda-matchers#634
[#637]: thoughtbot/shoulda-matchers#637
[#642]: thoughtbot/shoulda-matchers#642

# 2.7.0

### Deprecations

* `ensure_inclusion_of` has been renamed to `validate_inclusion_of`.
  `ensure_inclusion_of` is deprecated and will be removed in 3.0.0.

* `ensure_exclusion_of` has been renamed to `validate_exclusion_of`.
  `ensure_exclusion_of` is deprecated and will be removed in 3.0.0.

### Bug fixes

* Fix `delegate_method` so that it does not raise an error if the method that
  returns the delegate object is private.

* Warn when `ensure_inclusion_of` is chained with `.in_array([false, true])`
  as well as with `.in_array([true, false])`.

* Fix `set_session` so that the `to` qualifier if given nil checks that the
  session variable in question was set to nil (previously this actually did
  nothing).

* Fix `filter_param` so that it works when `config.filter_parameters` contains
  regexes.

* Fix `delegate_method` so that it can be required independent of Active
  Support.

* Fix `validate_uniqueness_of`. When used against an unpersisted record whose
  model contained a non-nullable column other than the one being validated, the
  matcher would break. Even if the test set that column to a value beforehand,
  the record had to be persisted in order for the matcher to work. Now this is
  no longer the case and the record can remain unpersisted.

* Fix `validate_absence_of`: it required that a string be passed as the
  attribute name rather than a symbol (which is the usual and documented usage).

### Improvements

* `have_and_belongs_to_many` now checks to make sure that the join table
  contains the correct columns for the left- and right-hand side of the
  association.

* Reword failure message for `delegate_method` so that it's a little more
  helpful.

### Features

* Add new matcher `define_enum_for` to test usage of the `enum` macro introduced
  in Rails 4.1.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant