On 13.5.2015 21:14, Martin Buchholz wrote:
David has given you an approval; feel free to ignore me!
I tried running the test against jdk9, but it timed out!
... and it did print nice messages, didn't it? ;) I managed to leave in
a piece of code I used for testing the error messages. Sorry about that.
It's common for users to introduce parallelism in classloaders or
jar-loaders (we do this at google) which may cause arbitrary thread
fluctuations. That makes this particular API rather difficult to test
robustly, especially if you are only testing against the spec.
Well, in such an environment this test makes absolutely no sense - there
is nothing deterministic in the thread counters we could assert against.
-JB-
On Wed, May 13, 2015 at 11:07 AM, Jaroslav Bachorik
<jaroslav.bacho...@oracle.com <mailto:jaroslav.bacho...@oracle.com>> wrote:
On 13.5.2015 19:40, Martin Buchholz wrote:
toString()should never return null, I think.
It doesn't matter much here. The test would fail with an NPE and it
would be right to do so. None of the suppliers should ever return null.
52 @Override
53 public String toString() {
54 T resolved = val.get();
55 return resolved != null ? resolved.toString()
: null;
56 }
I expected methods like waitForCondition to include a timeout with
failure. I like 10 seconds, being large enough to never be hit
spuriously in tests.
It's difficult to find a value 'large enough'. Imagine the test
running on a small embedded device and fastdebug build. I had my fun
fixing tests failing intermittently because it was thought that the
original timeout was large enough. I better leave it to the harness.
Why not
() -> (long) mbean.getThreadCount(),
Because curLiveThreadCount needs to be set to mbean.getThreadCount()
value.
169 ()->{
170 curLiveThreadCount = mbean.getThreadCount();
171 return (long)curLiveThreadCount;
172 },
I worry that
mbean.getThreadCount()
is hard to test since the "system" may spin up and shut down utility
threads at any time.
The 'system' threads are not reported by this method. And the
current understanding is that once VM is fully initialized no
user-observable threads are randomly started on behalf of the system.
-JB-
On Wed, May 13, 2015 at 4:46 AM, Jaroslav Bachorik
<jaroslav.bacho...@oracle.com
<mailto:jaroslav.bacho...@oracle.com>
<mailto:jaroslav.bacho...@oracle.com
<mailto:jaroslav.bacho...@oracle.com>>> wrote:
On 1.5.2015 21:55, Martin Buchholz wrote:
On Thu, Apr 30, 2015 at 11:27 AM, Jaroslav Bachorik
<jaroslav.bacho...@oracle.com
<mailto:jaroslav.bacho...@oracle.com>
<mailto:jaroslav.bacho...@oracle.com
<mailto:jaroslav.bacho...@oracle.com>>
<mailto:jaroslav.bacho...@oracle.com
<mailto:jaroslav.bacho...@oracle.com>
<mailto:jaroslav.bacho...@oracle.com
<mailto:jaroslav.bacho...@oracle.com>>>> wrote:
On 30.4.2015 19:18, Martin Buchholz wrote:
Tests that sleep can almost always be better
written
some other way.
In this case, I would prefer busy-waiting with
timeout
until the
expected condition becomes true.
The thing is that in case of a real issue with the
thread
counters we
a/ would be busy-waiting till the test times out
(using an
arbitrary
delay is also problematic due to different
performance of
different
machines running with different configurations)
Far less problematic (performance-wise and
reliability-wise)
than the
fixed sleep.
b/ would get a rather confusing message about the test
timing out at
the end
You can easily improve the error message.
Well, not that easily. It is not possible to get a
notification when
JTREG decides to timeout the test. So you will get the standard
JTREG message and that's all.
I was able to modify the test to wait for a given condition and
provide useful messages in case of mismatch and retry. For
the price
of an increased complexity. On the other hand, the test
should be
much more resilient to timing errors caused by slow setups.
Updated webrev:
http://cr.openjdk.java.net/~jbachorik/8078143/webrev.01
Thanks,
-JB-
