gzhao9 opened a new issue, #13154:
URL: https://github.com/apache/dubbo/issues/13154
Hi Dubbo developers,
I have noticed that the dependency `ServerCallToObserverAdapter<String>` is
mocked with the same behavior across three different test suites, specifically
in the following five test cases:
- ManyToManyMethodHandlerTest.java
-
[testInvoke()](https://github.com/apache/dubbo/blob/3.2/dubbo-plugin/dubbo-reactive/src/test/java/org/apache/dubbo/reactive/ManyToManyMethodHandlerTest.java#L43-L52)
Lines 43-52
- ManyToOneMethodHandlerTest.java
-
[testInvoke()](https://github.com/apache/dubbo/blob/3.2/dubbo-plugin/dubbo-reactive/src/test/java/org/apache/dubbo/reactive/ManyToOneMethodHandlerTest.java#L43-L52)
Lines 43-52
-
[testError()](https://github.com/apache/dubbo/blob/3.2/dubbo-plugin/dubbo-reactive/src/test/java/org/apache/dubbo/reactive/ManyToOneMethodHandlerTest.java#L67-L77)
Lines 68-77
- OneToManyMethodHandlerTest.java
-
[testInvoke()](https://github.com/apache/dubbo/blob/3.2/dubbo-plugin/dubbo-reactive/src/test/java/org/apache/dubbo/reactive/OneToManyMethodHandlerTest.java#L43-L52)
Lines 43-52
-
[testError()](https://github.com/apache/dubbo/blob/3.2/dubbo-plugin/dubbo-reactive/src/test/java/org/apache/dubbo/reactive/OneToManyMethodHandlerTest.java#L63-L74)
Lines 65-74
The following is the duplicate code snippet I just mentioned:
```java
AtomicInteger nextCounter = new AtomicInteger();
AtomicInteger completeCounter = new AtomicInteger();
AtomicInteger errorCounter = new AtomicInteger();
ServerCallToObserverAdapter<String> responseObserver =
Mockito.mock(ServerCallToObserverAdapter.class);
doAnswer(o -> nextCounter.incrementAndGet())
.when(responseObserver).onNext(anyString());
doAnswer(o -> completeCounter.incrementAndGet())
.when(responseObserver).onCompleted();
doAnswer(o -> errorCounter.incrementAndGet())
.when(responseObserver).onError(any(Throwable.class));
```
We can eliminate this duplication by creating a
[creatObserverAdapter](https://github.com/gzhao9/dubbo/blob/refactor-org.apache.dubbo.reactive.test/dubbo-plugin/dubbo-reactive/src/test/java/org/apache/dubbo/reactive/creatObserverAdapter.java)
class.
When a mock needs to be created in a test case, we can create it with this
class.
The code snippet of `creatObserverAdapter`:
```java
public class creatObserverAdapter {
//Attributes is used for Assert statements.
private AtomicInteger nextCounter;
private AtomicInteger completeCounter;
private AtomicInteger errorCounter;
private ServerCallToObserverAdapter<String> responseObserver;
//Implement duplicate mock and stub behavior in the constructor.
public creatObserverAdapter() {
nextCounter = new AtomicInteger();
completeCounter = new AtomicInteger();
errorCounter = new AtomicInteger();
responseObserver = Mockito.mock(ServerCallToObserverAdapter.class);
doAnswer(o -> nextCounter.incrementAndGet())
.when(responseObserver).onNext(anyString());
doAnswer(o -> completeCounter.incrementAndGet())
.when(responseObserver).onCompleted();
doAnswer(o -> errorCounter.incrementAndGet())
.when(responseObserver).onError(any(Throwable.class));
}
public ServerCallToObserverAdapter<String> getResponseObserver() {
return this.responseObserver; }
// Other Getter methods of Attributes
// ...
}
```
In both `OneToManyMethodHandlerTest.java` and
`ManyToOneMethodHandlerTest.java`, the `ServerCallToObserverAdapter<String>` is
mocked in two test cases. we can make `ServerCallToObserverAdapter<String>` and
other dependencies used in assertion statements as attributes of each test
classes and create them in a `@BeforeEach` method. As a result, each test case
can reuse these attributes to avoid duplicate creation of mock objects.
```java
///Attributes is used for Assert statements.
AtomicInteger nextCounter = new AtomicInteger();
AtomicInteger completeCounter = new AtomicInteger();
AtomicInteger errorCounter = new AtomicInteger();
ServerCallToObserverAdapter<String> responseObserver =
Mockito.mock(ServerCallToObserverAdapter.class);
@BeforeEach
void init() throws ExecutionException, InterruptedException {
creatObserverAdapter creator = new creatObserverAdapter();
ServerCallToObserverAdapter<String> responseObserver =
creator.getResponseObserver();
nextCounter = creator.getNextCounter();
completeCounter = creator.getCompleteCounter();
errorCounter = creator.getErrorCounter();
ManyToOneMethodHandler<String, String> handler = new
ManyToOneMethodHandler<>(requestFlux ->
requestFlux.map(Integer::valueOf).reduce(Integer::sum).map(String::valueOf));
CompletableFuture<StreamObserver<String>> future = handler.invoke(new
Object[]{responseObserver});
requestObserver = future.get();
}
```
To be more intuitive, the following shows the comparison of how a test case
looks before and after the refactoring, with the `testInvoker()` in test suit
[ManyToOneMethodHandlerTest](https://github.com/apache/dubbo/blob/3.2/dubbo-plugin/dubbo-reactive/src/test/java/org/apache/dubbo/reactive/ManyToOneMethodHandlerTest.java)
as an example:
Code before refactoring:
```java
@Test
void testInvoker() throws ExecutionException, InterruptedException {
AtomicInteger nextCounter = new AtomicInteger();
AtomicInteger completeCounter = new AtomicInteger();
AtomicInteger errorCounter = new AtomicInteger();
ServerCallToObserverAdapter<String> responseObserver =
Mockito.mock(ServerCallToObserverAdapter.class);
doAnswer(o -> nextCounter.incrementAndGet())
.when(responseObserver).onNext(anyString());
doAnswer(o -> completeCounter.incrementAndGet())
.when(responseObserver).onCompleted();
doAnswer(o -> errorCounter.incrementAndGet())
.when(responseObserver).onError(any(Throwable.class));
ManyToOneMethodHandler<String, String> handler = new
ManyToOneMethodHandler<>(requestFlux ->
requestFlux.map(Integer::valueOf).reduce(Integer::sum).map(String::valueOf));
CompletableFuture<StreamObserver<String>> future =
handler.invoke(new Object[]{responseObserver});
StreamObserver<String> requestObserver = future.get();
for (int i = 0; i < 10; i++) {
requestObserver.onNext(String.valueOf(i));
}
requestObserver.onCompleted();
Assertions.assertEquals(1, nextCounter.get());
Assertions.assertEquals(0, errorCounter.get());
Assertions.assertEquals(1, completeCounter.get());
}
```
Code after refactoring:
```java
@Test
void testInvoker() {
for (int i = 0; i < 10; i++) {
requestObserver.onNext(String.valueOf(i));
}
requestObserver.onCompleted();
Assertions.assertEquals(1, nextCounter.get());
Assertions.assertEquals(0, errorCounter.get());
Assertions.assertEquals(1, completeCounter.get());
}
```
We've applied a similar refactoring approach to three test suites,
[ManyToManyMethodHandlerTest](https://github.com/gzhao9/dubbo/blob/refactor-org.apache.dubbo.reactive.test/dubbo-plugin/dubbo-reactive/src/test/java/org/apache/dubbo/reactive/ManyToManyMethodHandlerTest.java),
[ManyToOneMethodHandlerTest](https://github.com/gzhao9/dubbo/blob/refactor-org.apache.dubbo.reactive.test/dubbo-plugin/dubbo-reactive/src/test/java/org/apache/dubbo/reactive/ManyToOneMethodHandlerTest.java)
and
[OneToManyMethodHandlerTest](https://github.com/gzhao9/dubbo/blob/refactor-org.apache.dubbo.reactive.test/dubbo-plugin/dubbo-reactive/src/test/java/org/apache/dubbo/reactive/OneToManyMethodHandlerTest.java).
As a result, we can reduce 50 lines of code, which makes the test cases
smaller, easier to understand, and better to maintain.
#### Benefits of the Proposed Refactoring:
- **Modularity:** The code is now more modular with specific class for
creating mock object.
- **Readability:** Test cases are more readable, focusing on specific test
logic rather than the setup code.
- **Maintainability:** Easier maintenance due to the reduction of redundancy
and a cleaner, more structured codebase.
- **Scalability:** Facilitates the addition of new test cases and makes
modifications to existing ones more efficient.
I'd love to hear your thoughts on this proposal! Do you agree with any of
these changes, or do you have other insights or preferences?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
