March 28, 2018

One thing I've discovered about being (1) self-taught and (2) the only developer in my workplace is that when I learn a new technique, I'm never sure how common it is. Recently, I was discussing some tests that I wrote with a more experienced developer, and it turned out that the technique I used was one he hadn't seen before.

Some background

When I wrote these tests, I was in the early stages of building a large app for the music school where I work. At this time, we knew very little about what we wanted as far as user interface, screens, happy path, and so on. However, since the school is very well-established and the app is being built to support our current practices, I knew very well what data we needed to model and how the models should relate to one another. This put me in a strange position where, instead of building features top-to-bottom, my only path to productivity was to build our data models. This led to me creating several dozen models that don't do much of anything yet, but contain the structure of our data to support future feature development.

At this point, I had yet to use Test-Driven Development in any form, so I sought to employ it as I created these models. Most particularly, I wanted to use it to make sure the associations between my models were correct. I used the default Rails testing stack of Minitest with fixtures, no Rspec or factories.

The Problem

When making this many models, it would be easy to accidentally omit an association on one model that is present on an associated model, like so:

# app/models/parent.rb
has_many: :children

# app/models/child.rb
# OOPS! Nothing here. Should say has_one: :parent

I wanted to be able to write this in my tests:

class ParentTest < ActiveSupport::TestCase
  test "Parent associations" do
    check_bidirectional_associations Parent
  end
end

class ChildTest < ActiveSupport::TestCase
  test "Child associations" do
    check_bidirectional_associations Child
  end
end

I found an ActiveRecord method to help me: Model.reflect_on_all_associations (docs). This method would return an array of associations defined on the current model. Since I was using fixtures in my tests anyway, I could get the first instance of the associated model from the fixture file, then check to see if it had the inverse association defined on it as a method.

First, I have to make sure there is an instance of the model in the fixture file. Models are called in tests by using the model name as a singular constant. Example: SomeModel.first, not some_model.first or SomeModels.first

def check_bidirectional_associations(model)
  associations = model.reflect_on_all_associations
  associations.each do |assoc|
    # attempt to assign the first instance of model to a variable. Must first convert assoc[:name] to singular constant.
    assert associated_object = assoc.name.to_s.singularize.camelize.constantize.first
  end
end

Now, if there is no fixture, the test will fail. It needs an error message, though.

assert associated_object = assoc.name.to_s.singularize.camelize.constantize.first,
       "#{assoc.name.to_s.camelize} not present in fixtures. Check test/fixtures/#{assoc.name.to_s.pluralize.underscore}.yml"

That's better. Now if the fixture is missing, the test tells me which file to fix. On to the next step, checking the association. First, I need the model name as a method:

model_method = model.to_s.underscore

Now to see if the associated_object responds to model_method:

assert associated_object.respond_to?(model_method)

Of course, some associations are plural. Better make it:

assert associated_object.respond_to?(model_method) || associated_object.respond_to?(model_method.pluralize)

Better. Now for the error message when the test fails:

assert associated_object.respond_to?(model_method) || associated_object.respond_to?(model_method.pluralize),
       "#{associated_object.class.to_s.camelize} is not properly associated with #{model.to_s}. Check for the correct association in app/models/#{assoc.name.to_s.underscore}.rb."

Awesome! Now I have a test that will tell me when the relationship between two models is only defined on one of them. Here's the method, defined in test_helper.rb:

# test/test_helper.rb
class ActiveSupport::TestCase
  def check_bidirectional_associations(model)
    associations = model.reflect_on_all_associations
    associations.each do |assoc|
      assert associated_object = assoc.name.to_s.singularize.camelize.constantize.first,
             "#{assoc.name.to_s.camelize} not present in fixtures. Check test/fixtures/#{assoc.name.to_s.pluralize.underscore}.yml"
      model_method = model.to_s.underscore
      assert associated_object.respond_to?(model_method) || associated_object.respond_to?(model_method.pluralize),
             "#{associated_object.class.to_s.camelize} is not properly associated with #{model.to_s}. Check for the correct association in app/models/#{assoc.name.to_s.underscore}.rb."
    end
  end
end

Now it gets complicated

Of course, ActiveRecord has many types of associations. The test above won't work for all of them. Some cases are best skipped:

associations.each do |assoc|

  # not necessarily bidirectional
  next if assoc.options[:through].present?

  # polymorphic has_many or has_one
  next if assoc.options[:as].present?

  # polymorphic belongs_to
  next if assoc.options[:polymorphic].present?

  # test it

  end
end

And others need modification:

associations.each do |assoc|

  # stuff from previous example

  # self join (model has an association to the same model)
  if assoc.klass == model
    assert assoc.options[:inverse_of].present?,
           "Only half of a self-join association is defined in app/models/#{model.to_s.underscore}.rb. The inverse of :#{assoc.name} is not defined."
    assert model.first.respond_to? assoc.options[:inverse_of],
           "The inverse of :#{assoc.name} is not functioning correctly. Check app/models/#{model.to_s.underscore}.rb."

  # Explicitly defined inverse association. Must have an inverse defined in both models.
  elsif assoc.options[:inverse_of].present?
    assert associated_object = assoc.klass.first,
           "#{assoc.name.to_s.camelize} not present in fixtures. Check test/fixtures/#{assoc.name.to_s.pluralize.underscore}.yml.\nAssociation name: #{assoc.name}\nAssociated object: #{assoc.klass}"
    model_method = assoc.options[:inverse_of].to_s
    assert associated_object.respond_to?(model_method) || associated_object.respond_to?(model_method.pluralize),
           "#{associated_object.class.to_s.camelize} is not properly associated with #{model.to_s} via :#{model_method}. Check for the correct association in app/models/#{model.to_s.underscore}.rb."

  # Using an alias to reference another model. Should probably have inverse relationships explicitly defined.
  elsif assoc.name != assoc.klass
    assert associated_object = assoc.klass.first,
           "#{assoc.name.to_s.camelize} not present in fixtures. Check test/fixtures/#{assoc.name.to_s.pluralize.underscore}.yml"
    model_method = model.to_s.underscore
    assert associated_object.respond_to?(model_method) || associated_object.respond_to?(model_method.pluralize),
           "#{associated_object.class.to_s.camelize} is not properly associated with #{model.to_s} using :#{model_method} as the inverse of :#{assoc.name}. Check for the correct associations and :inverse_of attributes in app/models/#{assoc.klass.to_s.underscore}.rb and app/models/#{model.to_s.underscore}.rb."

  # Normal bidirectional association
  else
    # do the stuff from earlier
 end
end

The Good

This method was extremely helpful to me in defining my data structures. I was able to develop quickly, knowing that my tests would warn me if I didn't get my associations right, with helpful error messages about which files to check. All I had to do was add check_bidirectional_associations ModelName to each of my model test files.

The Less Good

That is a huge conditional statement! No really, I didn't even put the whole method up yet. Here:

def check_bidirectional_associations(model)
  associations = model.reflect_on_all_associations
  associations.each do |assoc|
    # not necessarily bidirectional
    next if assoc.options[:through].present?

    # polymorphic has_many or has_one
    next if assoc.options[:as].present?

    # polymorphic belongs_to
    next if assoc.options[:polymorphic].present?

    # self join
    if assoc.klass == model
      assert assoc.options[:inverse_of].present?,
             "Only half of a self-join association is defined in app/models/#{model.to_s.underscore}.rb. The inverse of :#{assoc.name} is not defined."
      assert model.first.respond_to? assoc.options[:inverse_of],
             "The inverse of :#{assoc.name} is not functioning correctly. Check app/models/#{model.to_s.underscore}.rb."

    # Explicitly defined inverse association. Must have an inverse defined in both models.
    elsif assoc.options[:inverse_of].present?
      assert associated_object = assoc.klass.first,
             "#{assoc.name.to_s.camelize} not present in fixtures. Check test/fixtures/#{assoc.name.to_s.pluralize.underscore}.yml.\nAssociation name: #{assoc.name}\nAssociated object: #{assoc.klass}"
      model_method = assoc.options[:inverse_of].to_s
      assert associated_object.respond_to?(model_method) || associated_object.respond_to?(model_method.pluralize),
             "#{associated_object.class.to_s.camelize} is not properly associated with #{model.to_s} via :#{model_method}. Check for the correct association in app/models/#{model.to_s.underscore}.rb."

    # Using an alias to reference another model. Should probably have inverse relationships explicitly defined.
    elsif assoc.name != assoc.klass
      assert associated_object = assoc.klass.first,
             "#{assoc.name.to_s.camelize} not present in fixtures. Check test/fixtures/#{assoc.name.to_s.pluralize.underscore}.yml"
      model_method = model.to_s.underscore
      assert associated_object.respond_to?(model_method) || associated_object.respond_to?(model_method.pluralize),
             "#{associated_object.class.to_s.camelize} is not properly associated with #{model.to_s} using :#{model_method} as the inverse of :#{assoc.name}. Check for the correct associations and :inverse_of attributes in app/models/#{assoc.klass.to_s.underscore}.rb and app/models/#{model.to_s.underscore}.rb."

    # Normal bidirectional association
    else
      assert associated_object = assoc.name.to_s.singularize.camelize.constantize.first,
             "#{assoc.name.to_s.camelize} not present in fixtures. Check test/fixtures/#{assoc.name.to_s.pluralize.underscore}.yml"
      model_method = model.to_s.underscore
      assert associated_object.respond_to?(model_method) || associated_object.respond_to?(model_method.pluralize),
             "#{associated_object.class.to_s.camelize} is not properly associated with #{model.to_s}. Check for the correct association in app/models/#{assoc.name.to_s.underscore}.rb."
    end
  end
end

Huge conditionals are generally regarded as a code smell. I'm not really concerned here, though, because these tests are explicitly designed to enable fast development and they did that extremely well. Once the models contain business logic that is under test, this method no longer serves a purpose. Anything that would cause this method to fail should cause other tests to fail as well. In other words, these are the kinds of tests I won't refactor if they turn red later on. I'll just delete them, and work on getting my other tests green.

Is this good practice? I'd say generally not. I doubt many developers need to make dozens of models that don't contain any significant logic to test. This was a solution to a specific problem, at a specific stage of development. It was a great learning experience, though, and it helped me get comfortable with the general practice of TDD. It was also gratifying to write a test helper method that I was able to reuse in all of my model tests, because the time I invested in it payed off many times over.