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" <[email protected]> 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: [email protected] With regards, Apache Git Services
