Staffan, thank you for pointing out performance and test.src issues!
The webrev with fixes can be found here:
http://cr.openjdk.java.net/~ykantser/7185591/webrev.01/
Please, see my comments in-lined.
Thanks,
Katja
On 01/13/2014 01:19 PM, Staffan Larsen wrote:
Katja,
test/lib/testlibrary/jdk/testlibrary/JcmdBase.java
68 * Run jcmd standalone
I think you should expand a bit on what “standalone” means here. It took me a
while to understand the difference.
Fixed.
test/lib/testlibrary/jdk/testlibrary/OutputAnalyzer.java
423 public int indexOf(String pattern) {
This seems very inefficient. Add all lines to an ArrayList and then walk
through them one at a time to if it matches and then walk through them once
again to find the index of that line.
469 public int shouldMatchByLine(String from, String to, String pattern) {
Same inefficiency here, but worse because both asLines() and indexOf() does the
same work.
test/lib/testlibrary/jdk/testlibrary/Utils.java
65 public static final String TEST_SRC =
System.getProperty("test.src").trim();
Fixed.
I wonder if this really works. Isn’t “test.src” different for different tests?
A property that jtreg changes
Yes, it is different.
before invoking each test? Or does this work because each test is run in a
different class loader and Utils.java will exist once in each class loader?
I've learned now it works because jtreg compiles all classes a test
needs to a separate location. For example when I run TestJcmdDefaults
there will be both TestJcmdDefaults.class and
jdk/testlibrary/Utils.class under
/tmp/jtreg/jtreg-workdir/classes/sun/tools/jcmd/. The class loader will
load Utils for TestJcmdDefaults from
/tmp/jtreg/jtreg-workdir/classes/sun/tools/jcmd/.
...
[Loaded jdk.testlibrary.Utils from
file:/tmp/jtreg/jtreg-workdir/classes/sun/tools/jcmd/]
...
But I think it's better to declare TEST_SRC per test precisely because
it's different for different tests.
/Staffan
On 10 jan 2014, at 13:50, Yekaterina Kantserova
<[email protected]> wrote:
Hi,
Could I please have a review of this fix.
In this fix I've rewritten sun/tools/jcmd/* tests in pure java to get rid of
all intermittent failures depending on for example MKS or race conditions (test
application has not yet started when the test start to run).
Webrev:
http://cr.openjdk.java.net/~ykantser/7185591/webrev.00/
Primal bug:
https://bugs.openjdk.java.net/browse/JDK-7185591
Similar bugs:
https://bugs.openjdk.java.net/browse/JDK-6977426
https://bugs.openjdk.java.net/browse/JDK-8020798
https://bugs.openjdk.java.net/browse/JDK-7130656 (this one is blocked by
https://bugs.openjdk.java.net/browse/JDK-8031482 so far)
Thanks,
Katja
