[GitHub] [geode] gesterzhou commented on a change in pull request #4970: GEODE-7676: Add PR clear with expiration tests

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org