Define subject when using Shoulda-Matchers' validate_uniqueness_of

[c-lliope]

Can we revisit #133 with respect to thoughtbot/shoulda-matchers#567 ?

I just ran into the Non-NULL database column problem, where validates_uniqueness_of didn't know how to create a valid record.

Here's what the guides suggested my specs look like:

  describe "Validations" do
    it "validates uniqueness of email" do
      create(:registration)

      expect(Registration.new).to validate_uniqueness_of(:email)
    end

    it "validates uniqueness of mobile phone number" do
      create(:registration)

      expect(Registration.new).to validate_uniqueness_of(:mobile_phone_number)
    end

    it { is_expected.to validate_presence_of(:email) }
    it { is_expected.to validate_presence_of(:mobile_phone_number) }
  end

And here's what it looks like with an explicit subject:

  describe "Validations" do
    subject { build(:registration) }

    it { is_expected.to validate_presence_of(:email) }
    it { is_expected.to validate_presence_of(:mobile_phone_number) }
    it { is_expected.to validate_uniqueness_of(:email) }
    it { is_expected.to validate_uniqueness_of(:mobile_phone_number) }
  end

I would suggest that the second option is more readable and more manageable, with less duplication. Thoughts on allowing an explicit subject for use with shoulda-matchers?

[jessieay]

👍 second example looks obviously better.

I guess an alternative would be a PR to shoulda that makes this obsolete

[gylaz]

I was under the impression that the second example wouldn't work for uniqueness due to shoulda-matchers not creating a record before trying to see if another can be created. If however, the second example just works, then I'm 👍 on this.

[jessieay]

@gylaz the reason that the second example works, at least as far as I understand, is that otherwise shoulda uses an implicit subject of Object.new.

In that case, shoulda is not smart enough to use valid attributes for the object, whereas FG.build is. But I could be wrong on that...

[jferris]

Neither of these examples is using an explicit subject. By "explicit subject" the guideline means literally using the method subject within an it block:

describe "#author_name" do
  let(:author) { User.new(name: "Billy") }
  subject { Post.new(author: author) }

  it "returns its author name" do
    expect(subject.author_name).to eq(author.name)
  end
end

The idea is that "subject" is only useful if it's so obvious which object you're talking about that you don't need to call out the subject. If you need to reference it explicitly, you can use a name instead. If you're using is_expected_to, you're using subject as intended.

The change linked to for #133 was about should vs is_expected_to. If you replace the method is_expected_to with should, you're following the guideline in both of your examples. This was simply a preference for one alias over another, as we generally found should to be more readable.

Re: validates_uniqueness_of - I thought you would have to create the registration and not build it, so that shoulda could properly attempt a save with an existing attribute value. I don't think that's related to the explicit subject guideline, though.

[jessieay]

@jferris I am not sure why Grayson's example with the subject { build(:registration) } works for the validates_uniqueness_of matcher...but it does.

@Graysonwright do you have more insight into why including the subject inside the describe block fixed the shoulda issue with `validates_uniqueness_of`` that occurs without a subject?

[c-lliope]

Ah, thanks @jferris – I misunderstood.

I had assumed that "explicit subject" referred to defining the subject explicitly, not to using it explicitly. That makes a lot more sense now. To confirm, we should be using this syntax, then?

  describe "Validations" do
    subject { build(:registration) }

    it { should validate_presence_of(:email) }
    it { should validate_presence_of(:mobile_phone_number) }
    it { should validate_uniqueness_of(:email) }
    it { should validate_uniqueness_of(:mobile_phone_number) }
  end

[c-lliope]

It looks like validate_uniqueness_of now uses the subject as the original record, and saves it in the database. It then creates a new object using subject.class.new and validates the uniqueness of that one.

See https://github.com/thoughtbot/shoulda-matchers/pull/567/files#diff-d732d58560615deb1ccbca127d2ba736R208

[jferris]

To confirm, we should be using this syntax, then?

Yes, that looks correct to me.

It looks like validate_uniqueness_of now uses the subject as the original record, and saves it in the database.

Cool; that it explains why build works now.

[jessieay]

@Graysonwright are you going to open a PR for this? Would be good to have a comment explaining next steps :)

[TorvaldsDB]

require 'rails_helper'

RSpec.describe Score, type: :model do
  let(:score) { build(:score) }
  subject { described_class }

  it { is_expected.to validate_numericality_of(:score) }

  it 'belongs to staff' do
    assoc = subject.reflect_on_association(:staff)

    expect(assoc.macro).to eq :belongs_to
  end
end

This is my codes, but I occurred the issues, looks like this:

» rspec spec/models/score_spec.rb                                                                                                                             [ruby-2.5.1][19:04:09]
F

Failures:

  1) Score should validate that :score looks like a number
     Failure/Error: it { is_expected.to validate_numericality_of(:score) }

     Shoulda::Matchers::ActiveModel::AllowValueMatcher::AttributeDoesNotExistError:
       The matcher attempted to set :score on the Class to "abcd", but that
       attribute does not exist.

my Score looks like this:

class Score < ApplicationRecord
  belongs_to :staff

  validates :score, numericality: { greater_than_or_equal_to: 0, less_than_or_equal_to: 5 }
end

why? and Thank you very much.

[jessieay]

@TorvaldsDB when you set subject { described_class } that is for the Score class , but you should always test validations on an instance of a class. So if you change that to subject { described_class.new } it should pass or give you a different error at least.

In Grayson's example above, he sets subject { build(:registration) } - this is shorthand for subject { FactoryBot.build(:registration) }, which is creating an instance of the Registration class.

We frequently use FactoryBot (https://github.com/thoughtbot/factory_bot) in test suits to easily create objects for tests. You might consider giving it a try.