Added more unit tests to the JavaInstanceTest class #10369
Conversation
| String testString = "ABC123"; | ||
| CompletableFuture<JavaExecutionResult> result = instance.handleMessage(mock(Record.class), testString); | ||
| assertNull(result.get().getResult()); | ||
| assertNotNull(result.get().getUserException()); |
There was a problem hiding this comment.
What about using assertSame? And test about the absence or presence of wrapper?
There was a problem hiding this comment.
I have updated the code to use assertSame
| CompletableFuture<String> result = new CompletableFuture<>(); | ||
| executor.submit(() -> { | ||
| try { | ||
| Thread.sleep(500); |
There was a problem hiding this comment.
Why do you need this sleep?
There was a problem hiding this comment.
Just being consistent with the behavior of the other Async function tests
| String testString = "ABC123"; | ||
| CompletableFuture<JavaExecutionResult> result = instance.handleMessage(mock(Record.class), testString); | ||
| assertNull(result.get().getResult()); | ||
| // TODO Change this |
There was a problem hiding this comment.
Please do not leave TODOs
| log.info("input string: {}", input); | ||
| CompletableFuture<String> result = new CompletableFuture<>(); | ||
| executor.submit(() -> { | ||
| result.completeExceptionally(new InterruptedException("")); |
There was a problem hiding this comment.
Probably this is not the best way to simulate this.
You may set Thread.currentThread().interrupt() or better, call Thread.interrupt.
But I am not sure it is worth to make it so complicated WDYT?
There was a problem hiding this comment.
The call to completeExceptionally is required in order for the test to complete, any other approach, such as just throwing and exception results in the test hanging.
(cherry picked from commit c4098d6)
codelipenghui
added
release/2.7.3
cherry-picked/branch-2.7
|
Depends by #10618, cherry-pick this PR to branch-2.7 |
Motivation
Adding some more unit tests for the JavaInstance class, and doing a sanity check to see if a simple PR is able to build successfully in the OSS environment.
Modifications
Added more unit tests for the JavaInstanceTest class, that's it, nothing else.
Verifying this change
This change is the addition of new unit tests to an existing Test class
Does this pull request potentially affect one of the following parts:
No
Documentation