Hi Petr,
thank you for the review, here are my thoughts:
1. I added new method that way after analyzing the other methods and
their usage in jtreg tests. If you look at getExecutionCommand() you
could see the similar solution. Indeed my changes don't break the
previous usages of ProcessCommunicator. It's also possible to perform
bunch run, but of course without the termination ability. Another issue
is that we must kill a child VM explicitly if it doesn't exit
automatically, but we must do this only after test sequence is passed.
jtreg doesn't kill child VMs on timeout, this is the known problem
(that's why this JIRA issue exists).
2. I decided to leave it as is, as main classes in these tests are used
in two cases: applet itself and the main class for child VM.
3. I found this mistake and added it to the second version of fix.
Regards,
Oleg
On 14.05.2014 14:01, Petr Pchelko wrote:
Hello, Oleg.
1. I'm not sure your about your new API in ProcessCommunicator.
First of all, now the communicator can't be used twice in a single test to open
and then close a bunch of processes.
Also, you need to modify the tests themselves.
I think it would be better to do the termination implicitly: you can get jtreg
property with execution timeout, start a timer
thread when you create a process and kill it after the timeout. In case
doWaitFor finishes you kill the timer thread.
2. We normally remove .html if it's possible to convert test to a standalone
app.
3. NoFormatsCrashTest lacks .java file.
With best regards. Petr.
On 13 мая 2014 г., at 14:55, Sergey Bylokhov <sergey.bylok...@oracle.com> wrote:
Hi. Oleg.
A few notes.
- Some tests contains empty/non-rethrow catch blocks. I guess we should rethrow
an exception and the test should fail in this case.
- Some tests use System.exit() which should be replaced by the exception.
On 5/9/14 7:32 PM, Oleg Pekhovskiy wrote:
Hi all,
please review the fix
http://cr.openjdk.java.net/~bagiras/9/8014755.1/
for
https://bugs.openjdk.java.net/browse/JDK-8014755
These tests were moved from the closed workspace.
The main idea of fix is to force termination of child JVM if it's not exited
automatically after the test sequence.
Tests accuracy was improved to cover all error cases.
destroyProcess() method was added to
test.java.awt.regtesthelpers.process.ProcessCommunicator
for termination of child JVM when needed.
Thanks,
Oleg
--
Best regards, Sergey.
