Should we test rails attributes?-CodePudding

In Rails models we usually have attributes and relations tests, like:

describe 'attributes' do
  it { is_expected.to have_db_column(:identifier).of_type(:uuid) }
  it { is_expected.to have_db_column(:content).of_type(:jsonb) }
  it { is_expected.to have_db_column(:created_at).of_type(:datetime) }
end

describe 'relations' do
  it { is_expected.to belong_to(:user).class_name('User') }
end

And using a TDD style it seems to be some useful tests, however I have been dwelling if these are really necessary tests, and I would like to know if there is some common knowledge about it, is it good practice to create these tests? or are we just testing rails?

CodePudding user response：

Amongst the purposes of a unit test are...

Does it work?
Does it still work?

If it's a promise, if other things rely on it, you should test it to ensure you keep that promise. This is regression testing.

But don't test more than you promise. You'll be stuck with it, or your code will break when you make an internal change.

For example...

  it { is_expected.to have_db_column(:identifier).of_type(:uuid) }

This promises that it has a column called identifier which is a UUID. Usually you don't promise all that detail; it is glass-box testing and it makes your test brittle.

Instead, promise as little as you can. Its ID is a UUID. This is black-box testing.

require "rspec/uuid"

describe '#id' do
  subject { thing.id }
  let(:thing) { create(:thing) }

  it 'has a uuid ID' do
    expect(thing.id).to be_a_uuid
  end
end

It's possible there is an even higher level way to express this without holding yourself specifically to a UUID.

  it { is_expected.to have_db_column(:content).of_type(:jsonb) }

Similarly, don't promise it has a jsonb column. That is blackbox testing. Promise that you can store complex data structures.

describe '#content' do
  subject { create(:thing) }

  it 'can round trip complex data' do
    data = [1, { two: 3, four: [5] }]

    thing.update!(content: data)

    # Force it to re-load content from the database.
    thing.reload

    expect(thing.content).to eq data
  end
end

  it { is_expected.to belong_to(:user).class_name('User') }

Instead of promising what it belongs to, promise the relationship.

describe '#user' do
  let(:thing) { create(:thing) }
  let(:user) { create(:user) }

  before {
    user.things << thing
  }

  it 'belongs to a user' do
    expect(thing.user).to eq user
    expect(user.things).to contain(thing)
  end
end

CodePudding user response：

I have answered a nearly identical question here: https://stackoverflow.com/a/74195850/14837782

In summary: If it is end-developer code, I believe it should be tested. If it can be fat-fingered, I believe it should be tested. If you're going to remove it deliberately, I also believe you should have to remove a test deliberately as well. If it can fail, there should be a specific test for that failure mode.

This is not to be confused with testing the Rails framework. You obviously want to design your tests so that you're not testing Rails itself or Rails implementation, only your own code.

Attributes should be tested. Here is how I do it in minitest:

test/models/car_test.rb

class CarTest < ActiveSupport::TestCase

  ###################################################################
  #
  # Attributes
  #
  ###################################################################
  test 'describe some attr_reader fields' do
    expected = [:year, :make, :model, :vin]
    assert_has_attr_readers(Car, expected)
  end

  ###############################################
  test 'describe some attr_writer fields' do
    expected = [:infotainment_fimrware_version]
    assert_has_attr_writers(Car, expected)
  end

  ###############################################
  test 'describe some attr_accessor fields' do
    expected = [:owner, :color, :mileage]
    assert_has_attr_readers(Car, expected)
    assert_has_attr_writers(Car, expected)
  end
end

test/test_helpers/attributes_helper.rb

# frozen_string_literal: true

module AttributesHelper

  ###################################################################
  #
  # Assertions
  #
  ###################################################################

  #
  # Performs an assertion that the given class contains reader/getter methods for the given attribute names.
  # This helper checks for the existence of `attribute_name` methods on the class, and does not concern itself
  # with how those methods are declared: directly defined, attr_reader, attr_accessor, etc.
  #
  def assert_has_attr_readers(klass, attribute_names)

    # Get public and protected method names, passing `false` to exclude methods from super classes.
    actual_method_names = klass.instance_methods(false).map(&:to_s)

    attribute_names.each do |attribute|
      message = "Expected class #{klass.name} to contain a reader for attribute #{attribute}"
      assert_includes(actual_method_names, attribute.to_s, message)
    end
  end

  #
  # Performs an assertion that the given class contains writer/setter methods for the given attribute names.
  # This helper checks for the existence of `attribute_name=` methods on the class, and does not concern itself
  # with how those methods are declared: directly defined, attr_writer, attr_accessor, etc.
  #
  def assert_has_attr_writers(klass, attribute_names)

    # Get public and protected method names, passing `false` to exclude methods from super classes.
    actual_method_names = klass.instance_methods(false).map(&:to_s)

    attribute_names.each do |attribute|
      message = "Expected class #{klass.name} to contain a writer for attribute #{attribute}"
      assert_includes(actual_method_names, "#{attribute}=", message)
    end
  end

  #
  # Performs an assertion that the given class implements attr_encrypted for the given attribute names.
  # This helper is tied to the implementation details of the attr_encrypted gem. Changes to how attributes
  # are encrypted will need to be accounted for here.
  #
  def assert_has_encrypted_attrs(klass, attribute_names)
    message           = "Expected class #{klass.name} to encrypt specific attributes"
    actual_attributes = klass.encrypted_attributes.keys

    assert_equal(attribute_names.map(&:to_s).sort, actual_attributes.map(&:to_s).sort, message)
  end
end

Your example tests seem to be testing the existence of DB fields, not getter/setter model attributes. Database fields are impossible to fat-finger (they require a migration to modify) so if that's what you're talking about, I do not believe it makes sense to test them. (And I personally believe it is useful to test nearly everything.)

Although I guess in the case where the DB is accessible by other applications and could potentially be modified outside of a single application then it could make sense to test for the existence of those fields as well, as pointed out by Dave Newton in a comment below.

Ultimately it is up to you, and if your one application is the only one with access to the DB but you still want to test DB field existence and settings, maybe a 3rd option is some sort of migration test that you're looking for to make sure the migration is written properly. I've not written anything like that yet, but it might be feasible. I would hate to try to write one, and it does seem to go too far, but it's an idea...