search

thoughtbot / shoulda-matchers

Simple one-liner tests for common Rails functionality
https://matchers.shoulda.io
MIT License
3.53k stars 909 forks source link

ensure_inclusion_of does not work with polymorphic attributes #574

Closed gnilrets closed 4 years ago

gnilrets commented 10 years ago

So I had ensure_inclusion_of working just fine, then I migrated some things to use FactoryGirl instead of explicit create statements and suddenly it's failing. If I dup the object created by FactoryGirl, it works again. Here's an example in my spec:

  before { @job_spec = FactoryGirl.create(:job_spec) }
  subject { @job_spec }

  it { should ensure_inclusion_of(:job_template_type).in_array(JobSpec::JOB_TEMPLATE_TYPES) }

  context "using a copy to get around ensure_inclusion_of bug" do
    before { @job_spec_dup = @job_spec.dup }
    subject { @job_spec_dup }

    it { should ensure_inclusion_of(:job_template_type).in_array(JobSpec::JOB_TEMPLATE_TYPES) }
  end

The first test fails, the second is successful:

JobSpec
  should ensure inclusion of job_template_type in ["TplBirstSoapGenericCommand", "TplBirstDuplicateSpace"] (FAILED - 1)
  using a copy to get around ensure_inclusion_of bug
    should ensure inclusion of job_template_type in ["TplBirstSoapGenericCommand", "TplBirstDuplicateSpace"]

Failures:

  1) JobSpec should ensure inclusion of job_template_type in ["TplBirstSoapGenericCommand", "TplBirstDuplicateSpace"]
     Failure/Error: it { should ensure_inclusion_of(:job_template_type).in_array(JobSpec::JOB_TEMPLATE_TYPES) }
     NameError:
       wrong constant name shouldamatchersteststring
     # ./spec/models/job_spec_spec.rb:19:in `block (2 levels) in <top (required)>'

Finished in 0.3576 seconds (files took 1.73 seconds to load)
15 examples, 1 failure

Failed examples:

rspec ./spec/models/job_spec_spec.rb:19 # JobSpec should ensure inclusion of job_template_type in ["TplBirstSoapGenericCommand", "TplBirstDuplicateSpace"]
mcmire commented 10 years ago

Okay, can you re-run these tests with the --backtrace option?

gnilrets commented 10 years ago 
Failures:

  1) JobSpec should ensure inclusion of job_template_type in ["TplBirstSoapGenericCommand", "TplBirstDuplicateSpace"]
     Failure/Error: it { should ensure_inclusion_of(:job_template_type).in_array(JobSpec::JOB_TEMPLATE_TYPES) }
     NameError:
       wrong constant name shouldamatchersteststring
     # ./vendor/gems/activesupport-4.1.4/lib/active_support/inflector/methods.rb:238:in `const_get'
     # ./vendor/gems/activesupport-4.1.4/lib/active_support/inflector/methods.rb:238:in `block in constantize'
     # ./vendor/gems/activesupport-4.1.4/lib/active_support/inflector/methods.rb:236:in `each'
     # ./vendor/gems/activesupport-4.1.4/lib/active_support/inflector/methods.rb:236:in `inject'
     # ./vendor/gems/activesupport-4.1.4/lib/active_support/inflector/methods.rb:236:in `constantize'
     # ./vendor/gems/activesupport-4.1.4/lib/active_support/core_ext/string/inflections.rb:66:in `constantize'
     # ./vendor/gems/activerecord-4.1.4/lib/active_record/associations/belongs_to_polymorphic_association.rb:7:in `klass'
     # ./vendor/gems/activerecord-4.1.4/lib/active_record/associations/belongs_to_association.rb:37:in `find_target?'
     # ./vendor/gems/activerecord-4.1.4/lib/active_record/associations/association.rb:138:in `load_target'
     # ./vendor/gems/activerecord-4.1.4/lib/active_record/associations/association.rb:53:in `reload'
     # ./vendor/gems/activerecord-4.1.4/lib/active_record/associations/singular_association.rb:9:in `reader'
     # ./vendor/gems/activerecord-4.1.4/lib/active_record/autosave_association.rb:287:in `validate_single_association'
     # ./vendor/gems/activerecord-4.1.4/lib/active_record/autosave_association.rb:209:in `block in add_autosave_association_callbacks'
     # ./vendor/gems/activerecord-4.1.4/lib/active_record/autosave_association.rb:157:in `instance_eval'
     # ./vendor/gems/activerecord-4.1.4/lib/active_record/autosave_association.rb:157:in `block in define_non_cyclic_method'
     # ./vendor/gems/activesupport-4.1.4/lib/active_support/callbacks.rb:424:in `block in make_lambda'
     # ./vendor/gems/activesupport-4.1.4/lib/active_support/callbacks.rb:184:in `call'
     # ./vendor/gems/activesupport-4.1.4/lib/active_support/callbacks.rb:184:in `block in simple'
     # ./vendor/gems/activesupport-4.1.4/lib/active_support/callbacks.rb:185:in `call'
     # ./vendor/gems/activesupport-4.1.4/lib/active_support/callbacks.rb:185:in `block in simple'
     # ./vendor/gems/activesupport-4.1.4/lib/active_support/callbacks.rb:185:in `call'
     # ./vendor/gems/activesupport-4.1.4/lib/active_support/callbacks.rb:185:in `block in simple'
     # ./vendor/gems/activesupport-4.1.4/lib/active_support/callbacks.rb:185:in `call'
     # ./vendor/gems/activesupport-4.1.4/lib/active_support/callbacks.rb:185:in `block in simple'
     # ./vendor/gems/activesupport-4.1.4/lib/active_support/callbacks.rb:86:in `call'
     # ./vendor/gems/activesupport-4.1.4/lib/active_support/callbacks.rb:86:in `run_callbacks'
     # ./vendor/gems/activemodel-4.1.4/lib/active_model/validations.rb:376:in `run_validations!'
     # ./vendor/gems/activemodel-4.1.4/lib/active_model/validations/callbacks.rb:111:in `block in run_validations!'
     # ./vendor/gems/activesupport-4.1.4/lib/active_support/callbacks.rb:82:in `run_callbacks'
     # ./vendor/gems/activemodel-4.1.4/lib/active_model/validations/callbacks.rb:111:in `run_validations!'
     # ./vendor/gems/activemodel-4.1.4/lib/active_model/validations.rb:317:in `valid?'
     # ./vendor/gems/activerecord-4.1.4/lib/active_record/validations.rb:70:in `valid?'
     # ./vendor/gems/shoulda-matchers-2.6.2/lib/shoulda/matchers/active_model/validation_message_finder.rb:61:in `validate_instance'
     # ./vendor/gems/shoulda-matchers-2.6.2/lib/shoulda/matchers/active_model/validation_message_finder.rb:57:in `validated_instance'
     # ./vendor/gems/shoulda-matchers-2.6.2/lib/shoulda/matchers/active_model/validation_message_finder.rb:53:in `errors'
     # ./vendor/gems/shoulda-matchers-2.6.2/lib/shoulda/matchers/active_model/validation_message_finder.rb:23:in `has_messages?'
     # ./vendor/gems/shoulda-matchers-2.6.2/lib/shoulda/matchers/active_model/allow_value_matcher.rb:256:in `has_messages?'
     # ./vendor/gems/shoulda-matchers-2.6.2/lib/shoulda/matchers/active_model/allow_value_matcher.rb:252:in `errors_match?'
     # ./vendor/gems/shoulda-matchers-2.6.2/lib/shoulda/matchers/active_model/allow_value_matcher.rb:222:in `block in matches?'
     # ./vendor/gems/shoulda-matchers-2.6.2/lib/shoulda/matchers/active_model/allow_value_matcher.rb:219:in `each
     # ./vendor/gems/shoulda-matchers-2.6.2/lib/shoulda/matchers/active_model/allow_value_matcher.rb:219:in `each'
     # ./vendor/gems/shoulda-matchers-2.6.2/lib/shoulda/matchers/active_model/allow_value_matcher.rb:219:in `none?'
     # ./vendor/gems/shoulda-matchers-2.6.2/lib/shoulda/matchers/active_model/allow_value_matcher.rb:219:in `matches?'
     # ./vendor/gems/shoulda-matchers-2.6.2/lib/shoulda/matchers/active_model/validation_matcher.rb:43:in `allows_value_of'
     # ./vendor/gems/shoulda-matchers-2.6.2/lib/shoulda/matchers/active_model/ensure_inclusion_of_matcher.rb:388:in `disallows_value_outside_of_array?'
     # ./vendor/gems/shoulda-matchers-2.6.2/lib/shoulda/matchers/active_model/ensure_inclusion_of_matcher.rb:326:in `matches_for_array?'
     # ./vendor/gems/shoulda-matchers-2.6.2/lib/shoulda/matchers/active_model/ensure_inclusion_of_matcher.rb:304:in `matches?'
     # ./vendor/gems/rspec-expectations-3.0.4/lib/rspec/expectations/handler.rb:48:in `handle_matcher'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/memoized_helpers.rb:81:in `should'
     # ./spec/models/job_spec_spec.rb:20:in `block (2 levels) in <top (required)>'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/example.rb:148:in `instance_exec'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/example.rb:148:in `block in run'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/example.rb:210:in `call'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/example.rb:210:in `block (2 levels) in <class:Procsy>'
     # ./vendor/gems/rspec-rails-3.0.2/lib/rspec/rails/adapters.rb:72:in `block (2 levels) in <module:MinitestLifecycleAdapter>'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/example.rb:294:in `instance_exec'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/example.rb:294:in `instance_exec'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/hooks.rb:430:in `block (2 levels) in run'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/example.rb:210:in `call'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/example.rb:210:in `block (2 levels) in <class:Procsy>'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/hooks.rb:432:in `run'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/hooks.rb:485:in `run'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/example.rb:303:in `with_around_example_hooks'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/example.rb:145:in `run'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/example_group.rb:494:in `block in run_examples'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/example_group.rb:490:in `map'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/example_group.rb:490:in `run_examples'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/example_group.rb:457:in `run'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/runner.rb:112:in `block (2 levels) in run_specs'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/runner.rb:112:in `map'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/runner.rb:112:in `block in run_specs'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/reporter.rb:54:in `report'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/runner.rb:108:in `run_specs'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/runner.rb:86:in `run'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/runner.rb:70:in `run'
     # ./vendor/gems/rspec-core-3.0.4/lib/rspec/core/runner.rb:38:in `invoke'
     # ./vendor/gems/rspec-core-3.0.4/exe/rspec:4:in `<top (required)>'
     # ./vendor/bin/rspec:23:in `load'
     # ./vendor/bin/rspec:23:in `<main>'
mcmire commented 10 years ago

Ah. So it looks like it's failing because the attribute in question is polymorphic and thus ActiveRecord expects any value for this attribute to be a known class. I'll mark this as a bug, but in the meantime you can use allow_value to get around this:

it { should allow_value(JobSpec::JOB_TEMPLATE_TYPES).for(:job_template_type) }

it do
  all_other_models = ActiveRecord::Base.subclasses.map(&:to_s) - JobSpec::JOB_TEMPLATE_TYPES
  should_not allow_value(*all_other_models).for(:job_template_type)
end
cindyward1 commented 9 years ago

Using Ruby 2.1.5, Rails 4.2.0, and Shoulda-Matchers 2.8.0, shoulda-matchers also fails trying to validate_inclusion_of [:user] for resource_type in the popular 'rolify' gem. The Role class:

class Role < ActiveRecord::Base has_and_belongs_to_many :users, join_table: :users_roles belongs_to :resource, polymorphic: true validates :resource_type, inclusion: { in: [:user] }, allow_nil: true scopify end

My attempted validation of validates_inclusion_of(:resource_type): it { should validate_inclusion_of(:resource_type).in_array([nil, "user"]) }

The error message I get (it doesn't matter whether or not I include nil in the array): Failures: 1) Role should ensure inclusion of resource_type in [nil, "user"] Failure/Error: it { should validate_inclusion_of(:resource_type).in_array([nil, "user"]) } [nil, "user"] doesn't match array in validation

(If I use only ["user"], the error I get is the expected ["user"] doesn't match array in validation)

I added some puts in the shoulda-matcher code to try to figure out what was going on: allows_all_values_in_array @array = [nil, "user"] allows_all_values_in_array: @low_message = nil allows_all_values_in_array: true, , allows_all_values_in_array: @low_message = nil allows_all_values_in_array: false, user, matches_for_array: false true true true

(If I use only ["user"], the output is: allows_all_values_in_array @array = ["user"] allows_all_values_in_array: @low_message = nil allows_all_values_in_array: false, user, matches_for_array: false true true true)

My next attempted validation using allows_value doesn't work either: Failures: 1) Role should allow resource_type to be set to "user" Failure/Error: it { should allow_value("user").for(:resource_type) } Did not expect errors when resource_type is set to "user", got errors: * "is not included in the list" (attribute: resource_type, value: "user")

(Note that "Role should allow resource_type to be set to nil" does validate correctly, as would be expected from the output above.)

mcmire commented 9 years ago

@cindyward1 Doesn't resource_type take the name of a class as a value, and not a symbol? What if you say:

validates :resource_type, inclusion: { in: ['User'] }, allow_nil: true

and then to test it:

should validate_inclusion_of(:resource_type).in_array('User').allow_nil
cindyward1 commented 9 years ago

What you sent me works fine and explains why resource_type is always NULL in my development database. I misunderstood how to modify the Role class. Thanks for finding my bug ... Cindy

 On Tuesday, February 17, 2015 9:18 AM, Elliot Winkler <notifications@github.com> wrote:

@cindyward1 Doesn't resource_type take the name of a class as a value, and not a symbol? What if you say:validates :resource_type, inclusion: { in: ['User'] }, allow_nil: trueand then to test it:should validate_inclusion_of(:resource_type).in_array('User').allow_nil — Reply to this email directly or view it on GitHub.

NamrataSoni20 commented 7 years ago

Try this , It's works for me it do should validate_inclusion_of(:resource_type) .in_array(Rolify.resource_types) .allow_nil(true) end

amnesia7 commented 7 years ago

@mcmire I think I'm hitting this issue as well. I'm using ruby v2.4.1, rails v5.1.4 and shoulda-matchers v3.1.2

class Favourite < ApplicationRecord
  belongs_to :user
  belongs_to :favourable, polymorphic: true

  validates_presence_of :user_id, :favourable_id, :favourable_type
  validates :favourable_type, inclusion: { in: %w[Project],
                                           message: "%{value} can't be favourited" }

Rspec tests:

  context 'validation' do
    let!(:user) { create(:user) }
    let!(:project) { create(:project) }
    let!(:favourite_project) { user.favourites.build(favourable: project) }

    context 'favourable type' do
      specify 'valid type' do
        expect(favourite_project).to validate_inclusion_of(:favourable_type)
          .in_array(%w[Project])
      end
    end
  end

Error thrown:

     NameError:
       wrong constant name shoulda-matchers test string
mcmire commented 7 years ago

@amnesia7 Cool, thanks for reporting this :) This is definitely something we need help fixing.

amnesia7 commented 7 years ago

Let me know if you need any more info or need anything testing, or if there's anything straight forward that I can help with to get this resolved.

Col

On Thu, 26/10/17, Elliot Winkler notifications@github.com wrote:

Subject: Re: [thoughtbot/shoulda-matchers] ensure_inclusion_of does not work with polymorphic attributes (#574) To: "thoughtbot/shoulda-matchers" shoulda-matchers@noreply.github.com Cc: "amnesia7" col.gibson@gmail.com, "Mention" mention@noreply.github.com Date: Thursday, 26 October, 2017, 21:12

@amnesia7 Cool, thanks for reporting this :) This is definitely something we need help fixing.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

cickes commented 5 years ago

Still present in 2019. Will see if I can help.

mcmire commented 4 years ago

Hey folks. In an effort to lighten our load as maintainers and be able to serve you better in the future, the shoulda-matchers team is working on cleaning out the cobwebs in this repo by pruning the backlog. As there are few of us, there are a lot of items that will simply never earn our attention in a reasonable time frame, and rather than giving you an empty promise, we think it makes more sense to focus on more recent issues. That means, unfortunately, that we must close this issue.

Don't take this the wrong way: our aim is not to diminish the effort people have made or dismiss problems that have been raised. If you feel that we should reopen this issue, then please let us know so that we can reprioritize it. Thanks!

lucascarrias commented 3 years ago

Any update regarding this issue?

mcmire commented 3 years ago

No update, but if you can come up with a fix for this then we'd be happy to merge it in!

danderozier commented 3 years ago

If you're testing a lot of models with polymorphic associations, you can use rspec helpers to DRY things up a bit:

# spec/support/model_helpers.rb

module ModelHelpers
  def all_models_except(*models)
    ApplicationRecord.subclasses.map(&:to_s) - models
  end
end

# spec/models/your_model.rb

it { should_not allow_value(*all_models_except('Foo', 'Bar').for(:whatever_type) }