[GitHub] keith-turner commented on a change in pull request #963: Fix for FLUO-793 Modify ITs to use JUnits timeout rule

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

keith-turner commented on a change in pull request #963: Fix for FLUO-793 Modify ITs to use JUnits timeout rule
URL: https://github.com/apache/fluo/pull/963#discussion_r147760549
 
 

 ##########
 File path: modules/integration/src/test/java/org/apache/fluo/integration/accumulo/TimeskippingIT.java
 ##########
 @@ -24,11 +24,15 @@
 import org.apache.accumulo.core.security.Authorizations;
 import org.apache.fluo.integration.ITBase;
 import org.junit.Assert;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.Timeout;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 public class TimeskippingIT extends ITBase {
+  @Rule
+  public Timeout globalTimeout = Timeout.seconds(60);
 
 Review comment:
   I would recommend replacing 60 with a constant or static method call.  The constant or static method could be placed in the ITBase class, which most ITs extend.    I think a static method would be nice because it could easily be modified in the future to get the timeout from a java system property.  
   
   So could do something like the following where `getTestTimeout()` is declared in `ITBase`
   
   ```java
     @Rule
     public Timeout globalTimeout = Timeout.seconds(getTestTimeout());
   ```
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services