This is the seventeenth part of the Types and Programming Languages series. For your convenience you can find other parts in the table of contents in Part 1 — Do not return in finally
Today a short example of how to break LSP in practice. It’s a trivial code, but it’s relatively easy to fall the trap. Especially with async code around.
Let’s say, that we have a helper method like this one:
|
1 2 3 4 5 6 7 8 9 |
public <T, Client> T doWithOptionalBackupConnection(Client mainClient, Client backupClient, Function<Client, T> action){ try { return action.apply(mainClient); } catch(Exception e){ logger.log(e); } return action.apply(backupClient) } |
We try performing some operation with the first client. If it throws an exception, we try with a backup one. However, that’s just for understanding the context. The actual implementation is irrelevant, we may not know it, it may be inaccessible, whatever. Also, for the purpose of this exercise, we can’t use real doWithOptionalBackupConnection in testing because it’s expensive, slow, inaccessible, it’s legally unusable, etc. It could also be non-deterministic, like this:
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 |
public <T, Client> T doWithOptionalBackupConnection(Client mainClient, Client backupClient, Function<Client, T> action){ if(rand()%2 == 0){ // First main, then backup return doInternal(mainClient, backupClient, action); }else { // First backup, then main return (doInternal, backupClient, mainClient, action); } } private <T, Client> T doInternal(Client mainClient, Client backupClient, Function<Client, T> action){ try { return action.apply(mainClient); } catch(Exception e){ logger.log(e); } return action.apply(backupClient) } |
What is important, though, is the contract, that the second client will be used if lambda throws an exception.
We want to use the code in the following way:
|
1 2 3 4 5 6 7 8 9 10 |
interface Client { String getData(String parameter); } public String doWork(Client mainClient, Client backupClient){ String first = utilInstance.doWithOptionalBackupConnection(mainClient, backupClient, client -> client.getData("First")); String second = utilInstance.doWithOptionalBackupConnection(mainClient, backupClient, client -> client.getData("Second")); String third = utilInstance.doWithOptionalBackupConnection(mainClient, backupClient, client -> client.getData("Third")); return first + second + third; } |
We run the helper method three times, get results, and then do some processing.
Now, let’s think about unit test here. We assume that the helper method doWithOptionalBackupConnection is contained in some external library. There is no need for us to test it, no need to make sure it works, just like we don’t test Math.max or other standard library. So we test our method doWork, make sure it works correctly, and that’s it. How do we test it? We can use some unit test like this one:
|
1 2 3 4 |
@Test public void test(){ assert(doWork(throwingClient, workingClient) == "Expected"); } |
So we just pass two clients, and that’s it. This is how people typically think about such a logic. You can pause here for a second, and think what tests you would implement.
Time goes by. We realize that the method works synchronously, and is too slow. We need to rework it, so we change the code to this one:
|
1 2 3 4 5 6 7 8 9 10 |
interface Client { CompletableFuture<String> getData(String parameter); } public String doWork(Client mainClient, Client backupClient){ CompletableFuture<String> first = utilInstance.doWithOptionalBackupConnection(mainClient, backupClient, client -> client.getData("First")); CompletableFuture<String> second = utilInstance.doWithOptionalBackupConnection(mainClient, backupClient, client -> client.getData("Second")); CompletableFuture<String> third = utilInstance.doWithOptionalBackupConnection(mainClient, backupClient, client -> client.getData("Third")); return first.WaitForResult() + second.WaitForResult() + third.WaitForResult(); } |
We simply replace the synchronous client with the asynchronous one. We run unit tests, they all pass, and then we push to prod. However, the code doesn’t work anymore.
Why?
The answer lies in asynchronous vs synchronous call. When the synchronous client fails, it throws an exception. The helper method catches it, and calls the other client. However, the asynchronous client doesn’t throw exceptions anymore — it simply returns a future with the exception stored inside. This breaks the contract of doWithOptionalBackupConnection method.
How should we write unit tests to capture this situation?
First solution
The important thing here is the contract. doWithOptionalBackupConnection states that the lambda must throw the exception. However, this is something the author of doWithOptionalBackupConnection can’t test nor enforce. You can’t encode this requirement with type system, runtime checks, or unit tests. This contract must be tested and guaranteed by the caller.
So the important part here is that the caller needs to make sure, that the lambda throws in case of an error. However, unit test like this one:
|
1 |
assert(doWork(throwingClient, workingClient) == "Expected"); |
won’t catch it because you mock the clients. When you replace the synchronous client with the asynchronous one your test still works, but you don’t test the actual client at all. You need to test the lambda.
The test we need is the one testing if client.getData("First") throws an exception in case of failures. Not with mocks, but with an actual client implementation. If we had this test in place, we would realize that we can’t make it green with the asynchronous client because that client doesn’t throw. At the same time, we can’t have such a test inside “client package” just like that, because the asynchronous client isn’t supposed to throw. We need to have this unit test near the doWork method, and we need to reimplement it every time we change the client (because different client will have different setup to throw an exception). So we need a test like this one:
|
1 2 3 4 5 6 7 |
@Test public void test(){ // Whatever set up needed for client prepareFailingConditions(); assertThrows((client -> client.getData("First"))(clientUnderTest)); } |
We won’t be able to fix this test when moving from the synchronous client to the asynchronous one because it never throws. Also, we can’t use a mock for client here If we do that, then we just test the mock. The test will pass, but the actual implementation won’t work in production.
It’s crucial to understand, that we need to have this one additional test for the contract. Tests for sync client, async client, and doWithOptionalBackupConnection won’t be enough, because they won’t include test for the async client throwing an exception as the async client doesn’t do that. And that’s as designed because the async client isn’t supposed to throw. It’s the contract that requires it.
Second solution
There is another solution for this problem. Instead of checking the contract in place, we can mock doWithOptionalBackupConnection with some simple implementation (like the one from the top of this article). This will work, but will most likely couple our test with some implementation, so is probably less generic. Also, we need to remember to not change this mock when changing the client. Still, this should work.
Third solution
Another solution: we mock both doWithOptionalBackupConnection and the client, so they are equivalent to the production ones. This will catch the issue, but has a terrible drawback. What if we change the async client to use the sync one under the hood? The production now works, but the test still fails. We need to update the test to represent the same behavior, which is not what we want. We don’t want to couple these two.