gesterzhou commented on a change in pull request #4970:
URL: https://github.com/apache/geode/pull/4970#discussion_r412539205



##########
File path: 
geode-core/src/distributedTest/java/org/apache/geode/internal/cache/PartitionedRegionClearWithExpirationDUnitTest.java
##########
@@ -0,0 +1,537 @@
+/*

Review comment:
       The test needs 30 minutes to finish all combination. And many tests took 
more than 30 seconds each. 
   
   Many combinations are unnecessary, for example we want to see the expiration 
tasks are cancelled, we don't care what expiration. Only when we want to see an 
expiration to be triggered, then we need some (not all) expiration types. In my 
opinion, we can hard code to use DESTROY expiration type only in this test.
   
   We don't have to verify clear from accessor or server. Some other tests have 
verified that. You can just use server to do clear. 
   
   Region types can also be reduced to a few. We have other tests  to verify 
that all the combination of region type can do clear successfully. We only need 
to verify the expiration tasks are cleared in this test.
   




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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to