How to avoid duplicate logic with Mocks

Question

I have the following challenge, and I haven't found a good answer. I am using a Mocking framework (JMock in this case) to allow unit tests to be isolated from database code. I'm mocking the access to the classes that involve the database logic, and seperately testing the database classes using DBUnit.

The problem I'm having is that I'm noticing a pattern where the logic is conceptually duplicated in multiple places. For example I need to detect that a value in the database doesn't exist, so I might return null from a method in that case. So I have a database access class which does the database interaction, and returns null appropriately. Then I have the business logic class which receives null from the mock and then is tested to act appropriately if the value is null.

Now what if in the future that behavior needs to change and returning null is no longer appropriate, say because the state has grown more complicated, so I'll need to return an object that reports the value doesn't exist and some additional fact from the database.

Now, if I change the behavior of the database class to no longer return null in that case, the business logic class would still appear to function, and the bug would only be caught in QA, unless someone remembered the coupling, or properly followed the usages of the method.

I fell like I'm missing something, and there has to be a better way to avoid this conceptual duplication, or at least have it under test so that if it changes, the fact that the change is not propagated fails a unit test.

Any suggestions?

UPDATE:

Let me try to clarify my question. I'm thinking of when code evolves over time, how to ensure that the integration doesn't break between the classes tested via the mock and actual implementation of the classed that the mock represents.

For example, I just had a case where I had a method that was originally created and didn't expect null values, so this was not a test on the real object. Then the user of the class (tested via a mock) was enhanced to pass in a null as a parameter under certain circumstances. On integration that broke, because the real class wasn't tested for null. Now when building these classes at first this is not a big deal, because you are testing both ends as you build, but if the design needs to evolve two months later when you tend to forget about the details, how would you test the interaction between these two sets of objects (the one tested via a mock vs the actual implementation)?

The underlying problem seems to be one of duplication (that is violating the DRY principle), the expectations are really kept in two places, although the relationship is conceptual, there is no actual duplicate code.

[Edit after Aaron Digulla's second edit on his answer]:

Right, that is exactly the kind of thing I am doing (except that there is some further interaction with the DB in a class that is tested via DBUnit and interacts with the database during its tests, but it is the same idea). So now, say we need to modify the database behavior so that the results are different. The test using the mock will continue to pass unless 1) someone remembers or 2) it breaks in integration. So the stored procedure return values (say) of the database are essentially duplicated in the test data of the mock. Now what bothers me about the duplication is that the logic is duplicated, and it is a subtle violation of DRY. It could be that that is just the way it is (there is a reason for integration tests after all), but I was feeling that instead I'm missing something.

[Edit on starting the bounty]

Reading the interact with Aaron gets to the point of the question, but what I'm really looking for is some insight into how to avoid or manage the apparent duplication, so that a change in the behavior of the real class will show up in the unit tests that interact with the mock as something that broke. Obviously that doesn't happen automatically, but there may be a way to design the scenario correctly.

[Edit on awarding the bounty]

Thanks to everyone who spent the time answering the question. The winner taught me something new about how to think about passing the data between the two layers, and got to the answer first.

Answer 1

Unit tests can't tell you when a method suddenly has a smaller set of possible results. That's what code coverage is for: It will tell you that code isn't executed anymore. This in turn will lead to the discovery of the dead code in the application layer.

[EDIT] Based on a comment: A mock must not do anything but allowing to instantiate the class under test and allow to collect additional information. Especially, it must never influences the result of what you want to test.

[EDIT2] Mocking a database means that you don't care whether the DB driver works. What you want to know is whether your code can interpret the data returned by the DB correctly. Also, this is the only way to test whether your error handling works correctly because you can't tell the real DB driver "when you see this SQL, throw this error." This is only possible with a mock.

I agree, it takes some time to get used to. Here is what I do:

• I have tests which check whether the SQL works. Each SQL gets executed once against a static test DB and I verify that the data returned is what I expect.

• All other tests run with a mock DB connector that returns predefined results. I like to get these results by running the code against the database, logging the primary keys somewhere. I then write a tool which takes these primary keys and dumps Java code with the mock to System.out. This way, I can create new test cases very quickly and the test cases will reflect the "truth".

  Even better, I can recreate old tests (when the DB changes) by running the old IDs and my tool again

Answer 2

For the specific scenario, you are changing the return type of the method, that will be caught at compile time. If it didn't it would appear on code coverage (as mentioned by Aaron). Even then, you should have automated functional tests, which would be run soon after the check-in. That said, I do automated smoke tests, so in my case those would caught that :).

Without thinking on the above, you still have 2 important factors playing in the initial scenario. You want to give your unit testing code the same attention as the rest of the code, which means it is reasonable to want to keep them DRY. If you were doing TDD, that would even push this concern to your design in the first place. If you are not into that, the other contrary factor involved is YAGNI, you don't want to get every (un)likely scenario in your code. So, for me it would be: if my tests are telling me I am missing something, I double check the test is ok and proceed with the change. I make sure not to do what if scenarios with my tests, as it is a trap.

Answer 3

Your question is quite confusing, and the amount of text doesn't exactly help.

But the meaning I could extract through a quick read makes little sense to me, in that you want a non-contract changing change affect how the mock works.

Mocking is an enabler for you to concentrate on testing a specific part of the system. The mocked part will always work in a specified way, and the test can concentrate on testing the specific logic it should. Thus you won't be affected by unrelated logic, latency issues, unexpected data, etc.

You'll probably have a separate number of tests checking the mocked functionality in another context.

The point is, no connection should exist between the mocked interface and the real implementation of that at all. It just doesn't make any sense, since you're mocking the contract and are giving it an implementation of your own.

Answer 4

You are fundamentally asking for the impossible. You are asking for your unit tests to predict and notify you when you change the external resource's behaviour. Without writing a test to produce the new behaviour, how can they know?

What you are describing is adding a brand new state that must be tested for - instead of a null result, now there is some object coming out of the database. How could your test suite possibly know what the intended behaviour of the object under test should be for some new, random object? You need to write a new test.

The mock is not "misbehaving", as you commented. The mock is doing exactly what you set it up to do. The fact that the specification changed is of no consequence to the mock. The only problem in this scenario is that the person who implemented the change forgot to update the unit tests. I'm actually not too sure why you think there is any duplication of concerns going on.

The coder that is adding some new return result to the system is responsible for adding a unit test to handle this case. If that code is also 100% sure that there is no way that the null result could possibly be returned now, then he could also delete the old unit test. But why would you? The unit test correctly describes the behavior of the object under test when it receives a null result. What happens if you change the backend of your system to some new database that does return a null? What if the specification changed back to returning null? You might as well keep the test, since as far as your object is concerned, it could really get anything back from the external resource, and it should gracefully handle every possible case.

The whole purpose of mocking is to decouple your tests from real resources. It's not going to automatically save you from introducing bugs into the system. If your unit test accurately describes the behavior when it receives a null, great! But this test should not have any knowledge of any other state, and certainly should not be somehow informed that the external resource will no longer be sending nulls.

If you're doing proper, loosely coupled design, your system could have any backend you could imagine. You shouldn't be writing tests with one single external resource in mind. It sounds like you might be happier if you added some integration tests that use your real database, thereby eliminating the mocking layer. This is always a great idea for use with doing a build or sanity/smoke tests, but is usually obstructive for day to day development.

Answer 5

I think your problem is violating the Liskov Substitution Principle:

Subtypes must be substitutable for their base types

Ideally, you would have a class, which depends on an abstraction. An abstraction that says "for being able to work, I need an implementation of this method which takes this parameter, returns this result and if I do this wrong stuff, throws me this exception". These would all be defined on your interface which you depend, either by compile time constraints or by comments.

Technically you may seem to depend on an abstraction but in the scenario you tell, you do not really depend on an abstraction, you actually depend on an implementation. You say that "if this method changes its behavior, its users will break and my tests will never know". On the unit test level, you are right. But on the contract level, changing the behavior in this way is wrong. Because by changing the method, you clearly violate the contract between your method and callers of it.

Why do you change a method? It is clear that callers of that method needs a different behavior now. So, the first thing you want to do is not changing the method itself, but changing the abstraction, or the contract, that your clients depend on. They must change first and begin working with the new contract: "OK, my needs changed, I no longer want this method to return that in this particular scenario, implementors of this interface must return this instead". So, you go change your interface, you go change the users of the interface as necessary, and this includes updating their tests and the last thing you do is changing the actual implementation that you pass to your clients. This way, you will not encounter the error you talk about.

So,

class NeedsWork(IWorker b) { DoSth() { b.Work() }; }
...
AppBuilder() { INeedWork GetA() { return new NeedsWork(new Worker()); } }

1. Modify IWorker so that it reflects new needs of NeedsWork.
2. Modify DoSth so that it works with the new abstraction that satisfies its new needs.
3. Test NeedsWork and make sure it works with the new behavior.
4. Change all the implementations (Worker in this scenario) you provide for IWorker (which you are now trying to do first).
5. Test Worker so that it meets new expectations.

Seems scary but in real life this would be trivial for small changes and painful for huge changes as it, in fact, must be.

Answer 6

You're not missing something here. This is a weakness in unit testing with mock objects. It sounds like you are properly breaking your unit tests down into reasonably sized units. This is a good thing; it's far more common to find people testing too much in a "unit" test.

Unfortunately, when you test at this level of granularity, your unit tests don't cover the interaction between collaborating objects. You need to have some integration tests or functional tests to cover this. I don't really know a better answer than that.

Sometimes it's practical to use the real collaborator instead of a mock in your unit test. For example, if you're unit testing a data access object, using the real domain object in the unit test instead of a mock is often easy enough to set up and performs just as well. The reverse is often not true -- data access objects typically need a database connection, file or network connection and are pretty complicated and time consuming to set up; using a real data object when unit testing your domain object will turn a unit test that takes microseconds into one that takes hundreds or thousands of milliseconds.

So to summarize:

1. Write some integration/functional testing to catch problems with collaborating objects
2. It's not always necessary to mock out collaborators -- use your best judgement

Answer 7

I would like to narrow the problem down to it's core.

The Problem

Of course, most of your changes will be caught by the test.
But there is subset of Scenarios where your test won't fail - although it should:

As you write code, you use your methods multiple times. You get a 1:n relation between method definition and use. Each class that uses that method will use it's mock in the according test. So the mock is also used n times.

Your methods result was once expected to never be null. After you change this, you probably will remember to fix the according test. So far so good.

You run your Tests - all pass.

But over time you forgot something ... the mock never returns a null. So n test for n classes that use the mock do not test for null.

Your QA will fail - although your tests did not fail.

Obviously you will have to modify your other tests. But there are no fails to work along. So you need a solution, that works better than remembering all referencing tests.

A Solution

To avoid problems like this, you will have to write better tests from the beginning. If you miss out the cases, where the tested class should handle errors or null values, you simply have incomplete tests. It's like not testing all functions of your class.

It's hard to add this later. - So start early and be extensive with your tests.

As mentioned by other users - the code coverage reveals some untested cases. But missing error-handling code and the missing according test won't appear in code coverage. (Code coverage of 100% doesn't mean, that you are not missing something.)

So write good test: Assume the outside world to be malicious. That does not only include to pass bad parameters (like null values). Your mocks are a part of the outside world too. Pass nulls and exceptions - and watch your class handling them as expected.

If you decide null to be a valid value - these test will later fail (because of missing exceptions). So you get a list of fails to work along.

Because each calling class handles the errors or null different - it is not duplicate code that could be avoided. Different treatment needs different tests.

---

Hint: Keep your mock simple and clean. Move the expected return values to the testing method. (Your mock can pass them simply back.) Avoid testing decisions in mocks.

Answer 8

Your database abstraction uses null to mean "no results found". Ignoring the fact that it's a bad idea to pass null between objects, your tests should not use that null literal when they want to test what happens when nothing is found. Instead, use a constant or a test data builder so that your tests only refer to what information is passed between objects, not to how that information is represented. Then if you need to change the way that the database layer represents "no results found" (or whatever information your test relies upon) you only have one place in your tests to change that.

Answer 9

Here's how I understand your question:

You are using mock objects of your entities to test the business layer of your application using JMock. You are also testing your DAO layer (the interface between your app and your database) using DBUnit, and passing real copies of your entity objects populated with a known set of values. Because you are using 2 different methods of preparing your test objects, your code is violating DRY, and you risk your tests getting out of sync with reality as code changes.

Folwer says...

Its not exactly the same, but it certainly reminds me of Martin Fowler's Mocks Aren't Stubs article. I see the JMock route as being the mockist way, and the 'real objects' route as being the classicist way to perform testing.

One way to be as DRY as possible when tackling this problem is to be more of a classicist then a mockist. Maybe you can compromise and use real copies of your bean objects in your tests.

User Makers to avoid duplication

What we have done on one project is to create Makers for each of our business objects. The maker contains static methods which will construct a copy of a given entity object, populated with known values. Then, whichever kind of object you need, you can call the maker for that object and get a copy of it with known values to use for your testing. If that object has child objects, your maker will call makers for the children in order to construct it from top to bottom, and you will get back as much of the complete object graph as you need. You can use these maker objects for all of your tests -- passing them to the DB when testing your DAO layer, as well as passing them to your service calls when testing your business services. Because the makers are reusable, its a fairly DRY approach.

One thing you will still need to use JMock for, however, is to mock your DAO layer when testing your service layer. If your service makes a call to the DAO, you should make sure it is injected with a mock instead. But you can still use your Makers just the same -- when set up your expectations, just make sure your mocked DAO passes back the expected result using the Maker for the relevant entity object. That way we still aren't violating DRY.

Well written tests will notify you when code changes

My final advice to avoid your problem with code changing over time is to always have a test that addresses null inputs. Suppose when you first create your method nulls are not acceptable. You should have a test that verifies that an exception is thrown if null is used. If at a later time, nulls become acceptable, your app code might change so that null values are handled in a new way, and the exception is no longer thrown. When that happens, your test will begin to fail, and you will have a "heads up" that things are out of sync.

Answer 10

You simply need to make up your mind of wether the returning of null is an intended part of the external API or if it is an implementation detail.

Unit tests should not care about implementation details.

If it is part of your intended external API, then as your change would potentially break clients, this naturally also should break the unit test.

Does it make sense from an external POV that this thing returns NULL or is this a convenient consequence because direct assumptions can be made in the client as to the meaning of this NULL? A NULL should mean void/nix/nada/unavailable without any other meaning.

If you plan on granulating this condition later, then you should wrap the NULL check into something that returns either an informative exception, enum or an explicitly named bool.

One of the challenges with writing unit tests is that even the first unit tests written should reflect the complete API in the end product. You need to visualize the complete API and then program against THAT.

Also, you need to maintain the same discipline in your unit test code as you do in the production code avoiding smells like duplication and feature envy.

Answer 11

If I understand the question correctly, you have a Business Object that uses a Model. There is a test for the interaction between the BO and Model (Test A), and there is another test that tests the interaction between the model and the database (Test B). Test B changes to return an object, but that change doesn't effect test A because test A's model is mocked.

The only way I see to make test A fail when test B changes is to not mock the model in test A and combine the two into a single test, which isn't good because you'll be testing too much (and you're using different frameworks).

If you know about this dependency when you write the tests, I think an acceptable solution would be to leave a comment in each test describing the dependency and how if one changes, you need to change the other. You'll have to change Test B when you refactor anyway, the current test will fail as soon as you make your change.