Fix two unit tests failing locally #4140
Conversation
Fixes JanusGraph#4139 Signed-off-by: Clement de Groc <[email protected]>
| Mockito.doReturn(multiQuery).when(tx).multiQuery(); | ||
| Mockito.doReturn(multiQuery).when(tx).multiQuery(anyCollection()); |
There was a problem hiding this comment.
💭 thought: When not specifying the multiQuery() argument type, the mocked method returns null (in my tests). Could it be because that method is overloaded and accepts both a Collection and varargs?
| Assertions.assertEquals(10, suppressedExceptions.length); | ||
| for(int i=0; i<10; i++){ | ||
| Assertions.assertTrue(suppressedExceptions[i] instanceof ExecutionException && | ||
| suppressedExceptions[i].getCause() instanceof IllegalArgumentException); | ||
| Assertions.assertTrue(suppressedExceptions[i] instanceof IllegalArgumentException); | ||
| } |
There was a problem hiding this comment.
📝 Based on the code of CompletableFutureUtil#unwrap and CompletableFutureUtil#unwrapExecutionException, it would make sense to me that the exceptions returned are not ExecutionExceptions but their "unwrapped" counterpart (IllegalStateException or IllegalArgumentException).
There was a problem hiding this comment.
You are right. Sorry I forgot to change that previously. Initially it was returning ExecutionException but after some fixes I changed this behavior and completely forgot about these Unit tests.
| Assertions.assertEquals(10, suppressedExceptions.length); | ||
| for(int i=0; i<10; i++){ | ||
| Assertions.assertTrue(suppressedExceptions[i] instanceof ExecutionException && | ||
| suppressedExceptions[i].getCause() instanceof IllegalArgumentException); | ||
| Assertions.assertTrue(suppressedExceptions[i] instanceof IllegalArgumentException); | ||
| } |
There was a problem hiding this comment.
You are right. Sorry I forgot to change that previously. Initially it was returning ExecutionException but after some fixes I changed this behavior and completely forgot about these Unit tests.
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation and see the Github Action logs for details |
Fixes #4139
Thank you for contributing to JanusGraph!
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
For all changes:
master)?For code changes:
For documentation related changes: