Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor to Eliminate Repetitive Mock Object Creation in ServletPipelineRequestDispatcherTest #1825

Open
gzhao9 opened this issue Jun 14, 2024 · 0 comments

Comments

@gzhao9
Copy link

gzhao9 commented Jun 14, 2024

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<HttpServlet>: Repeated mocked 3 times
  • Binding<ServletDefinition>: 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:

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:

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:

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

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

mock Binding<HttpServlet> creation

public final Binding<HttpServlet> createMockBinding() {
    final Binding<HttpServlet> binding = mock(Binding.class);
    when(binding.acceptScopingVisitor(any())).thenReturn(true);
    return binding;
}

mock Binding<ServletDefinition> creation

public final Binding<ServletDefinition> createMockBinding(ServletDefinition servletDefinition) {
    Provider<ServletDefinition> bindingProvider = Providers.of(servletDefinition);
    Binding<ServletDefinition> mockBinding = mock(Binding.class);
    when(mockBinding.getProvider()).thenReturn(bindingProvider);
    return mockBinding;
}

mock Injector creation

public final Injector createMockInjector(Binding<HttpServlet> binding, HttpServlet mockServlet, Binding<ServletDefinition> mockBinding) {
    final Key<ServletDefinition> 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:

final Injector injector = mock(Injector.class);//mock injector
final Binding<HttpServlet> binding = mock(Binding.class);//mock injector Binding<HttpServlet> 
//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<ServletDefinition> servetDefsKey = Key.get(TypeLiteral.get(ServletDefinition.class));

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

Using these methods, the refactored code becomes much cleaner:

final Binding<HttpServlet> binding = createMockBinding();
// Other code in the test case
Binding<ServletDefinition> mockBinding = createMockBinding(servletDefinition);
final Injector injector = createMockInjector(binding, mockServlet, mockBinding);

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

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

This final refactored code is:

// 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.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant