The major concern here is that mocks can't (or shouldn't) return mocks. This is probably good advice, but talks around a solution: return a real
Message. If the
Message class is well-tested and passing, you can consider it to be just as friendly as a mock. Perhaps it is even friendlier as it will respond like the real thing because it is the real thing.
What kind of real
Messages can you return? Well, you can return a full-fledged real
Message, a simplified real
Message (wherein well-known defaults are used), or you can return a
NullMessage (as in the Null Object Pattern). A
NullMessage is just as valid a
Message as any other, and can be dropped in anywhere else in your application. Which one to use depends on the complexity of creating and returning a full message.
As to the Law of Demeter, there are multiple concerns here. First, your constructor takes its own builder as a parameter, then extracts elements from it. This is a clear violation of Demeter, and also creates a superfluous dependency. Worse yet, the builder is acting as a mini service locator, masking the real dependencies of the class. The
OrderBuilder should create these objects and pass them in as their own parameters.
In order to test this, then, you would pass in a mock
MessageFactory, which returns a real
Message (either full, simple, or null), and a mock
MessageLayer that takes the message. If you use a full or simplified
Message, you could get it back from your
MessageLayer mock and inspect it for testing assertions.
I would also look at the
MessageLayer as a functionality clump at a different level of abstraction, and so I would extract a
MessageSender class that encapsulated that functionality. You could test this class by using a simple mock
MessageSender, and shift everything I talked about above into the
MessageSender's tests, thereby adhering more closely to Single Responsibility as well.
I see there are really two questions here. There is a specific question of how to test this code, and a general question about mocks returning mocks. The specific question is what I dealt with above to a larger extent, and I have more thoughts at the end of here about it now that some more details have come to light, but there is not really a good answer yet to the general question: Why should mocks not return mocks?
The reason mocks should not return mocks is that you can end up testing your tests rather than testing your code. Instead of just making sure that the unit is fully functional, the test now depends on a whole new piece of code found only in the test case itself (which often is itself not tested). This creates two problems.
First, the test now cannot tell me for sure if the unit is broken or if the interrelated mocks are broken. The whole point of a test is to create an isolated environment where there should be only one cause for failure. A mock on its own is generally very simple and can be inspected directly for problems, but wiring multiple mocks together like this becomes exponentially harder to confirm by inspection.
The second problem is, as APIs change for the real objects, tests may start failing far away since the mocks do not automatically change as well. The Law of Demeter comes into play here, as these are exactly the type of effects following the law avoids. In my tests, I would have to worry about keeping in sync not only the mocks of direct dependencies, but also the mocks of dependencies of dependencies ad infinitum. This has the effect of shotgun surgery on the tests when classes change.
Now, as to the specific question of how to test this particular piece of code, let's break down some assumptions.
Question 1: What are we really testing? While this is an abbreviated portion of the code, we can see three essential activities going on here. First, we have a factory generating a
Message. We aren't testing whether the factory is producing the
Message, as you're already mocking that out. We're not testing the
Message, as it should be tested elsewhere, presumably in a suite of tests for the third-party API that generates the
Message. In the second line, we can see from inspection that the method is simply called on the
Message and so there is really nothing to test in the second line. Once again, there should be tests elsewhere that make testing this redundant. The third line calls the
MessageLayer API, and simply passes through the result. Once again,
MessageLayer's API should already be tested elsewhere. This leaves us with essentially nothing to test. There are no direct visible side effects to the external code, and we should not be testing internal implementation. That leads us to the one conclusion that it would be inappropriate to test this code at all. (For more on this line of reasoning, see Sandi Metz's presentation Magic Tricks of Testing, [slides, video])
Question 2: Wait, so then...wha?? Yes, that's right, don't test this at all. Now, as mentioned, this is an abbreviated version of the code. If you have other logic, test that, but encapsulate this into a separate unit (like the
MessageSender implementation mentioned above). You can then mock this entire aspect of the code easily, while still having the ability to test other logic.
You are basically using a third-party API directly in your code. Third-party code is notoriously hard to test because it can have these types of dependency issues you have here. Encapsulating it off into a corralled area can make it easier to test your other code, and reduce shotgun surgery if that third-party changes their code (or just changes). While there may still be pain in testing the part that interacts with the third-party API, it is limited to one small facet that you can isolate.