Shoulda-matchers: Matcher to test if model is using a custom validator class

[adilsoncarvalho]

Hi everyone,

I'm using on a project custom classes to validate models and I also need to group those validations depending on the role of the user that is responsible for its creation. The code looks like this:

class CommonAreaSchedule < ActiveRecord::Base
  # global validations
  validates_with CommonAreaSchedule::BookingDatesValidator

  # specific 
  with_options unless: :syndic_or_admin? do |resident|
    resident.validates_with CommonAreaSchedule::DebtValidator
  end
end

I covered the validator class with tests to ensure that it's ok but I would like an easy way to check if it's set on the model, something like this

describe CommonAreaSchedule, type: :model do
  it { is_expected.to validate_with_class(CommonAreaSchedule::BookingDatesValidator) }
  it { is_expected.to validate_with_class(CommonAreaSchedule::DebtValidator).with_arguments(unless: :syndic_or_admin?) }
end

I already wrote it to use on my code and I want to ask if a PR with it would be interesting to be part of the shoulda matchers.

Regards

[marcocarvalho]

I liked! :+1:

[maurogeorge]

Hi @adilsoncarvalho I like the idea.

What you think about validate_with over validate_with_class?

I don't think the with_arguments qualifier is needed, since it is a value passed to the with_options not to the matcher. I think we can have the same behavior defining a subject like:

describe CommonAreaSchedule, type: :model do
  it { is_expected.to validate_with_class(CommonAreaSchedule::BookingDatesValidator) }
  it { is_expected.to_not validate_with_class(CommonAreaSchedule::DebtValidator) }

  describe 'syndic_or_admin' do
    context 'when is a syndic' do
      subjet do
        # build a syndic.
      end

      it { is_expected.to validate_with_class(CommonAreaSchedule::DebtValidator) }
    end

    context 'when is a admin' do
      subjet do
        # build a admin.
      end

      it { is_expected.to validate_with_class(CommonAreaSchedule::DebtValidator) }
    end
  end
end

Make sense to you?

Thanks

[adilsoncarvalho]

As a matter of fact adopting validate_with seems better as it keeps the same name used on the model validation @maurogeorge.

I am not sure if I got the second part. My intention was to just test the way the validator was tested without having to actually run it. I used the with_arguments because that's the way delegate_method does when we use the delegate :from, to: :to, allow_nil: true to address to the allow_nil: true part.

The code I created just go to the model validation definitions and check if such a declaration was made, like this:

def matches?(subject)
  @subject = subject

  class_under_test.validators.find_all do |validator|
    validator.class == klass && validator.options == arguments
  end.one?
end

Your suggestion seems to want to actually exercise the validator, isn't it or did I got it wrong?

[maurogeorge]

@adilsoncarvalho my idea is to follow the same logic we have on all other matchers, today all validation matchers can be tested against the with_options, like validates_presence_of, validates_numericality_of etc. But any of these matchers doesn't have a specific qualifier to handle this, we need to build the object ourselves like the example I send before.

Makes sense to you?

[adilsoncarvalho]

I was fooling around the shoulda-matchers source code and thinking about your point @maurogeorge.

I think the way I wrote makes it easier to test by querying the declaration of the validators but I do see the value of maintaining the expected behaviour across the code.

I'll try to rewrite the way my matcher works to see how it would be on real code. I'll bring news soon :bowtie:

[maurogeorge]

@adilsoncarvalho cool! Waiting for the news

[mcmire]

@adilsoncarvalho I don't understand what this matcher would be doing. Would it be testing a validator is defined on the model? If so, don't you think that would be implicitly covered by testing the validation itself?

[mcmire]

@adilsoncarvalho Sorry, but I don't think this is something we're interested in adding to shoulda-matchers. If you'd like to create your own gem and release it yourself, we're all for that, but I don't think this is something that we'd personally use at thoughtbot. If it were me I'd probably make a matcher that would run the validation. Why? Well, that's the philosophy we follow for Rails' built-in matchers. For instance, if I have a model that validates the presence of an attribute, I _could_ check that the model has the presence validation within Model._validators. But I'd argue that this wouldn't be a great test. I'm more interested in the actual behavior of the model. What if the validation has an :if option on it? Now you have two kinds of behavior to consider. I feel like that difference of behavior wouldn't be properly exercised by a simple check to see if that validation is present within the validators array. I hope that makes sense.

[rmcsharry]

I agree with @mcmire that what the OP was trying does not make sense since it does not test the behavior of the model.

I arrived here because I was looking for something similar at the attribute level, since that is where behaviour resides for custom validators.

To see what I mean I have put a detailed question on SO here.