RFR(M): JDK-8165500: TestJpsJar shouldn't jar all test.classpath directories
Dmitry Samersoff
dmitry.samersoff at oracle.com
Mon Oct 3 12:25:09 UTC 2016
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.
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.
2) I combined TestJpsClass and TestJpsJar to single test to save a bit
of execution time.
3) Also I fixed couple of minor issues in the code.
-Dmitry
On 2016-10-03 09:23, serguei.spitsyn at 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
>>
--
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.
More information about the serviceability-dev
mailing list