How to prevent NoMethodError in validation which have an if statement ?

[guppy0356]

I saw #1198 and I faced same issue too. Do we really have to use early return in the method? This is depends on shouda-matcher not depends on its spec.

Even if I set up the association correctly using subject, it is still nil in the test. How to prevent NoMethodError in testing. Is there nothing I can do? Early return is NOT good method to avoid this.

class WheelTest < ActiveSupport::TestCase
  context :association do
    subject do
      car = cars(:one)
      Wheel.new(car: car)
    end

    should belong_to(:car)
  end
end

class Wheel
  belongs_to :car 
  validates_presence_of :colour, if: :car_requires_coloured_wheels?

  def car_requires_coloured_wheels?
    return false unless car # no need to do this

    car.requires_coloured_wheels?
  end
end

Appendix

gem 'shoulda', '~> 4.0', group: :test
gem 'shoulda-matchers', '~> 4.3.0', group: :test

[mcmire]

Hi @guppy0356,

The short answer is yes, you still have to check for the presence of car.

You can think about this two ways. First, belongs_to implicitly places a validation on the association. So it's like your model has validates_presence_of :car on it. Because of this, the belong_to matcher double-checks this validation. It does this by setting car to nil and confirming that your model has errors. When it does this, your other validation runs, and if you did not have a check for car then it would raise an error.

Second, even if there were not a validation on car, it is totally possible for car to be nil -- nothing is preventing that -- so any validations you have that depend on car need to make sure that car is not nil. Otherwise, you would get a surprising error that would crash your application.

Hope that helps.

[guppy0356]

Thank you for your respond @mcmire,

Hmm, your answer is my sample code is prefer to early return, isn't it?

[mcmire]

@guppy0356 Yeah -- or some kind of logic at least. You could also use the &. operator if you want to shorten it:

  def car_requires_coloured_wheels?
    car&.requires_coloured_wheels?
  end

[guppy0356]

@mcmire
Okay, I understand very well. How about pinning this issue? I think people would be interested.

[gilesdotcodes]

Late to the party here, but have been banging my head against a wall with this issue.

Note: I'm going to intentional not comment or add opinions on whether associations should be tested. Or whether it's good to have validations that rely on values from other records… sometimes we have what we have! 😂

But I'm not overly keen on approaches that use a guard clause, or try, or the safe navigation operator (&.) because it feels (to me) like code that is there just to make the specs pass. It also - imo - reduces readability, because a developer has to second guess why a required association could be nil.
Another solution in this scenario, is to not use shoulda-matchers when you have validations that are dependent on other records.

It could be solved with:

it "should belong to a car" do
  association = described_class.reflect_on_association(:car)

  expect(association.macro).to eq(:belongs_to)
  expect(association.options[:optional]).to eq(false)
end

Obviously this is a lot more verbose and goes against using the gem consistently though. But thought I'd leave this here in case it helps someone out.

I sort of understand why shoulda are validating the association "properly"…I think it is essentially doing model.new.valid?…but for some of the codebases we live and work in, that isn't reflective of the real world environment where records are initialised with required associations.

[gilesdotcodes]

Me again! Another solution…for when you do want to use shoulda-matchers 😂

I think this should work in your scenario!

let(:car) { build_stubbed(:car) }

before do
  allow_any_instance_of(described_class).to receive(:car).and_return(car)
end

should belong_to(:car)