5

One of unit test best practices is to make each test independent to all the others. Lets say I want to test add() method of a BoundedPriorityBlockingQueue custom class:

public void testAdd() {
    BoundedPriorityBlockingQueue q = BoundedPriorityBlockingQueue();
    q.add(1);
    assertEquals(1, q.size());
}

as you can see currently testAdd uses size() method so it depends on it but I dont want testAdd() to fail when size() is broken. What is the best practice in this situation?

Evgeniy Dorofeev
  • 133,369
  • 30
  • 199
  • 275
  • You could use reflection to obtain your information, without relying on other methods from the object – Morfic Sep 26 '13 at 12:43
  • 2
    @Grove: That would be far more brittle than using `size()`, IMO. It would rely on implementation details. – Jon Skeet Sep 26 '13 at 12:47
  • @Jon I partially agree, since if I'm not mistaking unit testing is supposed to be white-box-testing – Morfic Sep 26 '13 at 12:49
  • @Grove: It's not a black or white issue; where you *can* easily avoid relying on private implementation details, it's good to do so - but where it makes it simpler to test, I'm happy to dig into the implementation more. In this case, using reflection would make the tests more complicated rather than simpler, with no clear benefit. – Jon Skeet Sep 26 '13 at 12:50
  • @Jon I totally agree that reflection is more tedious and should be avoided, hence you got my +1. Just wanted to point out that it can be used if you want really bad to avoid depending on other methods from the object. I usually have no problem in cross checking save/load, add/remove and so on in my tests, but I try to have multiple scenarios/assertions to make sure it's not a lucky coincidence that it works. – Morfic Sep 26 '13 at 12:58

4 Answers4

12

What is the best practice in this situation?

Just suck it up, bearing in mind that tests are meant to serve you, not the other way round.

Will your tests break if something goes horribly wrong? Yes.

Will it be clear where the problem is? Probably, given that anything using size will fail.

Is this test driving you towards a less testable design? No.

Is this the simplest approach to testing add, which is robust in the face of changing implementation details? Probably. (I'd test that you can get the value out again, mind you.)

Yes, it's sort of testing two parts of the same class - but I really don't think that's a problem. I see a lot of dogma around testing ("only ever test the public API, always use AAA" etc) - in my experience you should temper that dogmatism with a healthy dose of pragmatism.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
6

The goal is to make all test methods independent of other test methods, and this method is independent. It will pass or fail based on the operation of the methods in the class under test, regardless of what you do in other test methods.

It's fine for this test to fail if another method from the class under test is broken. If size() is broken you'll have multiple test failures (this one and the one that explicitly tests size()) so it will be obvious where the problem is. If add() is broken, only this test will fail (along with any other methods that rely on add()).

Bill the Lizard
  • 398,270
  • 210
  • 566
  • 880
2

As others have already said, if your size method is broken the test will fail anyway so you have a reason there to investigate and understand why is that happening.

Anyway, if you are still interested on having such independence between your tests you could go for a white-box testing strategy: I guess that your BoundedPropertyBlockingQueue uses internally either any of the java.util collections, an array or an collection implementation from other provider (Guava, Apache Collections, etc) that you rely on so you don't need to verify that those structures work as they are expected to do.

So, define that internal structure as protected, place your test class in a package with the same name and, instead of relying on the implementation of the size method, go into the guts of the BoundedPropertyBlockingQueue:

BoundedPriorityBlockingQueue q = BoundedPriorityBlockingQueue();
q.add(1);
assertEquals(1, q.contents.size());  // assuming that `contents` attribute is a collection.

The main drawback is that now if your internal implementation of the queue changes, you'll need to change the test whilst with your previous test method you won't need to.

IMO I would chose your current implementation, is less coupled and, at the end, meets its goal.

Alonso Dominguez
  • 7,750
  • 1
  • 27
  • 37
1

There's nothing wrong with doing such cross-testing - some methods tend to live in pairs (add/remove, enqueue/dequeue, etc) and it makes little sense to test one without its complementary part.

However, I would give a bit more thought to how the add method will be used by your clients (class users). Most likely won't call add only to determine whether size changed, but rather to later retrieve added item. Perhaps your test should look more like this:

BoundedPriorityBlockingQueue q = new BoundedPriorityBlockingQueue();
QueueItem toAdd = 1;
QueueItem added = q.dequeue();
assertEquals(toAdded, added);

On top of that you can also add guard assert to the test above (to assure queue doesn't start with some items already added) or even better - include separate test that guarantees initial state of queue (size is 0, dequeue returning null/throwing).

Community
  • 1
  • 1
k.m
  • 30,794
  • 10
  • 62
  • 86