jinmeiliao commented on pull request #7395:
URL: https://github.com/apache/geode/pull/7395#issuecomment-1051069871


   > Thanks for moving that test to `CliFunction`. It fits much better there.
   > 
   > These two problems remain:
   > 
   > 1. Do not spy the thing being tested. `CliFunction` is designed to be 
extended. So make the test create a derived class, and make its 
`executeFunction` method throw.
   > 2. Write tests for `ImportDataFunction`.
   > 
   > Here are some test ideas.
   > 
   > One important change is that `ImportDataFunction.executeFunction(…)` 
returns a result, where the old `execute(…)` did not. Here are some things to 
test:
   > 
   > * If the region exists, is the result correct (status code, member name, 
message)?
   > * If the region doesn't exist, does the result correctly describe the 
problem (status code, member name, message)?
   > 
   > Here are some other things to test, to make sure they did not change:
   > 
   > * Does `executeFunction` invoke the callbacks from the snapshot service?
   > * Does `executeFunction` set parallel mode according to arg 3?
   > * Does `executeFunction` tell the snapshot service to load the snapshot 
with the right parameters?
   
   Thanks for the suggestion. More tests added. I didn't change the 
implementation of that function, only the error handling. so that part is 
tested.


-- 
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


Reply via email to