[GitHub] kpm1985 commented on issue #963: Fix for FLUO-793 Modify ITs to use JUnits timeout rule

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

kpm1985 commented on issue #963: Fix for FLUO-793 Modify ITs to use JUnits timeout rule
URL: https://github.com/apache/fluo/pull/963#issuecomment-340535149
 
 
   Yes sir.
   
   On Oct 30, 2017 9:41 AM, "Keith Turner" <no...@github.com> wrote:
   
   *@keith-turner* commented on this pull request.
   ------------------------------
   
   In modules/integration/src/test/java/org/apache/fluo/integration/accumulo/
   TimeskippingIT.java
   <https://github.com/apache/fluo/pull/963#discussion_r147760549>:
   
   >  import org.slf4j.Logger;
    import org.slf4j.LoggerFactory;
   
    public class TimeskippingIT extends ITBase {
   +  @Rule
   +  public Timeout globalTimeout = Timeout.seconds(60);
   
   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
   
     @Rule
     public Timeout globalTimeout = Timeout.seconds(getTestTimeout());
   
   ------------------------------
   
   In modules/integration/src/test/java/org/apache/fluo/
   integration/impl/WorkerIT.java
   <https://github.com/apache/fluo/pull/963#discussion_r147761360>:
   
   > @@ -42,7 +44,9 @@
     * index of node degree.
     */
    public class WorkerIT extends ITBaseMini {
   -
   +  @Rule
   +  public Timeout globalTimeout = Timeout.seconds(120);
   
   If using the static method, this could be modified to the following
   
     @Rule
     public Timeout globalTimeout = Timeout.seconds( 2 * getTestTimeout());
   
   ------------------------------
   
   In modules/integration/src/test/java/org/apache/fluo/integration/impl/
   GarbageCollectionIteratorIT.java
   <https://github.com/apache/fluo/pull/963#discussion_r147761865>:
   
   > @@ -54,7 +58,7 @@ private void waitForGcTime(long expectedTime) throws Exception {
        }
      }
   
   -  @Test(timeout = 60000)
   
   Nice
   ------------------------------
   
   In modules/integration/src/test/java/org/apache/fluo/integration/impl/
   StochasticBankIT.java
   <https://github.com/apache/fluo/pull/963#discussion_r147762014>:
   
   > @@ -51,6 +53,8 @@
     * sum of all money in the bank should be the same, therefore the
   average should not vary.
     */
    public class StochasticBankIT extends ITBaseImpl {
   +  //@Rule
   +  //public Timeout globalTimeout = Timeout.seconds(120);
   
   Was this test causing problems with a timeout?
   
   ?
   You are receiving this because you authored the thread.
   Reply to this email directly, view it on GitHub
   <https://github.com/apache/fluo/pull/963#pullrequestreview-72901816>, or mute
   the thread
   <https://github.com/notifications/unsubscribe-auth/Acg-LOi3EVsXHCxBsrbnxwIflFQw-CHxks5sxfxSgaJpZM4QJ0xo>
   .
   

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