Optimize Tests in Rails6 - Minitest - Factory

Question

I finally got this working but I am concerned I am being 'too' specific in my tests, and also that I am repeating myself a significant amount.

I could also combine all of the failures into a single test but that feels incorrect as I could change max to 500 and then the entire thing would fail.

Namely, I am not happy with the article = build sets everywhere, but is it correct?

articles.rb - Factory

require 'faker'
# Article Specs
# Title - Min 6, Max 100
# Desc - Min 10, Max 400
FactoryBot.define do 
  factory :article do
    trait :short_title do
      title { Faker::Lorem.characters(number: 5) }
    end
    trait :long_title do
      title { Faker::Lorem.characters(number: 101) }
    end
    trait :short_desc do
      description { Faker::Lorem.characters(number: 9) }
    end
    trait :long_desc do
      description { Faker::Lorem.characters(number: 401) }
    end
    title { Faker::Lorem.characters(number: 8) }
    description { Faker::Lorem.characters(number: 53) }
  end
end

articles_controller_test.rb

require "test_helper"
require "faker"
class ArticlesControllerTest < ActionDispatch::IntegrationTest
  setup do
    @article = build(:article)
  end

  test "should get index" do
    get articles_url
    assert_response :success
  end

  test "should get new" do
    get new_article_url
    assert_response :success
  end

  test "should create article" do
    assert_difference('Article.count') do
      post articles_url, params: { article: { description: @article.description, title: @article.title }}
    end
  end

  test "should deny due to small desc size" do
    article = build(:article, :short_desc)
    assert_no_difference('Article.count') do
      post articles_url, params: { article: { description: article.description, title: article.title }} 
    end
  end
  test "should deny due to long desc size" do
    article = build(:article, :long_desc)
    assert_no_difference('Article.count') do
      post articles_url, params: { article: { description: article.description, title: article.title }} 
    end
  end
  test "should deny due to small title size" do
    article = build(:article, :short_title)
    assert_no_difference('Article.count') do
      post articles_url, params: { article: { description: article.description, title: article.title }} 
    end
  end
  test "should deny due to large title size" do
    article = build(:article, :long_title)
    assert_no_difference('Article.count') do
      post articles_url, params: { article: { description: article.description, title: article.title }} 
    end
  end
end
```

Azahe · Accepted Answer · 2021-11-16 11:09:04Z

Approaching duplication in test one should usually be very careful not to introduce unnecessary dependencies between the test and fixtures.

Oftentimes we encounter accidental duplication between tests that gets too eagerly removed - like creating some kind of helper for test inputs and then relaying too much on its implementation in test (ofc you want to relay on the helper to some degree - like if you want something to be valid number when its a helper for numbers etc.).
When we not remove it we have seemingly repeated setup of inputs - but as long as these inputs could be different in each test such duplication should be fine.

I would even dare to argue that making the @article instance variable is already a little too much and would let it be test local variable (with perhaps different per-test name as it may be different kind of article and this difference could be conveyed in name).

The main idea why such ""duplication"" is acceptable is because we usually consider test self-containment as more valuable:

its easier to reason about something if it has no hidden dependencies
refactoring self contained chunks of code is easier that ones with hidden dependencies

On the other hand I would consider the duplication of making the request based of article instance - as this is not something per-test specific (like test input) but rather a reflection of implementation and would change with it (adding something to the body would currently require changes in multiple tests and while it could be in one place).

BTW as you have yourself noticed changing max in factory could break the tests - which means it is a currently hidden (from the test perspective) dependency. I would consider making it explicit in tests - which, honestly would be quite readable to see test for posting an article failing because it is too long (or to short) and seeing exactly what it means.

Wouldn't making every article instance be local to the test in itself go against Rails principles? If I am doing article = build(:article) in every test that should pass, I'm repeating myself a lot. The setup section is how the documentation declares to pass in the Fixture. — StartingRails
– StartingRails, Commented Nov 16, 2021 at 14:45
Are you really repeating yourself? Is it meaningful repetition? Or maybe its just you typing a little bit more? How easy to change is solution with and without duplication? Note that even with mutable instance variable you are already reassigning values quite often - have you considered how default article "pollutes" scope of tests where it is not needed? You don't have to believe me - see discussion (and the post) in here by a lot of experienced developers and make your own mind. — Azahe
– Azahe, Commented Nov 17, 2021 at 10:18

Stack Exchange Network

Optimize Tests in Rails6 - Minitest - Factory

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Optimize Tests in Rails6 - Minitest - Factory

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions