Unit-testing account transaction code

Question

I'm playing with unit-testing in Ruby. There is a situation that I don't know if is good enough or if I am doing the wrong abstraction.

I have two classes: Transaction and Account. An Account could have 0..* Transactions. The only constraint is shall not have duplicated transactions for an Account.

it "add a duplicated transaction will fails" do
  transaction = double(:transaction, id: '124-acd-345')
  expect(transaction).to receive(:==).and_return true

  subject.add_transaction transaction
  subject.add_transaction transaction.clone

  expect(subject.transactions.count).to eq 1
end

Account#add_transaction piece:

def add_transaction(transaction)
  @transactions << transaction if transactions.select { |t| t == transaction }.empty?
end

Transaction#== piece:

def ==(other)
  other.id == self.id
end

The point is: Transaction#== is already unit tested in their test class. I know that transaction == works properly. So, my approach in Account#add_transaction was Test Behavior (with that expect(transaction) ...) and Test Value, checking if there is only one transaction into object.

IMO, I didn't like this solution, but I can't find anything better to do. Tips?

Answer 1

My first thought based on your comment (behavioral and state testing in same test) was that you should just make two different tests.

However, then I took a step back. The specification says that:

add[ing] a duplicated transaction will fail

It doesn't say anything about how a duplicate is determined. Currently, you are using the naive implementation is to check if and of existing transactions equal the one being added. However, that is not the only way satisfy the requirement.

If there are a large number of transaction, it will be slow to iterate over each transaction. Instead, the transactions could be stored in a map that uses the id as a key. Now be can check if the transaction already exists by doing a single key lookup, \$O(1)\$ instead of \$O(n)\$.

The fact that Transaction#== is called is an implementation detail. Based on this, the behavioral assertion should be removed from the test. If there happens to be a different requirement, that was not presented here, that the duplication check must be done throw an equivalence check, then that should be a different test.