How to identify design cues by looking at the tests? Part 2

Objective


Learn how to use design cues found in the tests to improve the design

Discussion


A test that has complicated setup knows too much about the structure of the objects. Here is an example:

  context 'Identify fraudulent changes' do
    before do
      @item_number = 1
      item_name = 'Essential TDD'
      @actual_amount = 100.00
      @transaction = double('Transaction')
      @transaction.stub(:amount) { @actual_amount }
      @transaction.stub(:currency) { 'USD' }      
      @notify = double('Notify')      
      @notify.stub(:currency) { 'USD' }
      @notify.stub(:item_number) { @item_number }
      @notify.stub(:item_name) { item_name }
      @notify.stub(:gross) { 1 }
      @order = double('Order')
      @order.stub_chain(:product, :name) { item_name }
    end

    it 'should identify fraudulent change when payment does not have correct amount' do
      @order.stub(:product_id) { @item_number }
      @notify.stub(:gross) { @actual_amount - 50.00 }

      fraud_policy = FraudPolicy.new(@notify, @transaction, @order)

      expect(fraud_policy.no_fraudulent_change?).to be(false)
    end

    it 'should not be fraudulent change when payment has correct amount' do
      @order.stub(:product_id) { @item_number }

      fraud_policy = FraudPolicy.new(@notify, @transaction, @order)

      expect(fraud_policy.no_fraudulent_change?).to be(true)
    end    

    it 'should be fraudulent change when item number (product_id) is not the same' do
      @order.stub(:product_id) { 2 }

      fraud_policy = FraudPolicy.new(@notify, @transaction, @order)

      expect(fraud_policy.no_fraudulent_change?).to be(false)      
    end

    it 'should not be fraudulent change when item number (product_id) is the same' do
      @order.stub(:product_id) { @item_number }

      fraud_policy = FraudPolicy.new(@notify, @transaction, @order)

      expect(fraud_policy.no_fraudulent_change?).to be(true)      
    end

    it 'should be fraudulent change when item name (product name) is not the same' do
      @order.stub(:product_id) { @item_number }
      @notify.stub(:item_name) { 'Sneaky Thing' }

      fraud_policy = FraudPolicy.new(@notify, @transaction, @order)

      expect(fraud_policy.no_fraudulent_change?).to be(false)      
    end

    it 'should not be fraudulent change when item name (product name) is the same' do
      @order.stub(:product_id) { @item_number }

      fraud_policy = FraudPolicy.new(@notify, @transaction, @order)

      expect(fraud_policy.no_fraudulent_change?).to be(true)      
    end

  end

The individual tests override the values to put the required objects to a certain state for that particular test. The test seems to be easy to read, nevertheless, it leaks the structure of the objects and therefore does not hide the implementation details.

After the redesign, the tests have much less context to build before we can exercise the SUT.

describe Bacardi::FraudCheck do

  it 'return true if there is no malicious change in product_id and product_name' do
    order = Order.new
    order.product_name = 'ETDD'
    order.product_id = 1

    notifier = Notifier.new
    notifier.item_number = 1
    notifier.item_name = 'ETDD'

    fc = Bacardi::FraudCheck.new(order, notifier)

    result = fc.no_malicious_product_change?

    expect(result).to be_truthy
  end

  it 'return false if there is malicious change in product_id and product_name' do
    order = Order.new
    order.product_name = 'ETDD'
    order.product_id = 1

    notifier = Notifier.new
    notifier.item_number = 1
    notifier.item_name = 'ETDD2'

    fc = Bacardi::FraudCheck.new(order, notifier)

    result = fc.no_malicious_product_change?

    expect(result).to be_falsy
  end

end

Here there is only two objects that is required in the setup phase. Similarly, here is notification_spec.rb:

describe Bacardi::Notification do

  before do
    @notifier = Notifier.new
  end

  it 'retrieve paid currency' do
    @notifier.currency = 'USD'
    notification = Bacardi::Notification.new(@notifier)

    result = notification.paid_currency

    expect(result).to eq('USD')
  end

  it 'retrieve paid gross' do
    @notifier.gross = '20'
    notification = Bacardi::Notification.new(@notifier)

    result = notification.paid_gross

    expect(result).to eq(BigDecimal.new(2000))
  end

  it 'retrieve confirmation number' do
    @notifier.invoice = '1'
    notification = Bacardi::Notification.new(@notifier)

    result = notification.confirmation_number

    expect(result).to eq('1')
  end

  it 'retrieve receiver email' do
    @notifier.account = 'bugs@rubyplus.com'
    notification = Bacardi::Notification.new(@notifier)

    result = notification.receiver_email

    expect(result).to eq('bugs@rubyplus.com')
  end

end

The payment_spec.rb also has simpler setup.

describe Bacardi::Payment do

  before do
    @transaction = Transaction.new    
  end

  it 'retrieve currency for a transaction' do
    @transaction.currency = 'USD'
    payment = Bacardi::Payment.new(@transaction)

    actual_currency = payment.actual_currency 

    expect(actual_currency).to eq('USD')
  end

  it 'retrieve gross for a transaction' do
    @transaction.gross = '300.00'

    payment = Bacardi::Payment.new(@transaction)

    actual_gross = payment.actual_gross 
    expected_amount_in_cents = BigDecimal.new(30000)

    expect(actual_gross).to eq(expected_amount_in_cents)
  end

end

You can browse the entire source code of Bacardi gem to see how the tests are simpler. This results in a better design.

Conclusion


The setup methods that knows too much about the object structure leaks implementation details to the client. Our tests are the very first client of our code. We must look for ways to reduce the context required to test for a given scenario, this will result in better design and easier to read tests.


Related Articles


Ace the Technical Interview

  • Easily find the gaps in your knowledge
  • Get customized lessons based on where you are
  • Take consistent action everyday
  • Builtin accountability to keep you on track
  • You will solve bigger problems over time
  • Get the job of your dreams

Take the 30 Day Coding Skills Challenge

Gain confidence to attend the interview

No spam ever. Unsubscribe anytime.