0

I face some situations similar to the following simplified one:

@Component class ServiceOne {
    @Autowired ServiceTwo two;

    void act() {
        ...
        two.a();
        ...
    }
}

@Component class ServiceTwo {
    // suppose it has a LOT of methods
    void a() { ... }
    void b() { ... }
    void c() { ... }
...
    void z() { ... }
}

In other words, ServiceOne depends on only one (or a few) methods of ServiceTwo, but ServiceTwo itself has exposed a lot of methods.

Now I need to test ServiceOne.

// (delete this sentence) Shall I mock ServiceTwo? In other words, is the following the best practice, or bad code smell?

Question: Is there a more efficient way which requires less boilerplate code? Does this lead to maintenance issues? Is there an approach which does not force me to mock 9 unrelated methods?

@Test class ServiceOneTest {
    @Component ServiceOne one;
    @MockBean ServiceTwo two; // or use Mockito.mock or sth like that, unrelated to question
    void testAct() {
        when(two.a()).thenReturn(...);
        one.act();
    }
}

I demonstrate the problem using Java Spring, but surely it is more general and is not limited to that. (For example, this is also a problem when I test Flutter code.)

5
  • 2
    Whenever people on this site ask "is this best practice" or "is this a code smell", they are usually asking the wrong question. Please stop doing this. Instead, ask questions "is there a more efficient way which requires less boilerplate code"? Or: "does this lead to maintenance issues?", or (in this case) "is there an approach which does not force me to mock 9 unrelated methods?".
    – Doc Brown
    May 14 at 10:07
  • @DocBrown Thank you! I have updated the question
    – ch271828n
    May 14 at 10:24
  • I think you just discovered one of the motivating principles behind the interface-segregation principle, first-hand!
    – Alexander
    May 14 at 16:11
  • Also note that a mock is not supposed to re-implement the internal behavior of the actual service. A mock is test case–specific; it should only do what that particular test case needs it to do. It should return canned answers required by that particular test scenario. Remember, you're not testing the mock. The mock is just a part of the test setup. So, if your component depends on an interface, if all else fails, you can just manually write an implementation that does only what you need. The tests for the component you're mocking are a completely separate set of tests. May 14 at 19:48
  • ^P.S. For clarity: In my previous comment, I'm using the term "mock" in the more general sense; I'm not distinguishing between different kinds of test doubles - Dummies, Stubs, Spies, and Mocks (in the strict sense). May 14 at 19:52

1 Answer 1

3

A number of options come to mind:

  1. Decompose ServiceTwo into an additional interface that ServiceTwo inherits from. This new parent interface would define the two methods that ServiceOne needs, or a cohesive subset of the 9 methods in ServiceTwo.

    Prefer a cohesive subset of the 9 methods instead of just the two required by ServiceOne so you are not creating an interface for just one method. Still, this is subjective. If those two methods alone are cohesive, then define an interface with two methods and be done with it. Now ServiceOne has a simpler dependency and your tests do not need to mock unnecessary methods.

  2. Don't mock the unnecessary methods. Most mocking frameworks do not require you to mock every method. You should be able to mock only the methods that get called.

  3. Define your own test implementation of ServiceTwo where you provide behavior for two of the methods and throw exceptions in the other methods.

    For languages that support checked exceptions and your mock cannot throw an exception due to the interface not declaring that the method throws, consider returning values from unnecessary methods that would very obviously be wrong. For instance, if the unnecessary method returns a string, return "Not implemented". When returning numeric values, return something that is always invalid, like a negative number or the largest number for that particular return type.

Option 1 might require a little more work, because now you need to name something that represents a subset of methods in ServiceTwo, but it simplifies the interaction between ServiceOne and ServiceTwo. There is nothing wrong with splitting an interface up into multiple smaller, more focused units of behavior and giving them a good name.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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