jinmeiliao commented on pull request #7395:
URL: https://github.com/apache/geode/pull/7395#issuecomment-1051322125
> Do not mock or spy the thing being tested.
>
> `CliFunction` is designed to be extended. So make the test create a
subclass, and make the subclass's `executeFunction` method throw. See below (or
my previous comment) for an example of how to do this.
>
> It would be a good idea to assert that the result contains the exception
that was thrown by the subclass.
>
> Here's a test that does all of those things. Notice that it tests
`CliFunction`'s error handling' without mocking or spying the thing being
tested, and without altering an existing implementation subclass in order to
test the `CliFunction`:
>
> ```
> @Test
> public void
executeShouldSendCliFunctionResultIfExecuteFunctionThrowsError() {
> Error errorThrownByExecuteFunction = new InternalGemFireError("test");
> CliFunction<Object[]> function = new CliFunction<Object[]>() {
> @Override
> public CliFunctionResult executeFunction(FunctionContext<Object[]>
context) {
> throw errorThrownByExecuteFunction;
> }
> };
>
> function.execute(context);
>
> ArgumentCaptor<CliFunctionResult> captor =
ArgumentCaptor.forClass(CliFunctionResult.class);
> verify(resultSender).lastResult(captor.capture());
> CliFunctionResult result = captor.getValue();
> assertThat(result)
> .isNotNull();
> assertThat(result.getResultObject())
> .isSameAs(errorThrownByExecuteFunction);
> }
> ```
>
> If the only reason to change `ImportDataFunction` was to use it to test
`CliFunction`, then those changes are not necessary. (Though it would be a
shame to throw those new tests away.)
I understand it as a theory. But in this particular case, isn't `spy` a
better way or a more readable way to stub out the ` executeFunction ` method
rather than having to implement an anonymous class to do exactly the same
thing? I thought the purpose of spy or mock is to reduce the need for test
classes.
--
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
