Reconsidering unit tests

Przemysław Szyszka

@szysza_p

http://szyszka.herokuapp.com

Motivations

Motivations

  • Do we write good unit tests?

Motivations

  • Do we write good unit tests?
  • Do we cover all of the test cases?

Motivations

  • Do we write good unit tests?
  • Do we cover all of the test cases?
  • Do we need to write a certain test?

Use blocks for multi-line tests

require 'rails_helper'

RSpec.describe Calculator do
  describe '.call'
    subject { described_class.call(payment) }

    it {
      calculated_value = ...
      expect(subject).to eq calculated_value
    }
  end
end

Use blocks for multi-line tests

require 'rails_helper'

RSpec.describe Calculator do
  describe '.call'
    subject { described_class.call(payment) }

    it 'returns the calculated value' do
      calculated_value = ...
      expect(subject).to eq calculated_value
    end
  end
end

How to choose a good test name?

  • Don't describe the state too much
  • Describe what's changing

The same for variables declaration

require 'rails_helper'

RSpec.describe Calculator do
  describe '.call'
    let(payment) {
      create(:payment, status: :some_status, amount: 100, payment_method: :credit_card, provider: :some_provider)
    }
    
    ...
  end
end

The same for variables declaration

require 'rails_helper'

RSpec.describe Calculator do
  describe '.call'
    let(payment) do
      create(:payment,
        status: :some_status,
        amount: 100,
        payment_method: :credit_card,
        provider: :some_provider)
    end
    
    ...
  end
end

How to test that something changed

require 'rails_helper'

RSpec.describe PaymentProcess do
  describe '.call'
    subject { described_class.call(payment) }

    it 'changes payment status from pending to processing' do
      expect(payment.status).to eq 'pending'
      subject
      expect(payment.status).to eq 'processing'
    end
  end
end

How to test that something changed

require 'rails_helper'

RSpec.describe PaymentProcess do
  describe '.call'
    subject { described_class.call(payment) }

    it 'changes payment status from pending to processing' do
      expect { subject }.to change { payment.reload.status }
        .from('pending').to('processing')
    end
  end
end

Avoid multi-line expects

require 'rails_helper'

RSpec.describe PaymentProcess do
  describe '.call'
    subject { described_class.call(payment) }

    it 'returns some values' do
      expect(subject).to eq {
                              some_key: some_value,
                              example_key: example_value
                            }
    end
  end
end

Avoid multi-line expects

require 'rails_helper'

RSpec.describe PaymentProcess do
  describe '.call'
    subject { described_class.call(payment) }

    it 'returns some values' do
      expected_hash = {
        some_key: some_value,
        example_key: example_value
      }

      expect(subject).to eq expected_hash
    end
  end
end

Testing Hashes

class PaymentCalculator
  class << self
    def call
      {
        processing_amount: processing_amount,
        settled_amount: settled_amount
      }
    end

    private

    def processing_amount
      ...
    end

    def settled_amount
      ...
    end
  end
end

Testing Hashes

require 'rails_helper'

RSpec.describe PaymentCalculator do
  describe '.call'
    it 'returns processing amount' do
      expect(subject).to include(processing_amount: ...)
    end

    it 'returns settled amount' do
      ...
    end
  end
end

Do we need two tests?

require 'rails_helper'

RSpec.describe PaymentCalculator do
  describe '.call'
    it 'returns amounts' do
      expect(subject).to eq { processing_amount: ..., paid_amount: ... }
    end
  end
end

Do we need two tests?

require 'rails_helper'

RSpec.describe PaymentCalculator do
  describe '.call'
    it 'returns amounts' do
      expected_amounts = {
        processing_amount: ...,
        paid_amount: ...,
        ...
      }

      expect(subject).to eq expected_amounts
    end
  end
end

Expecting array response

require 'rails_helper'

RSpec.describe FailedPayments do
  describe '.call'
    subject { described_class.call }

    it 'returns the failed payments' do
      expect(subject.size).to eq 2
      expect(subject[0]).to eq payment_one
      expect(subject[1]).to eq payment_two
    end
  end
end

Expecting array response

require 'rails_helper'

RSpec.describe FailedPayments do
  describe '.call'
    subject { described_class.call }

    it 'returns the failed payments' do
      expect(subject).to eq [payment_one, payment_two]
    end
  end
end

Expecting array response

require 'rails_helper'

RSpec.describe FailedPayments do
  describe '.call'
    subject { described_class.call }

    it 'returns the failed payments' do
      expect(subject).to match_array [payment_two, payment_one]
    end
  end
end

Avoid global variables

RSpec.describe PaymentCalculator do
  describe '.call'
    let(:customer) { regular_user }
    let(:params) do
      customer: customer,
      new_payment: new_payment
    end

    context 'with manager privileges' do
      let(:customer) { manager_user }

      context 'with excluded payments' do
        let(:parameters) { params.merge!(exclude_pending: true) }

        it 'returns amounts' do
          expected_amounts = { ... }
          expect(subject).to eq expected_amounts
        end
      end

      context 'without excluded payments' do
        let(:parameters) { params.merge!(exclude_pending: false) }

        it 'returns amounts' do
          expected_amounts = { ... }
          expect(subject).to eq expected_amounts
        end
      end
    end

    context 'without manager privileges' do
      ...
    end
  end
end

Avoid global variables

require 'rails_helper'

RSpec.describe PaymentCalculator do
  describe '.call'
    it 'returns amounts for manager with excluded payments' do
      calculator = described_class.new(customer: manager_user, ...)
      expected_amounts = { ... }

      expect(calculator.call).to eq expected_amounts
    end

    it 'returns amounts for manager with NOT excluded payments' do
      calculator = described_class.new(
        customer: manager_user,
        exclude_pending: true
      )
      expected_amounts = { ... }

      expect(calculator.call).to eq expected_amounts
    end
    
    ...
  end
end

Avoid global variables

require 'rails_helper'

RSpec.describe PaymentCalculator do
  describe '.call'
    context 'example context name' do
      before do
        ...
      end
    
      it 'returns amounts for manager with excluded payments' do
        calculator = described_class.new(customer: manager_user, ...)
        expected_amounts = { ... }

        expect(calculator).to eq expected_amounts
      end
    end
    ...
  end
end

Calls to other classes

PaymentProcessor

PaymentCalculator

Calls to other classes

PaymentProcessor

PaymentCalculator

Calls to other classes

PaymentProcessor

PaymentCalculator

  • Possible responses
    • Nil
    • Empty value
    • One element response
    • Many elements response
    • Error response

Calls to other classes

Mock calls to other class

require 'rails_helper'

RSpec.describe PaymentProcessor do
  describe '.call'
    it 'calls payment calculator' do
      user = double(:user)

      expect(PaymentCalculator).to receive (:call).with(user)
      described_class.call(user)
    end

    it 'creates a new payment when user did not pay enough' do
      amounts = { processing_amount: ... }
      allow(PaymentCalculator).to receive(:call).and_return(amounts)
      processor = described_class.call(user)
      
      expect { processor }.to change { Payment.count }.by(1)
    end    
    ...
  end
end

Always mock the exact response

# Gemfile
gem 'money-rails'

# paid_amount.rb
class PaidAmount
  def self.call
    amount = paid_amount

    if amount == 0
      'Nothing paid yet'
    else
      "Paid amount: #{amount}"
    end
  end

  def self.paid_amount
    PaidAmountCalculator.call
  end
end

Always mock the exact response

RSpec.describe PaidAmount do
  describe '.call'
    before do
      allow(Calculator).to receive(:call).and_return(0)
    end

    it 'return an information that nothing is paid yet' do
      expect(subject).to eq 'Nothing paid yet'
    end
  end
end

Calls to external services

require 'rails_helper'

RSpec.describe TaxCalculator do
  describe '.call'
    subject { described_class.call(payment) }

    it 'calls external service' do
      payment = ...
      response = ...
      allow(TaxExternalService).to receive(:calculate).and_return response

      expect { subject }.to change { payment.tax }.from(0).to(2)
    end
  end
end

Calls to external services

require 'rails_helper'

RSpec.describe TaxCalculator do
  describe '.call'
    subject { described_class.call(payment) }

    it 'calls external service' do
      # Use VCR for mocking responses
      payment = ...
      VCR.use_cassette('calculated_tax') do
        expect { subject }.to change { payment.tax }.from(0).to(2)
      end
    end
  end
end

Filtering functionality

module Report
  class ApplyFilters
    def initialize(date: :all, status: 'All', user_id: 'All')
      @date    = date
      @status  = status
      @user_id = user_id
    end

    def self.call(*args); new(*args).call; end

    def call
      payments = Payment.order(created_at: :desc)
      payments = apply_date_filter(payments, @date)
      payments = apply_status_filter(payments, @status)
      payments = apply_user_filter(payments, @user_id)
    end

    private

    def apply_date_filter(payments, date)
      ...
    end

    ...
  end
end

First version of tests

RSpec.describe Report::ApplyFilters do
  let!(:payment_for_today) { create(:payment, ...) }
  let!(:payment_for_tomorrow) { create(:payment, ...) }

  describe 'date filter' do
    it 'shows only for specified date' do
      params = { date: Date.current.to_s, user_id: 'All', status: 'All' }

      payments_for_date = @payments.fetch(:for_date)
      expect(described_class.call(params)).to match_array([payment_for_today])
    end
  end

  describe 'status filter' do
    ...
  end

  describe 'user filter' do
    ...
  end
end

Second version of tests

RSpec.describe Report::ApplyFilters do
  describe 'date filter' do
    let!(:payment_for_today) { create(:payment, ...) }
    let!(:payment_for_tomorrow) { create(:payment, ...) }

    it 'shows only for specified date' do
      params = { date: Date.current.to_s, user_id: 'All', status: 'All' }

      payments_for_date = @payments.fetch(:for_date)
      expect(described_class.call(params)).to match_array([payment_for_today])
    end
  end

  describe 'status filter' do
    ...
  end

  describe 'user filter' do
    ...
  end
end

Database transaction

# rails_helper.rb

config.before(:all, type: :query) do
  DatabaseCleaner.strategy = :transaction
  DatabaseCleaner.start
end

config.after(:all, type: :query) do
  DatabaseCleaner.clean
end

Final version of tests

RSpec.describe Report::ApplyFilters, type: :query do
  before(:all) do
    payment_for_today = create(:payment, ...)
    payment_for_tomorrow = create(:payment, ...)

    @payments = { for_date: [payment_for_today],  }
  end

  describe 'date filter' do
    it 'shows only for specified date' do
      params = { date: Date.current.to_s, user_id: 'All', status: 'All' }

      payments_for_date = @payments.fetch(:for_date)
      expect(described_class.call(params)).to match_array(payments_for_date)
    end
  end

  describe 'status filter' do
    ...
  end

  describe 'user filter' do
    ...
  end
end

Inspirations

J.B. Rainsberger - Integration tests are a scam

Thank you

Questions?

Reconsidering unit tests

By Przemysław Szyszka

Reconsidering unit tests

  • 92
Loading comments...

More from Przemysław Szyszka