CODEWITHSUNDEEP
ruby-on-railstestingtddminitest
obreil54
obreil54

Reputation: 15

Should I write a test for each required attribute in a Rails model or is one enough?

I'm working on a Ruby on Rails project and practicing Test-Driven Development (TDD) using Minitest.

For user authentication (using Devise), I wrote a test to check that a user cannot sign up without an email. However, I also have other required attributes like first_name, last_name, username, and password.

This got me thinking:

Another developer suggested that if I have presence validation for multiple attributes, I should also write a test for each one. Their reasoning was that every implemented functionality should be tested. They also suggested using a looping approach to avoid redundant code instead of writing multiple nearly identical tests.

I understand that from a TDD perspective, where I write the test first and then write the code to make it pass, a looping test for missing attributes would force me to implement presence validation for each field. This ensures I don’t accidentally forget one. However, from a more pragmatic approach, I still wonder if this level of testing is necessary when Rails already enforces presence validation at the model level.

Would love to hear the best practice here—how do experienced Rails developers approach this?

Here is the test that I currently have:

  test "user cannot sign up without email" do
    post user_registration_path, params: {
      user: {
        first_name: "Test",
        last_name: "User",
        username: "testuser",
        email: "",
        password: "password123",
        password_confirmation: "password123",
      }
    }

    assert_response :unprocessable_entity
  end

Upvotes: 1

Views: 99

Answers (2)

max
max

Reputation: 102001

You can either test validations at the model level or a higher level. There is no right answer and both have different tradeoffs and are not mutally exclusive.

Unit testing on the model level lets you quickly test the validations before you even write anything else with very little overhead. You don't need to do HTTP, database interactions (well except for uniqueness) or anything else so they run super fast compared to sending a request and trying to fish out a message out of some JSON or HTML.

class UserTest < ActiveSupport::TestCase
  def validated_user(**attributes)
    User.new(attributes).tap(&:valid?)
  end

  test 'invalid without email' do
    user = validated_user(email: '')
    # You can create methods to make this less of a PITA
    assert_includes(user.errors.details[:email], { error: :blank })
  end
end

Don't fall for the trap of carpet bombing tests as popularized by a certain book with assert thing.valid?.

Integration and system tests at a higher level insure that the functionality of your API remains the same even if the underlying internals change.

For example you might want to refactor your validations out of the model and into a form object - this would still be covered by an integration test. And they would also cover when that wonky if: :something? condition you added to the validation to just make it fire on one action doesn't actually work the way you intented.

But just testing the HTTP response code is really the barest of minimums. If it's an JSON API it might return an errors object that you can inspect. For classical apps a system test can cover that not only is the validation there but the user is also getting the correct feedback.

You also shouldn't just assume that getting the right response code means the record was/wasn't created/updated/destroyed.

Should I write a separate test for each missing attribute?

Depends. Everything in testing is a tradeoff between acuity, coverage, the time it takes you write the test and the speed of the test suite. In the perfect world you would do the belts and suspenders approach and test at both levels but what in the end is acceptable is up to your team.

In the case of Devise you might want to focus on higher level tests as what matters is the integration of the things you're adding to the functionality provided by the gem.

Another developer suggested that if I have presence validation for multiple attributes, I should also write a test for each one. Their reasoning was that every implemented functionality should be tested. They also suggested using a looping approach to avoid redundant code instead of writing multiple nearly identical tests.

Loops or not in tests is a fairly controversial topic. Introducing additional logic may make them more brittle. And are you going to have a test that tests the test (ad infinitum)?

I would argue that a better way to make testing validations less painful is the Shoulda-Matchers gem and that it's more important to have simple declarative tests than to avoid duplication.

Upvotes: 2

lmtaq
lmtaq

Reputation: 391

I agree with what engineersmnky said in the comment. If your model has presence validations for those attributes, you're expected to write unit tests for each one of the validations in the model tests.

If you have a User model, I'd expect you also have a UserTest like the one below:

# test/models/user_test.rb
require 'test_helper'

class UserTest < ActiveSupport::TestCase
  def setup
    @user = User.new(
      first_name: 'Test',
      last_name: 'User',
      username: "testuser",
      email: "[email protected]",
      password: "password123",
      password_confirmation: "password123",
   )
  end

  test 'invalid without email' do
    @user.email = nil
    refute @user.valid?
    assert_not_nil @user.errors[:email]
  end

  # And other tests like the one above should be added for
  # any other validations
end

Then, when testing the request flow, you'll likely want to write a test for the happy path scenario and a generic test for an unhappy path.

Something like:

  # Happy path
  test "user can sign up as long as the data is valid" do
    post user_registration_path, params: {
      user: {
        first_name: "Test",
        last_name: "User",
        username: "testuser",
        email: "[email protected]",
        password: "password123",
        password_confirmation: "password123",
      }
    }

    assert_response :success
  end

  # Unhappy path
  test "user cannot sign up with invalid data" do
    post user_registration_path, params: {
      user: {
        first_name: "Test",
        last_name: "User",
        username: "testuser",
        email: "",
        password: "password123",
        password_confirmation: "password123",
      }
    }

    assert_response :unprocessable_entity
  end

These tests demonstrate the checks for the HTTP status responses for illustration purposes. It should cover any logic that's handled by the controller actions.

Note the "Unhappy path" test here is almost identical to the one in your question (the only difference being the test description). But on this approach, one unhappy test should be enough. You do not have to test that email should be present for every user, what you should test here is that your controller is properly handling the case where the user might be invalid, regardless of the reason.

Upvotes: 1

Related Questions