thoughtbot / shoulda-matchers

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

Issue with belongs_to having a delegate called in validations 4.0.1 #1185

Closed ArcDlrt closed 5 years ago

ArcDlrt commented 5 years ago

After upgrading the gem from 3.1.2 to 4.0.1 the following spec failded

require 'rails_helper'

RSpec.describe RiskVariable, type: :model do
  context 'Associations' do

    it { should belong_to(:profile_risk_variable) }

  end
end

The result of running this spec is an undefined methodcriteria_type' for nil:NilClass`

0) RiskVariable Associations should belong to profile_risk_variable required: true
     Failure/Error: delegate :criteria_type, to: :profile_risk_variable

     Module::DelegationError:
       RiskVariable#criteria_type delegated to profile_risk_variable.criteria_type, but profile_risk_variable is nil:

This is the relevant code for the example given

class RiskVariable < ApllicationRecord
  belongs_to :profile_risk_variable

  validates :range_type, numericality: true, if: :range_criteria?

  delegate :criteria_type, to: :profile_risk_variable

  def range_criteria?
    criteria_type == 'range'
  end

end
class ProfileRiskVariable < ApplicationRecord
  has_many :risk_variables, dependent: :destroy

  CRITERIA_TYPE_LIST = ['range', 'fixed']

  validate :criteria_type, inclusion: {  in: CRITERIA_TYPE_LIST } 
end

This was run using RoR 5.2.0, Ruby 2.4.3 and shoulda-matchers 4.0.1

mcmire commented 5 years ago

You're getting this because the belong_to matcher now tests whether your association is allowed to be nil. If you have some sort of action that attempts to create a ProfileRiskVariable without profile_risk_variable_id being set, you will likely run into the same error — the matcher is just revealing a code path that you may have never executed before.

It looks like in your range_criteria? method, criteria_type is delegating to profile_risk_variable, but if profile_risk_variable is nil, this won't work because you can't call profile_risk_variable on nil. You might want to add allow_nil: true to the delegate line.

ArcDlrt commented 5 years ago

I see, it make sense since now the relationship is by default required.

Thank you very much

mcmire commented 5 years ago

No problem!

davidmyersdev commented 2 years ago

For others that land on this thread, you can also use the without_validating_presence modifier to prevent the nil check from running when you know it won't be nil.

it { should belong_to(:profile_risk_variable).without_validating_presence }