microsoft / kiota-java

Java libraries for Kiota-generated API clients.
https://aka.ms/kiota/docs
MIT License
22 stars 23 forks source link

Problems similar to issue #1374: Refactor to Eliminate Repetitive Mock Object Creation in OkHttpRequestAdapterTest.java #1386

Closed gzhao9 closed 2 weeks ago

gzhao9 commented 2 weeks ago

Hi there!

Similar to issue #1374. I did some additional similar refactoring after I got a positive response.

While working with theOkHttpRequestAdapterTest class. I've noticed that there are 3 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 18 lines.

  1. Mock Parsable Creation: Repeated 3 times.
  2. Mock ParseNode Creation: Repeated 2 times.
  3. Mock ParseNodeFactory Creation: Repeated 2 times.

For instance, the ParseNodeFactory mock creation logic is as follows:

final var mockFactory = mock(ParseNodeFactory.class);
when(mockFactory.getParseNode(any(String.class), any(InputStream.class)))
        .thenReturn(mockParseNode);
when(mockFactory.getValidContentType()).thenReturn("application/json");

To eliminate redundancy, We can create a method that can be called repeatedly, creatMockParseNodeFactory:

public ParseNodeFactory creatMockParseNodeFactory(
        ParseNode mockParseNode, String validContentType) {
    final var mockFactory = mock(ParseNodeFactory.class);
    when(mockFactory.getParseNode(any(String.class), any(InputStream.class)))
            .thenReturn(mockParseNode);
    when(mockFactory.getValidContentType()).thenReturn(validContentType);
    return mockFactory;
}

Using these method, the refactored code becomes much cleaner:

final var mockFactory = creatMockParseNodeFactory(mockParseNode, "application/json");

For the creation of the other two mocks, I used similar re-callable methods creatMockEntity and creatMockParseNode in order to avoid duplicate creation:

public Parsable creatMockEntity() {
    final var mockEntity = mock(Parsable.class);
    when(mockEntity.getFieldDeserializers()).thenReturn(new HashMap<>());
    return mockEntity;
}

public ParseNode creatMockParseNode(Parsable entity) {
    final var mockParseNode = mock(ParseNode.class);
    when(mockParseNode.getObjectValue(any(ParsableFactory.class))).thenReturn(entity);
    return mockParseNode;
}

With the above method, take sendReturnsObjectOnContent as an example, before refactoring. The configuration mock looks like this:

final var mockEntity = mock(Parsable.class);
when(mockEntity.getFieldDeserializers()).thenReturn(new HashMap<>());
final var mockParseNode = mock(ParseNode.class);
when(mockParseNode.getObjectValue(any(ParsableFactory.class))).thenReturn(mockEntity);
final var mockFactory = mock(ParseNodeFactory.class);
when(mockFactory.getParseNode(any(String.class), any(InputStream.class)))
        .thenReturn(mockParseNode);
when(mockFactory.getValidContentType()).thenReturn("application/json");

After refactoring, the mock is created like this:

final var mockEntity = creatMockEntity();
final var mockParseNode = creatMockParseNode(mockEntity);
final var mockFactory = creatMockParseNodeFactory(mockParseNode, "application/json");

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

I’ve created a draft PR in my forked project where you can see the detailed changes here.

gzhao9 commented 2 weeks ago

Since this issue essentially does the same thing as issue #1374. So I submitted PR #1387 directly. Please take a look at it if you are interested. @baywet

andrueastman commented 2 weeks ago

Closed via https://github.com/microsoft/kiota-java/pull/1387