drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From paul-rogers <...@git.apache.org>
Subject [GitHub] drill pull request #1023: DRILL-5922 Fixed Child Allocator Leak. DRILL-5926 ...
Date Sun, 10 Dec 2017 04:55:23 GMT
Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/1023#discussion_r155938262
  
    --- Diff: common/src/test/java/org/apache/drill/test/DrillTest.java ---
    @@ -54,7 +54,7 @@
       static MemWatcher memWatcher;
       static String className;
     
    -  @Rule public final TestRule TIMEOUT = TestTools.getTimeoutRule(100000);
    +  @Rule public final TestRule TIMEOUT = TestTools.getTimeoutRule(180000);
    --- End diff --
    
    Two comments. One a nit: I find it hard to read 180000. Suggestion, use 180_000.
    
    More substantial comment: 180K ms = 180 seconds = 3 minutes. This seems FAR too long for
a default timeout.
    
    I wonder, should we make this much smaller? Then, find the tests that need extra time
and mark them specially? Maybe tag them with "SlowTests" and set a larger limit for just those
tests?
    
    This will cause tests to fail if they are too slow. That, in turn, will force us to either
fix those tests, or label them as slow.


---

Mime
View raw message