Painfully Simple Test Case Mistakes That Are Easy To Fix

Painfully Simple Test Case Mistakes That Are Easy To Fix

Rishi Jain
Rishi Jain
on Best Practices

Writing effective test cases is as important as writing the business logic in your application. This blog post dives into the simple yet effective ways developers can use when crafting test cases.

Lack of assertions in the test cases

We all have written controller test cases in the past. It could be rspecs, minitests, or any other framework. Consider we have an UsersController and the code look like this:

# app/controllers/users_controller.rb
class UsersController < ApplicationController
  def create
    @user = User.new(user_params)

    if @user.save
      # Sending welcome email
      UserMailer.welcome_email(@user).deliver_later

      # Creating default settings for user
      UserSettings.create!(user: @user, theme: 'light', notifications: true)

      render json: @user, status: :created
    else
      render json: { errors: @user.errors.full_messages }, status: :unprocessable_entity
    end
  end

  private

  def user_params
    params.require(:user).permit(:email, :username, :password)
  end
end

In the controller, we are creating users, and upon successful creation of users, we are also sending an email and creating a row UserSettings table. We have the following test case

# test/controllers/users_controller_test.rb
class UsersControllerTest < ActionDispatch::IntegrationTest
  # Bad test - Only checking response status
  test "should create user with valid params" do
    post users_path, params: {
      user: {
        email: "test@example.com",
        username: "testuser",
        password: "password123"
      }
    }

    # Only asserting the response code - Missing many important checks!
    assert_response :created
  end

This brings me to the point of lack of right assertions. The only thing we are testing is the response of the request. But we are not testing if the mail is being sent and if a record is created in the UserSettings table. If someone were to remove any of that code by mistake, the test cases will never catch it.

Ideally we should be asserting if a mail is being sent, if a record is created in the UserSettings table.

Testing only the positive outcomes

The business logic that we write handles all kinds of scenarios. It could handle success, but it also needs to handle failure graciously. But sometimes, we developers tend to only test the successful outcomes and ignore the unsuccessful ones which are as important as the successful ones.

To understand this better, let’s look at the code below:

# app/controllers/orders_controller.rb
class OrdersController < ApplicationController
  def create
    @order = Order.new(order_params)
    @product = Product.find(params[:product_id])

    # Various failure scenarios that should be tested
    if @product.out_of_stock?
      render json: { error: 'Product is out of stock' }, status: :unprocessable_entity
      return
    end

    if !current_user.can_afford?(@product.price)
      render json: { error: 'Insufficient funds' }, status: :payment_required
      return
    end

    if current_user.exceeded_monthly_purchase_limit?
      render json: { error: 'Monthly purchase limit exceeded' }, status: :forbidden
      return
    end

    if @order.save
        # Send confirmation
        OrderMailer.confirmation_email(@order).deliver_later
        render json: @order, status: :created
    else
      render json: { errors: @order.errors.full_messages }, status: :unprocessable_entity
    end
  end

  private

  def order_params
    params.require(:order).permit(:delivery_address, :notes)
  end
end

In the controller code, we are creating orders for a customer. There are various conditions which should be met before we can create the order for a customer. For example: the product the customer is trying to purchase should be present in stock, or the customer should have sufficient wallet balance to buy this product.

Lets look at a test case which does not test all the scenarios:

# Problematic test suite - only testing successful scenario
  test "should create order successfully" do
    assert_difference 'Order.count' do
      assert_enqueued_with(job: ActionMailer::MailDeliveryJob) do
        post orders_path, params: {
          product_id: @product.id,
          order: {
            delivery_address: "123 Main St",
            notes: "Please deliver after 6 PM"
          }
        }
      end
    end

    assert_response :created

    json_response = JSON.parse(@response.body)
    assert_equal "123 Main St", json_response['delivery_address']
    assert_equal "Please deliver after 6 PM", json_response['notes']
  end
end

In this test case, we are only testing the use case when the order is created successfully. This is how we should be writing tests for use cases when the order is not created successfully.

test "should not create order when product is out of stock" do
  @product.update!(stock: 0)  # Set product as out of stock

  assert_no_difference 'Order.count' do
    assert_no_enqueued_emails do
      post orders_path, params: {
        product_id: @product.id,
        order: {
          delivery_address: "123 Main St",
          notes: "Please deliver after 6 PM"
        }
      }
    end
  end

  assert_response :unprocessable_entity

  json_response = JSON.parse(@response.body)
  assert_equal 'Product is out of stock', json_response['error']
end

test "should not create order when user has insufficient funds" do
  @user.update!(balance: 0)  # Set user balance to 0

  assert_no_difference 'Order.count' do
    assert_no_enqueued_emails do
      post orders_path, params: {
        product_id: @product.id,
        order: {
          delivery_address: "123 Main St",
          notes: "Please deliver after 6 PM"
        }
      }
    end
  end

  assert_response :payment_required

  json_response = JSON.parse(@response.body)
  assert_equal 'Insufficient funds', json_response['error']
end

test "should not create order when user exceeds monthly limit" do
  # Simulate user exceeding monthly limit
  @user.update!(monthly_orders_count: @user.monthly_purchase_limit)

  assert_no_difference 'Order.count' do
    assert_no_enqueued_emails do
      post orders_path, params: {
        product_id: @product.id,
        order: {
          delivery_address: "123 Main St",
          notes: "Please deliver after 6 PM"
        }
      }
    end
  end

  assert_response :forbidden

  json_response = JSON.parse(@response.body)
  assert_equal 'Monthly purchase limit exceeded', json_response['error']
end

test "should not create order with invalid attributes" do
  assert_no_difference 'Order.count' do
    assert_no_enqueued_emails do
      post orders_path, params: {
        product_id: @product.id,
        order: {
          delivery_address: "", # Invalid: blank delivery address
          notes: "Please deliver after 6 PM"
        }
      }
    end
  end

  assert_response :unprocessable_entity

  json_response = JSON.parse(@response.body)
  assert json_response['errors'].present?
  assert_includes json_response['errors'], "Delivery address can't be blank"
end

Testing multiple scenarios under a single test case

Consider the test case below.

# test/controllers/products_controller_test.rb
class ProductsControllerTest < ActionDispatch::IntegrationTest
  # BAD PRACTICE: Multiple scenarios clubbed into one test
  test "should handle product updates" do
    @product = products(:laptop)

    # Testing basic update
    patch product_path(@product), params: {
      product: {
        name: "New Name",
        price: 999.99,
        category: "Electronics",
        description: "Updated description"
      }
    }
    assert_response :ok
    assert_equal "New Name", @product.reload.name

    # Testing price change notification
    patch product_path(@product), params: {
      product: { price: 899.99 }
    }
    assert_enqueued_with(job: ActionMailer::MailDeliveryJob)
    assert PriceTracker.changes.last.present?

    # Testing category change notification
    patch product_path(@product), params: {
      product: {
        category: "Computers",
        category_changed: true
      }
    }
    assert_enqueued_with(job: ActionMailer::MailDeliveryJob)

    # Testing validation failure
    patch product_path(@product), params: {
      product: { name: "" }
    }
    assert_response :unprocessable_entity

    # Testing non-existent product
    patch product_path("invalid-id"), params: {
      product: { name: "Test" }
    }
    assert_response :not_found
  end
end

In this test case, we are testing multiple scenarios like a basic update, category change notification, validation failure, non-existent product. There are various problems with this approach like state dependencies, maintenance issues, poor readability, unclear failure.

State Dependencies

  • The test modifies the product multiple times, creating implicit dependencies between scenarios
  • Later scenarios depend on the state changes from earlier scenarios
  • Makes it harder to understand what state the product should be in

Maintenance Issues

  • Hard to modify individual scenarios without affecting others
  • Difficult to add new test cases

Poor readability

  • Hard to quickly understand what scenarios are being tested
  • No clear separation between different test cases
  • Mixed concerns (basic updates, notifications, validations, error handling)

Unclear Failures

  • Stack trace will point to a single large test method
  • The test failure message won’t clearly indicate which part of the test failed

For the above listed reasons, most times, it makes sense to keep separation in test cases and not club multiple different scenarios under 1 large test case.

Poor test descriptions

The purpose of tests is not only to help catch regressions early but also to act as a documentation for the application. When we read a test file, we should be able to understand the workflow involved, the different scenarios that can happen, and how the system is supposed to react to them. And for that reason how we describe the test cases we write is very important.

Here are some examples of poor test descriptions:

# test/controllers/users_controller_test.rb
class UsersControllerTest < ActionDispatch::IntegrationTest
  # Poor test descriptions

  # Vague and uninformative
  test "should work" do
    post users_path, params: { user: { name: "John", age: 25 } }
    assert_response :success
  end

  # Too generic
  test "should test user" do
    user = users(:john)
    patch user_path(user), params: { user: { status: "active" } }
    assert_response :success
  end

  # Doesn't indicate what's being tested
  test "update test" do
    user = users(:john)
    user.update(email: "new@example.com")
    assert user.valid?
  end

  # Doesn't describe the scenario
  test "email check" do
    user = User.new(email: "invalid")
    refute user.valid?
  end

  # Unclear about what's being verified
  test "should verify" do
    user = users(:john)
    user.deactivate
    assert_not user.active?
  end
end

Here is how we can make them better:

class UsersControllerTest < ActionDispatch::IntegrationTest

  # Better test descriptions - Clear and descriptive
  test "should create user with valid parameters" do
    post users_path, params: { user: { name: "John", age: 25 } }
    assert_response :success
  end

  test "should activate user when admin approves registration" do
    user = users(:john)
    patch user_path(user), params: { user: { status: "active" } }
    assert_response :success
  end

  test "should update user email and send confirmation mail" do
    user = users(:john)
    user.update(email: "new@example.com")
    assert user.valid?
  end

  test "should reject user creation with invalid email format" do
    user = User.new(email: "invalid")
    refute user.valid?
  end

  test "should mark user as inactive after deactivation" do
    user = users(:john)
    user.deactivate
    assert_not user.active?
  end
end

Break your test cases

This is not a mistake that we make while writing test cases but it is more of a good practice. One way to make sure that the tests are actually strong is to make some random change in the code you are testing and then checking if the test fails. Look at this sample code:

# app/controllers/orders_controller.rb
class OrdersController < ApplicationController
  def create
    @order = Order.new(order_params)
    @product = Product.find(params[:product_id])

    # Various failure scenarios that should be tested
    if @product.out_of_stock?
      render json: { error: 'Product is out of stock' }, status: :unprocessable_entity
      return
    end

    if !current_user.can_afford?(@product.price)
      render json: { error: 'Insufficient funds' }, status: :payment_required
      return
    end

    if current_user.exceeded_monthly_purchase_limit?
      render json: { error: 'Monthly purchase limit exceeded' }, status: :forbidden
      return
    end

    if @order.save
        # Send confirmation
        OrderMailer.confirmation_email(@order).deliver_later
        render json: @order, status: :created
    else
      render json: { errors: @order.errors.full_messages }, status: :unprocessable_entity
    end
  end
end

Now imagine if we change the status in this render json: { error: 'Product is out of stock' }, status: :unprocessable_entity line to success. And if we run the test case after making the change, it should fail as we should have tested against the status of the response. But if we only tested the response body then it might pass since we did not make a change in the body. This tells us that our test cases are not testing everything in the code.

While I have done this manually in the past, there is a tool called Mutant opens a new window that does similar things for you.

Conclusion

These are some of the simple things you can apply when writing test cases. I hope this article provides valuable insights and helps you write quality test cases.

Need help with your Rails application? Talk to us today! opens a new window

Rishi Jain's Picture

Rishi Jain

If you need help with your Rails upgrade project, contact us! FastRuby.io provides a top-notch Rails upgrade service so you can focus on features. If you enjoyed this article, follow me on Github!

Get the book