On 10/4/16 01:36, serguei.spit...@oracle.com wrote:
Dmitry,
On 10/3/16 05:25, Dmitry Samersoff wrote:
Serguei,
Thank you for comments. I'll fix it.
It is completely unclear what was the exact intention with this fix.
It's not the first time I fix this test. It starts failing again as soon
as something is changed in the environment, testlib or jtreg.
So provide a stable test is the motivation for this work.
Ok, thanks.
1) After refactoring the test uses LingeredApp as an attach target in
all cases. Not only for sanity check but for tests against class file
and jar file.
LingeredApp has well defined behavior and doesn't depend to other
parts of testlib so I expect that it fixes intermittent failures and
ClassNotFound exceptions.
It seems, you used the LingeredApp instead of the JpsBase (which is
deleted now).
Is it correct?
If so, then the JpsBase tested the jps options qlmvV.
Is this functionality thrown away?
2) I combined TestJpsClass and TestJpsJar to single test to save a bit
of execution time.
This is good, thanks.
3) Also I fixed couple of minor issues in the code.
What issues, could you, please, list them?
It makes sense to do as it needs to be checked specifically.
In fact, I've got lost a little bit in reviewing this fix.
Sorry, not lost but confused.
Thanks,
Serguei
It seems, it implements some test enhancement, not a fix for the
original issue.
Also, the DKFL failure matching is going to be lost with this
refactoring, as the
test has been renamed now and all bug report failure details won't
help anymore.
Were you able to reproduce the original issue with the unaltered test?
Can you confirm that it is gone now?
What about the "TestJpsJar shouldn't jar all test.classpath directories"?
Has this issue been resolved with this update?
Thanks,
Serguei
-Dmitry
On 2016-10-03 09:23,serguei.spit...@oracle.com wrote:
Hi Dmitry,
It is completely unclear what was the exact intention with this fix.
Could you, please, explain more?
What was the refactoring plan?
Why four files are removed?
Are two new files a replacement of the deleted four files?
What functionality was deleted and why and what was moved?
http://cr.openjdk.java.net/~dsamersoff/JDK-8165500/webrev.02/_webrev/test/lib/jdk/test/lib/apps/LingeredApp.java.frames.html
Minor comments:
259 * Analyze an environment and prepare a command line to 260 * run
theApp, app name should be added explicitlyPlease, change: theApp => the
app (to be consistent with the line L326. Also, it makes sense to have
to separate sentences with dots at the end.
329 * @throws IOException
It'd be nice to add a comment at the place where exactly this exception
can be thrown. 357 // startApp of derived app can throw
358 // an exception before, LA actually starts The comma is not needed.
This would look better: 357 // The startApp() of the derived app can throw
358 // an exception before the LA actually starts
Thanks, Serguei On 9/23/16 02:01, Dmitry Samersoff wrote:
Everybody,
Please, review:
http://cr.openjdk.java.net/~dsamersoff/JDK-8165500/webrev.02/
I refactored the test to improve stability and simplify further
debugging.
Testing: rbt
PS: Diffs is messy, so pleas look at raw files.
-Dmitry
