Bigger picture: all of this timeout-fiddling infrastructure should be in a test library. It's not easy!
test/jdk/java/util/concurrent/tck effectively has its own test library, for historical reasons. On Fri, Sep 6, 2019 at 7:59 AM Martin Buchholz <marti...@google.com> wrote: > I took another look at LdapTimeoutTest.java. > > I was surprised to see RIGHT_MARGIN. In jsr166 we succeed in having one > timeout that is long enough to "never happen". I'm still advocating the 10 > second value. > > I was surprised to see LEFT_MARGIN. We just fixed Thread.sleep, so we > have no known problems with JDK methods returning early - you can trust > timed get! > You start measuring, by calling nanoTime, before you start the activity > you are measuring, so there should be no need for LEFT_MARGIN. > > We have some fresh thread-awaiting code here: > > http://gee.cs.oswego.edu/cgi-bin/viewcvs.cgi/jsr166/src/test/tck/JSR166TestCase.java?view=markup#l1443 > > Instead of communicating startTime from the test thread back to the main > thread, I would do my loMillis checking in the test thread, and hiMillis > checking in the main thread, like e.g. compare with a fresh test > method testTimedOffer > > http://gee.cs.oswego.edu/cgi-bin/viewcvs.cgi/jsr166/src/test/tck/ArrayBlockingQueueTest.java?view=markup#l394 > > Timeouts should be adjusted via Utils.adjustTimeout > > > On Fri, Sep 6, 2019 at 4:31 AM Pavel Rappo <pavel.ra...@oracle.com> wrote: > >> Martin, thanks for having a look at it. >> >> I'd appreciate if you could have a look at the timeout measuring >> mechanics in assertCompletion/assertIncompletion specifically, maybe to >> spot something that is grossly inadequate. >> >> I tried to accommodate some usual suspects of timeout measurements >> failures. I understand that since we're not working with real-time systems, >> my attempts to build bullet-proof measurement mechanics are futile. >> >> -Pavel >> >> > On 30 Aug 2019, at 18:19, Martin Buchholz <marti...@google.com> wrote: >> > >> > Not really a review, but: >> > >> > For many years we've been using 10 seconds (scaled by timeout factor) >> as a duration long enough that a timeout is a real failure. >> > Which is close to your own 20 seconds. Of course, no value is surely >> safe. >> > >> > Probably, parallel testing infrastructure for timeouts should be a test >> library method. I do something similar in JSR166TestCase >> > >> > /** >> > * Runs all the given actions in parallel, failing if any fail. >> > * Useful for running multiple variants of tests that are >> > * necessarily individually slow because they must block. >> > */ >> > void testInParallel(Action ... actions) { >> > ExecutorService pool = Executors.newCachedThreadPool(); >> > try (PoolCleaner cleaner = cleaner(pool)) { >> > >> > On Fri, Aug 30, 2019 at 6:23 AM Daniel Fuchs <daniel.fu...@oracle.com> >> wrote: >> > On 30/08/2019 13:54, Pavel Rappo wrote: >> > > Updated, >> > > >> > > http://cr.openjdk.java.net/~prappo/8151678/webrev.01/ >> > > >> > >> > Changes look good! >> > >> > best regards, >> > >> > -- daniel >> >>
