1

I'm trying to mock the DateTimeFormatter class. I've done the following:

@RunWith(PowerMockRunner.class)
@PrepareForTest({DateTimeFormatter.class})
public class UnitTest {

private DateTimeFormatter mockDateFormatter;

private AwesomeClass awesomeClass;

@Before
public void setUp() {
    mockDateFormatter = PowerMockito.mock(DateTimeFormatter.class);
    awesomeClass = new AwesomeClass(mockDateFormatter);
}

@Test
public void shouldToTestSomethingAwesome() {
   // Other test code
    PowerMockito.when(mockDateFormatter.format(any(LocalDate.class)))
                    .thenReturn("20150224");
   // Other test code

}

AwesomeClass uses it to format LocalDateTime.now(ZoneId.of("UTC"));. The formatted string is then further used to generate another string. I need to ensure that the string is properly generated. So I need to return a consistent date from either the formatter or mock the LocalDateTime.now(..) static method

What am I doing wrong?

Varun Achar
  • 14,781
  • 7
  • 57
  • 74
  • 6
    *Why* do you want to mock `DateTimeFormatter`, out of interest? Why would you not just use a real one? I would be wary of over-mocking - it can make tests a lot more fragile (and hard to read) than simply using the real code or fakes with the approximate same behaviour. – Jon Skeet Feb 24 '15 at 13:47
  • 1
    Sounds like unit testing run a-mock to me.... – duffymo Feb 24 '15 at 13:47
  • The class into which I'm passing the formatter uses it to format `LocalDateTime.now(ZoneId.of("UTC"));`. The formatted string is then further used to generate another string. I need to ensure that the string is properly generated. So I need to return a consistent date from either the formatter or mock the `LocalDateTime.now(..)` static method – Varun Achar Feb 24 '15 at 13:54
  • 1
    Indeed, you should just use a real `DateTimeFormatter`. The argument that you "need to ensure that the string is properly generated" doesn't sound convincing to me. Tests should aim to verify correct behavior, and as much as possible through the public contract of the class under test; surely you can pass correct input and then check the correct outputs and/or outcomes are produced; use of `DateTimeFormatter` is just an implementation detail. (BTW, to make it work with PowerMock you need to prepare the class under test as well; but, again, the best is to not mock at all.) – Rogério Feb 24 '15 at 15:13
  • I already have the `@PrepareForTest({DateTimeFormatter.class})` in the code. Anyway, I've used the approach suggested by @assylias – Varun Achar Feb 25 '15 at 08:52

2 Answers2

8

On the mockito wiki : Don't mock types you don't own !

This is not a hard line, but crossing this line may have repercussions! (it most likely will.)

  1. Imagine code that mocks a third party lib. After a particular upgrade of a third library, the logic might change a bit, but the test suite will execute just fine, because it's mocked. So later on, thinking everything is good to go, the build-wall is green after all, the software is deployed and... Boom
  2. It may be a sign that the current design is not decoupled enough from this third party library.
  3. Also another issue is that the third party lib might be complex and require a lot of mocks to even work properly. That leads to overly specified tests and complex fixtures, which in itself compromises the compact and readable goal. Or to tests which do not cover the code enough, because of the complexity to mock the external system.

Instead, the most common way is to create wrappers around the external lib/system, though one should be aware of the risk of abstraction leakage, where too much low level API, concepts or exceptions, goes beyond the boundary of the wrapper. In order to verify integration with the third party library, write integration tests, and make them as compact and readable as possible as well.

Mock type that you don't have the control can be considered a (mocking) antipattern. While DataTimeFormatter is pretty much standard, one should not consider there won't be any behavior change in upcoming JDK releases (it already happened numerous time in other part of the API, just look at the JDK release notes).

My point is that if the code needs to mock a type I don't own, the design should change asap so I, my colleagues or future maintainers of this code won't fall in these traps.

Also the wiki links to other blogs entries describing issues they had when they tried to mock type they didn't have control.

bric3
  • 40,072
  • 9
  • 91
  • 111
  • It's the year 2018, and your link is dead. What does this answer really mean? Who knows! – Duncan Jones Feb 24 '15 at 14:29
  • Either flesh out your answer with some info from that page or change this to a comment under the question, where the same rules of quality don't apply. Links **do** break, so one day your answer may just read "*Don't mock types you don't own !*", which isn't that helpful. – Duncan Jones Feb 24 '15 at 14:36
  • I didn't understand that from your first comment. But you're right I'll paste in some comment. – bric3 Feb 24 '15 at 14:39
  • Ah, I see. That'll teach me to try and be humorous. (*Yes, that was me trying to be funny...*) – Duncan Jones Feb 24 '15 at 14:39
  • @Duncan, thanks for the feedback anyway, I think it's more suited. I see a lot of these bad mock design principles, that I solely wanted to ink this wiki page we set up some time ago. – bric3 Feb 24 '15 at 14:53
  • 1
    To me, this advice to not mock types you don't own would be better said as "favor *integration* tests". In my experience, mocking is way too often abused and misused; there are several other ways to do it wrong than just mocking 3rd-party libs (for example, mocking value types or collection/map interfaces). Of course, sometimes you do have a legitimate need to mock 3rd-party code (for example, JSF's FacesContext class), but even in these cases it's usually best to minimize mocking while writing an integration test. – Rogério Feb 24 '15 at 15:08
  • I agree @Rogério, too many stuff can go wrong in unit test. In this case integration test should indeed be favored. At the unit level though those traps are probable design issues, for exemple DI is often abused and misused this has a bad side effect in tests too regarding readability, simplicity, maintainability. – bric3 Feb 24 '15 at 15:45
4

An alternative to mocking LocalDateTime.now() is to inject the clock into your class and change your (or add another) constructor like this:

AwesomeClass(DateTimeFormatter fmt, Clock clock) {
  //instead of LocalDateTime now = LocalDateTime.now():
  LocalDateTime now = LocalDateTime.now(clock);
}

Then in your test:

new AwesomeClass(formatter, Clock.fixed(the time you want here));
assylias
  • 321,522
  • 82
  • 660
  • 783
  • 1
    In case it's not clear, it's perfectly reasonable to add a second constructor (package-private) for the purposes of your test. Your existing constructor can then be altered to call the package-private one with the argument `Clock.systemUTC()`, for example. – Duncan Jones Feb 24 '15 at 14:41