google / google/guice

Refactor to Eliminate Repetitive Mock Object Creation in `ServletPipelineRequestDispatcherTest`

Open
#1,825 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
12.7k
Forks
1.7k
Avg merge
11m
Merged PRs (30d)
2

Description

Hi there!

While working with the `ServletPipelineRequestDispatcherTest`, I've noticed that there are four mock variables repeatedly created across various tests. To simplify the code, I propose a small refactor to eliminate these redundancies, which could reduce the code by 60 lines.

Here's a summary of the repetitive mock creations:
- **`Binding`**: Repeated mocked 3 times
- **`Binding`**: Repeated mocked 3 times
- **`Injector`**: Repeated mocked 3 times
- **`HttpServletRequest`**: Repeated mocked 4 times

For instance, creating a mock for `HttpServletRequest` currently looks like this:

```java
final HttpServletRequest mockRequest = mock(HttpServletRequest.class);
when(mockRequest.getScheme()).thenReturn("https");
when(mockRequest.getServerName()).thenReturn("the.server");
when(mockRequest.getServerPort()).thenReturn(443);
```

To make this process more efficient, we can introduce a `createMockRequest` method:

```java
public final HttpServletRequest createMockRequest(String scheme, int serverPort) {
HttpServletRequest mockRequest = mock(HttpServletRequest.class);
when(mockRequest.getScheme()).thenReturn(scheme);
when(mockRequest.getServerName()).thenReturn("the.server");
when(mockRequest.getServerPort()).thenReturn(serverPort);
return mockRequest;
}
```

With this method, creating a mock `HttpServletRequest` becomes:

```java
final HttpServletRequest mockRequest = createMockRequest("https", 443);
```

Similarly, for the mock creation of `Binding`, `Binding` and `Injector` We can introduce methods for creating mocks for these classes as well:

**mock `Binding` creation**:
```java
public final Binding createMockBinding() {
final Binding binding = mock(Binding.class);
when(binding.acceptScopingVisitor(any())).thenReturn(true);
return binding;
}
```
**mock `Binding` creation**:
```java
public final Binding createMockBinding(ServletDefinition servletDefinition) {
Provider bindingProvider = Providers.of(servletDefinition);
Binding mockBinding = mock(Binding.class);
when(mockBinding.getProvider()).thenReturn(bindingProvider);
return mockBinding;
}
```
**mock `Injector` creation**:
```java
public final Injector createMockInjector(Binding binding, HttpServlet mockServlet, Binding mockBinding) {
final Key servletDefsKey = Key.get(TypeLiteral.get(ServletDefinition.class));
final Injector injector = mock(Injector.class);
when(injector.getBinding(Key.get(HttpServlet.class))).thenReturn(binding);
when(injector.getInstance(HTTP_SERLVET_KEY)).thenReturn(mockServlet);
when(injector.findBindingsByType(eq(servletDefsKey.getTypeLiteral())))
.thenReturn(ImmutableList.of(mockBinding));
return injector;
}
```

The code to create the mock before using these methods looks like this:

```java
final Injector injector = mock(Injector.class);//mock injector
final Binding binding = mock(Binding.class);//mock injector Binding
//Other java code in testcase
when(binding.acceptScopingVisitor((BindingScopingVisitor) any())).thenReturn(true);
when(injector.getBinding(Key.get(HttpServlet.class))).thenReturn(binding);
when(injector.getInstance(HTTP_SERLVET_KEY)).thenReturn(mockServlet);

final Key servetDefsKey = Key.get(TypeLiteral.get(ServletDefinition.class));

Binding mockBinding = mock(Binding.class);//mock Binding
when(injector.findBindingsByType(eq(servetDefsKey.getTypeLiteral())))
.thenReturn(ImmutableList.>of(mockBinding));
Provider bindingProvider = Providers.of(servletDefinition);
when(mockBinding.getProvider()).thenReturn(bindingProvider);
```

Using these methods, the refactored code becomes much cleaner:

```java
final Binding binding = createMockBinding();
// Other code in the test case
Binding mockBinding = createMockBinding(servletDefinition);
final Injector injector = createMockInjector(binding, mockServlet, mockBinding);
```

And for further improvement, we can overload `createMockInjector` for a more streamlined approach:

```java
public final Injector createMockInjector(ServletDefinition servletDefinition, HttpServlet mockServlet) {
return createMockInjector(createMockBinding(), mockServlet, createMockBinding(servletDefinition));
}
```

This final refactored code is:

```java
// Other code in the test case
final Injector injector = createMockInjector(servletDefinition, mockServlet);
```

I’ve created a draft PR in my forked project where you can see the detailed changes [here](https://github.com/gzhao9/guice/pull/1/files).

The refactor reduced the test cases by 60 lines of code, and I believe these changes will improve code readability.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.