jinmeiliao edited a comment on pull request #7395: URL: https://github.com/apache/geode/pull/7395#issuecomment-1051122960
> That was I did originally, I thought you don't want that. So should I use a spy or create a new derived class or just as is? I did want a derived class. Here's way to do that without mocking or spying the thing being tested: ``` function = new CliFunction<Object[]>() { @Override public CliFunctionResult executeFunction(FunctionContext<Object[]> context) { throw new InternalGemFireError("test"); } }; ``` Either that or create an inner class in the test called something like `ThrowingCliFunction` that's implemented the same way. Something I just noticed: It would be a good idea to assert that the result contains the exception that was thrown. Like this: ``` ArgumentCaptor<CliFunctionResult> captor = ArgumentCaptor.forClass(CliFunctionResult.class); verify(resultSender).lastResult(captor.capture()); CliFunctionResult result = captor.getValue(); assertThat(result.getResultObject()) .isSame(InternalGemFireError.class); // Or maybe isEqualTo() the actual instance that was thrown, or isInstanceOf() ``` If the only reason to change `ImportDataFunction` was to use it to test `CliFunction`, that's no longer necessary. (Though it would be a shame to throw those new tests away.) -- 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: notifications-unsubscr...@geode.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org