83

Today I was watching a "JUnit basics" video and the author said that when testing a given method in your program, you shouldn't use other of your own methods in the process.

To be more specific, he was talking about testing some record-creation method that took a name and last name for arguments, and it used them to create records in a given table. But he claimed that in the process of testing this method, he shouldn't use his other DAO methods to query the database to check the final result (to check that record was indeed created with the right data). He claimed that for that, he should write additional JDBC code to query the database and check the result.

I think I understand the spirit of his claim: you don't want one method's test case to depend on the correctness of the other methods (in this case, the DAO method), and this is accomplished by writing (again) your own validation/supporting code (which should be more specific and focussed, hence, simpler code).

Nonetheless, voices inside my head started protesting with arguments like code duplication, unnecessary additional efforts, etc. I mean, if we run the whole test battery, and we test all our public methods thoroughly (including the DAO method in this case), shouldn't it be OK to just use some of those methods while testing other methods? If one of them is not doing what it's supposed to, then its own test case will fail, and we can fix it and run the test battery again. No need for code duplication (even if the duplicate code is somewhat simpler) or wasted efforts.

I have an strong feeling about this because of several recent Excel-VBA applications I've written (properly unit-tested thanks to Rubberduck for VBA), where applying this recommendation would mean a lot of additional extra work, with no perceived benefit.

Can you please share your insights about this?

  • 79
    It's slightly strange to see a unit test that involves the database at all – Richard Tingle Sep 6 '16 at 16:59
  • 4
    IMO it's fine to call other classes IFF they're fast enough. Mock anything that has to go to disk or over a network. There's no sense in mocking a plain ol' class IMO. – RubberDuck Sep 6 '16 at 17:18
  • 2
    Have a link to this video? – candied_orange Sep 6 '16 at 17:32
  • 17
    "properly unit-tested thanks to Rubberduck for VBA" - you sir, have just made my day. I'd fix the typo and edit it from "RubberDuck" to "Rubberduck", but I'd feel like some spammer doing that and adding a link to rubberduckvba.com (I own the domain name and the co-own the project with @RubberDuck) - so I'll just comment here instead. Anyway it's awesome to see people actually using the tool that's been responsible for most of my sleepless nights for the better part of the past two years! =) – Mathieu Guindon Sep 6 '16 at 20:36
  • 4
    @Mat'sMug and RubberDuck I love what you are doing with Rubberduck. Please keep it up. Certainly my life is easier because of it (I happen to do a lot of small programs and prototypes in Excel VBA). BTW, the Rubberduck mention in my post was just me trying to be nice to RubberDuck himself, which in turn has been nice to me here in PE. :) – carlossierra Sep 6 '16 at 20:43

11 Answers 11

186

The spirit of his claim is indeed correct. The point of unit tests is to isolate code, test it free of dependencies, so that any erroneous behavior can be quickly recognized where it is happening.

With that said, unit testing is a tool, and it is meant to serve your purposes, it is not an altar to be prayed to. Sometimes that means leaving dependencies in because they work reliably enough and you don't want to bother mocking them, sometimes that means some of your unit tests are actually pretty close if not actually integration tests.

Ultimately you're not getting graded on it, what's important is the end product of the software being tested, but you'll just have to be mindful of when you're bending the rules and deciding when the trade-offs are worth it.

  • 117
    Hooray for pragmatism. – Robert Harvey Sep 6 '16 at 19:42
  • 6
    I would add that is is not some much the reliability as the speed that is the issue that leads to stubbing out dependencies. The test in isolation mantra is so compelling however that this argument is often missed. – Michael Durrant Sep 6 '16 at 22:01
  • 4
    "... unit testing is a tool, and it is meant to serve your purposes, it is not an altar to be prayed to. " -- this! – Wayne Conrad Sep 7 '16 at 14:08
  • 15
    "not an altar to be prayed to" - angry TDD coaches incoming! – Den Sep 7 '16 at 14:46
  • 5
    @jpmc26: even worse you don't want all your unit tests passing because they correctly handled the web service being down, which is a valid and correct behaviour, but never actually exercising the code for when it's up! Once you stub it you get to choose whether it's "up" or "down" and test both. – Steve Jessop Sep 7 '16 at 19:50
36

I think this comes down to terminology. To many, a "unit test" is a very specific thing, and by definition cannot be have a pass/fail condition that depends on any code outside of the unit (method, function, etc.) being tested. This would include interaction with a database.

To others, the term "unit test" is much looser, and encompasses any sort of automated testing, including test code that tests integrated portions of the application.

A test purist (if I may use that term) might call that an integration test, distinguished from a unit test on the basis that the test depends on more than one pure unit of code.

I suspect that you are using the looser version of the term "unit test", and are actually referring to an "integration test".

Using those definitions, you shouldn't depend on code outside of the unit under test in a "unit test". In an "integration test", however, it is perfectly reasonable to write a test that exercises some specific piece of code and then inspects the database for the passing criteria.

Whether you should depend on integration tests or unit tests or both is a much larger topic of discussion.

  • You are right on your suspicions. I am misusing the terms and I can see now how each type of test can be handled more appropriately. Thanks! – carlossierra Sep 7 '16 at 3:28
  • 11
    "To others, the term "unit test" is much looser" -- aside from anything else, so-called "unit testing frameworks" are a really convenient way to organise and run higher-level tests ;-) – Steve Jessop Sep 7 '16 at 14:32
  • 1
    Rather than a "pure" vs "loose" distinction, which is loaded with connotations, I'd suggest the distinction is between those who view "units", "dependencies", etc. from a domain perspective (e.g. "authentication" would count as a unit, "database" would count as a dependency, etc.) vs. those who view them from a programming language perspective (e.g. methods are units, classes are dependencies). I find that defining what is being meant by phrases like "the unit under test" can turn arguments ("You're wrong!") into discussions ("Is this the right level of granularity?"). – Warbo Sep 8 '16 at 15:18
  • 1
    @EricKing Of course, to the very large majority of those in your first category, the very idea of involving the database at all in unit tests is anathema, whether you use your DAOs or hit the database directly. – Periata Breatta Sep 8 '16 at 17:03
  • 1
    @AmaniKilumanga The question was basically "somebody says I shouldn't do this thing in unit tests but I do it in unit tests and I think it's fine. Is that ok?" And my answer was "Yes, but most people would call that an integration test instead of a unit test.". As far as your question, I don't know what you mean by "may integration tests use production code methods?". – Eric King Sep 9 '16 at 13:20
7

The answer is yes and no...

Unique tests that perform in isolation are an absolute must as it allows you to lay down the ground work that your low-level code is functioning appropriately. But in coding larger libraries, you will also find areas of code that will require you have tests that cross units.

These cross-unit tests are good for code coverage and when testing end to end functionality but they do come with a few drawbacks you should be aware of:

  • Without an isolated test to confirm just what is breaking, a failed "cross-unit" test will require additional troubleshooting to determine just what is wrong with your code
  • Relying too much on cross-unit tests can get you out of the contract mindset that you should always be in when writing SOLID Object-Oriented code. Isolated tests typically make sense because your units should only be performing just one basic action.
  • End to end testing is desirable but can be dangerous if a test require you to write to a database or perform some action that you wouldn't want to occur in a production environment. This one of the many reasons why mocking frameworks like Mockito are so popular because it allows you to fake an object and mimic an end to end test without actually changing something you shouldn't.

At the end of the day you want to have some of both.. a lot of tests that trap low level functionality and a few that test end to end functionality. Focus on the reason why you are writing tests in the first place. They are there to give you confidence that your code is performing the way you expect it to. Whatever you need to do to make that happen is just fine.

The sad but true fact is if you are using automated tests at all then you already have a leg up on a lot of developers. Good testing practices is the first thing to slide when a development team is faced with tough deadlines. So as long as you are sticking to your guns and writing unit tests that are a meaningful reflection of how your code should perform, I wouldn't obsess on just how "pure" your tests are.

4

One issue with using other object methods to test a condition is that you will miss errors that cancel each other out. More importantly, you're missing out on the pain of a difficult test implementation, and that pain is teaching you something about your underlying code. Painful tests now mean painful maintenance later.

Make your units smaller, split your class, refactor, and redesign until it's easier to test without reimplementing the rest of your object. Get some help if you think your code or tests are unable to be simplified any further. Try to think of a colleague who seems to always luck out and get assignments that are easy to test cleanly. Ask him or her for help, because it isn't luck.

  • 1
    Key here is in unit. If you need to make calls to other procedures and processes it would not be, to me, a unit. If multiple calls are made and multiple steps need to be validated, then if it a large piece of code than a unit, break it into smaller pieces covering a single piece of logic. Normally, a DB call would also be mocked or an in memory DB would be used in the unit test over accessing a real DB and needing to undo data after the test. – dlb Sep 6 '16 at 21:20
1

If the unit of functionality being tested is 'is the data stored persistently and retrievable', then I would have the unit test test that - store to a real database, destroy any objects holding references to the database, then call the code to fetch the objects back.

Testing whether a record is created in the database seems to be concerned with the implementation details of the storage mechanism rather than testing the unit of functionality exposed to the rest of the system.

You might want to shortcut the test to improve performance using a mock database, but I have had contractors who did exactly that and left at the end of their contract with a system which passed such tests, but didn't actually store anything in the database between system reboots.

You can argue whether 'unit' in unit test denotes a 'unit of code' or 'unit of functionality', functionality perhaps created by many code units in concert. I don't find this a useful distinction - the questions I'd keep in mind are 'does the test tell you something about whether the system provides business value', and 'is the test brittle if the implementation changes?' Tests like the one described are useful when you are TDDing the system - you haven't yet written the 'get object from database record' yet so can't test the full unit of functionality - but are brittle against implementation changes so I'd remove them once the full operation is testable.

1

The spirit is correct.

Ideally, in a unit test, you are testing a unit (a individual method or small class).

Ideally, you would stub out the whole database system. I.e., you would run your method in a faked environment and just make sure that it calls the correct DB APIs in the correct order. You explicitely, positively do not want to test the DB when testing one of your own methods.

Benefits are many. Most of all, tests get blinding fast because you do not need to bother about setting up a correct DB environment and rolling it back afterwards.

Of course, this is a lofty goal in any software project that does not do this right from the get-go. But it is the spirit of unit testing.

Note that there are other tests, like feature tests, behaviour tests etc. that are different from this approach - don't confuse "testing" with "unit testing".

  • "just make sure that it calls the correct DB APIs in the correct order" — yeah, sure. Until it turns out you've misread the documentation and the actual correct order you should have been using is completely different. Don't mock systems outside of your control. – Joker_vD Sep 7 '16 at 14:48
  • 4
    @Joker_vD That's what integration tests are for. In unit tests, external systems absolutely should be mocked. – Ben Aaronson Sep 7 '16 at 15:00
1

There is at least one very important reason not to use direct database access in your unit tests for business code that interacts with the database: if you change your database implementation, you'll have to rewrite all of those unit tests. Rename a column, for your code you just have to change a single line in your data mapper definition. If you don't use your data mapper while testing, however, you'll then find you also have to change every single unit test that references this column. This could turn into an extraordinary amount of work, particularly for more complex changes that are less amenable to search-and-replaced.

Also, using your data mapper as an abstract mechanism rather than talking to the database directly makes it easier to remove the dependency on the database entirely, which might not be relevant now, but when you get up to thousands of unit tests and all of them are hitting a database, you'll be thankful that you can easily refactor them to remove that dependency because your test suite will drop from taking minutes to run to taking seconds, which can have a huge benefit on your productivity.

Your question indicates that you are thinking along the right lines. As you suspect, unit tests are code too. It is just as important to make them maintainable and easy to adapt to future changes as for the rest of your code base. Strive to keep them readable and to eliminate duplication, and you'll have a much better test suite for it. And a good test suite is one that gets used. And a test suite that gets used helps to find errors, while one that doesn't get used is just worthless.

1

The best lesson I learned when learning about unit testing and integration testing is to not test methods, but test behaviors. In other words, what does this object do?

When I look at it that way, a method which persists data and another method which reads it back start to be testable. If you are testing the methods specifically, of course, you end up with a test for each method in isolation such as:

@Test
public void canSaveData() {
    writeDataToDatabase();
    // what can you assert - the only expectation you can have here is that an exception was not thrown.
}

@Test
public void canReadData() {
    // how do I even get data in there to read if I cannot call the method which writes it?
}

This problem occurs because of the perspective of testing methods. Don't test methods. Test behaviors. What is the behavior of the WidgetDao class? It persists widgets. Ok, how do you verify it persists widgets? Well, what is the definition of persistence? It means when you write it, you can read it back again. So read + write together become a test, and in my opinion, a more meaningful test.

@Test
public void widgetsCanBeStored() {
    Widget widget = new Widget();
    widget.setXXX.....
    // blah

    widgetDao.storeWidget(widget);
    Widget stored = widgetDao.getWidget(widget.getWidgetId());
    assertEquals(widget, stored);
}

That is a logical, cohesive, reliable, and my in opinion, meaningful test.

The other answers focus how important isolation is, the pragmatic versus realistic debate, and whether a unit test may or may not query a database. Those do not really answer the question though.

To test if something can be stored, you have to store it and then read it back. You cannot test if something is stored if you are not allowed to read it. Don't test the storing of data separately from the retrieval of data. You will end up with tests which don't tell you anything.

  • Very insightful approach, and as you say, I think you really hit one of the core doubts I had. Thanks! – carlossierra Sep 8 '16 at 21:13
0

One example of a failure case you'd prefet to catch, is that the object under test uses a caching layer but fails to persist the data as required. Then if you query the object it'll say "yup, I've got the new name and address", but you want the test to fail because it hasn't actually done what it was supposed to.

Alternatively (and overlooking the single-responsibility violation), suppose it's required to persist a UTF-8-encoded version of the string to a byte-oriented field, but actually persists Shift JIS. Some other component is going to read the database and expects to see UTF-8, hence the requirement. Then the round-trip through this object will report the correct name and address because it'll convert it back from Shift JIS, but the error isn't detected by your test. It hopefully will be detected by some later integration test, but the whole point of unit tests is to catch problems early, and know exactly what component is responsible.

If one of them is not doing what it's supposed to, then its own test case will fail and we can fix it and run the test battery again.

You can't assume this, because if you're not careful you write a mutually dependent set of tests. The "does it save?" test calls the save method it's testing, and then the load method to confirm it saved. The "does it load?" test calls the save method to set up the test fixture and then the load method it's testing to check the result. Both tests rely on the correctness of the method they aren't testing, meaning that neither of them actually tests the correctness of the method it is testing.

The clue that there's a problem here, is that two tests that supposedly are testing different units actually do the same thing. They both call a setter followed by a getter, then check the result is the original value. But you wanted to test that the setter persists the data, not that the setter/getter pair works together. So you know something's wrong, you just have to figure out what and fix the tests.

If your code is well-designed for unit testing, then there are at least two ways you can test whether the data has really been correctly persisted by the method under test:

  • mock the database interface, and have your mock record the fact that the proper functions have been called on it, with the expected values. This tests the method does what it's supposed to, and is the classic unit test.

  • pass it an actual database with exactly the same intention, to record whether or not the data has been correctly persisted. But rather than having a mocked function that just says "yup, I got the right data", your test reads back out of the database directly and confirms it's correct. This may not be the purest possible test, because an entire database engine is quite a big thing to use to write a glorified mock, with more chance of me overlooking some subtlety that makes a test pass even though something is wrong (so for example I shouldn't use the same database connection to read as was used to write, because I might see an uncommitted transaction). But it tests the right thing, and at least you know that it precisely implements the entire database interface without having to write any mock code!

So it is a mere detail of test implementation whether I read the data out of the test database by JDBC or whether I mock the database. Either way the point is that I can test the unit better by isolating it than I can if I allow it to conspire with other incorrect methods on the same class to look right even when something is wrong. Therefore I want to use any convenient means to check that the correct data was persisted, other than trust the component whose method I'm testing.

If your code is not well-designed for unit testing, then you may have no choice, because the the object whose method you want to test might not accept the database as an injected dependency. In which case the discussion about the best way to isolate the unit under test, changes into a discussion about how close it's possible to get to isolating the unit under test. The conclusion is the same, though. If you can avoid conspiracies among faulty units then you do, subject to available time and to anything else you think of that would be more effective at finding faults in the code.

0

This is how I like to do it. This is just a personnal choice since this won't impact the result of the product, only the way to produce it.

This depend on the possibilites to be able to add mock values in your database without the application you are testing.

Let's say you have two tests : A - reading the database B - insert into the database ( depends on A)

If A pass, then you are sure that the result of B would depends on the part tested in B and not the dependencies. If A failes, you can have a false negative for B. The error might be in B or in A. You can't know for sure until A return a success.

I would not try to write some queries in a unit-testing since you should not be able to know the DB structure behind the application (or it might change). Meaning your test case could failed because of your code itself. Unless you write a test for the test, but who will test the test for the test...

-1

In the end, it comes down to what you want to test.

Sometimes it is enough to test the provided interface of your class (e.g. record gets created and stored and can be retrieved later, maybe after a "restart"). In this case it is fine (and usually a good idea) to use the provided methods for testing. This enables the tests to be reused, for example if you want to change the storage mechanism (different database, in-memory database for testing, ...).

Note that this means you only really care that the class is adhering to its contract (creating, storing and retrieving records in this case), not how it achieves this. If you create your unit tests smartly (against an interface, not a class directly), you can test different implementations, as they also have to adhere to the contract of the interface.

On the other hand, in some cases you have to actually verify how stuff gets persisted (maybe some other process relies on the data being in the right format in that database?). Now you can't just rely on getting the correct record back from the class, instead you have to verify that it is correctly stored in the database. And the most direct way to do that is querying the database itself.

Now you don't only care about the class adhering to its contract (create, store and retrieve), but also how it achieves this (since that persisted data needs to adhere to another interface). However, now the tests are dependent on both the class interface and the storage format, so it will be hard to reuse them fully. (Some might call these kinds of tests "integration tests".)

  • @downvoters: might you tell me your reasons so I can improve on the aspects you think my answer is lacking? – hoffmale Sep 9 '16 at 5:41

protected by gnat Sep 8 '16 at 13:45

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

Not the answer you're looking for? Browse other questions tagged or ask your own question.