Статьи

На PowerMock Злоупотребление

Июнь 16, 2017

Логотип PowerMock

Все еще работаю над своим унаследованным приложением и пытаюсь  улучшить  юнит-тесты .

На этой неделе я заметил, сколько PowerMock использовалось во время тестов, чтобы высмеивать статические или частные методы. В одном конкретном пакете его удаление улучшило время выполнения тестов на один порядок (примерно с 20 секунд до 2). Это явно злоупотребление: я видел три основные причины использования PowerMock.

Недостаток знаний API

Вероятно, должны были быть веские причины, но некоторых из применений PowerMock можно было бы избежать, если бы разработчики только что проверили базовый код. Одним из примеров такого кода было следующее:

@RunWith(PowerMockRunner.class)
@PrepareForTest(SecurityContextHolder.class)
public class ExampleTest {

    @Mock private SecurityContext securityContext;

    public void setUp() throws Exception {
        mockStatic(SecurityContextHolder.class);
        when(SecurityContextHolder.getContext()).thenReturn(securityContext);
    }

    // Rest of the test
}

SecurityContextHolder Простой взгляд на Spring  показывает, что у него есть метод setContext (), так что предыдущий фрагмент можно легко заменить на:

@RunWith(MockitoJUnitRunner.class)
public class ExampleTest {

    @Mock private SecurityContext securityContext;

    public void setUp() throws Exception {
        SecurityContextHolder.setContext(securityContext);
    }

    // Rest of the test
}

Другой общий фрагмент, который я заметил, был следующим:

@RunWith(PowerMockRunner.class)
@PrepareForTest(WebApplicationContextUtils.class)
public class ExampleTest {

    @Mock private WebApplicationContext wac;

    public void setUp() throws Exception {
        mockStatic(WebApplicationContextUtils.class);
        when(WebApplicationContextUtils.getWebApplicationContext(any(ServletContext.class))).thenReturn(wac);
    }

    // Rest of the test
}

Хотя немного сложнее , чем в предыдущем примере, глядя на  исходный код  из  WebApplicationContextUtils показывает она выглядит в контексте сервлета для контекста.

Код тестирования можно легко изменить, чтобы удалить PowerMock:

@RunWith(MockitoJUnitRunner.class)
public class ExampleTest {

    @Mock private WebApplicationContext wac;
    @Mock private ServletContext sc;

    public void setUp() throws Exception {
        when(sc.getAttribute(WebApplicationContext.ROOT_WEB_APPLICATION_CONTEXT_ATTRIBUTE)).thenReturn(wac);
    }

    // Rest of the test
}

Слишком строгая видимость

As seen above, good frameworks — such as Spring, make it easy to use them in tests. Unfortunately, the same cannot always be said of our code. In this case, I removed PowerMock by widening the visibility of methods and classes from private (or package) to public.

You could argue that breaking encapsulation to improve tests is wrong, but in this case, I tend to agree with Uncle Bob:

Tests trump Encapsulation.

In fact, you think your encapsulation prevents other developers from misusing your code. Yet, you break it with reflection within your tests. What guarantees developers won’t use reflection the same way in production code?

A pragmatic solution is to compromise your design a bit but — and that’s the heart of the matter, document it. Guava and Fest libraries have both a @VisibleForTesting annotation that I find quite convenient. Icing on the cake would be for IDEs to recognize it and not propose auto-completion in src/main/java folder.

Direct Usage of Static Methods

This last point has been explained times and times again, but some developers still fail to apply it correctly. Some very common APIs offer only static methods and they have no alternatives e.g. Locale.getDefault() or Calendar.getInstance(). Such methods shouldn’t be called directly on your production code, or they’ll make your design testable only with PowerMock.

public class UntestableFoo {

    public void doStuff() {
        Calendar cal = Calendar.getInstance();
        // Do stuff on calendar;
    }
}

@RunWith(PowerMock.class)
@PrepareForTest(Calendar.class)
public class UntestableFooTest {

    @Mock
    private Calendar cal;

    private UntestableFoo foo;

    @Before
    public void setUp() {
        mockStatic(Calendar.class);
        when(Calendar.getInstance()).thenReturn(cal);
        // Stub cal accordingly
        foo = new UntestableFoo();
    }

    // Add relevant test methods
}

To fix this design flaw, simply use injection and more precisely constructor injection:

public class TestableFoo {

    private final Calendar calendar;

    public TestableFoo(Calendar calendar) {
        this.calendar = calendar;
    }

    public void doStuff() {
        // Do stuff on calendar;
    }
}

@RunWith(MockitoJUnitRunner.class)
public class TestableFooTest {

    @Mock
    private Calendar cal;

    private TestableFoo foo;

    @Before
    public void setUp() {
        // Stub cal accordingly
        foo = new TestableFoo(cal);
    }

    // Add relevant test methods
}

At this point, the only question is how to create the instance in the first place. Quite easily, depending on your injection method: Spring @Bean methods, CDI @Inject Provider methods or calling the getInstance() method in one of your own. Here’s the Spring way:

@Configuration
public class MyConfiguration {

    @Bean
    public Calendar calendar() {
        return Calendar.getInstance();
    }

    @Bean
    public TestableFoo foo() {
        return new TestableFoo(calendar());
    }
}

Conclusion

PowerMock is a very powerful and useful tool. But it should only be used when it’s strictly necessary as it has a huge impact on test execution time. In this article, I’ve tried to show how you can do without it in 3 different use-cases: lack of knowledge of the API, too strict visibility and direct static method calls. If you notice your test codebase full of PowerMock usages, I suggest you try the aforementioned techniques to get rid of them.

Note: I’ve never been a fan of TDD (probably the subject of another article) but I believe the last 2 points could easily have been avoided if TDD would have been used.

32
Share: