Re: [ping] Re: RFR 8049194: com/sun/tools/attach/StartManagementAgent.java start failing after JDK-8048193
On 07/23/2014 02:15 PM, Daniel Fuchs wrote: Hi Jaroslav, It looks reasonable. I imagine you have run testset svc core to verify that there's no regression in other tests that use the library? Daniel, are you ok with this change? I ran JPRT for svc and core testsets and it didn't encounter any new failures when compared to the runs without this patch applied. Thanks, -JB- best regards, -- daniel On 7/23/14 1:55 PM, Jaroslav Bachorik wrote: On 07/22/2014 12:39 PM, Jaroslav Bachorik wrote: Hi, might I have a (R)eviewer to take a look at this? The test still fails pretty regularly. Thanks, -JB- On 07/07/2014 12:15 PM, Jaroslav Bachorik wrote: Please, review the following test change Issue : https://bugs.openjdk.java.net/browse/JDK-8049194 Webrev: http://cr.openjdk.java.net/~jbachorik/8049194/webrev.00 jdk.testlibrary.ProcessThread was erroneously starting the external application with timeout of -1 - meaning no waiting for the target application to initialize. What it should have done was to wait for the target application indefinitely and let the harness timeout the test. The change modifies jdk.testlibrary.ProcessTools.startProcess() methods to be explicit about the meaning of -1 and 0 (newly added) timeouts. It also adds a convenient method where you can start a process waiting for the warmup indefinitely without actually providing 0 and a dummy TimeUnit. I also took liberty and encapsulated the test.timeout.factor system property handling into jdk.testlibrary.Utils.adjustTimeou(to) method. The changes to the test library keep the current semantics (eg. the timeout of -1 still means no wait etc.) - they are only enhancing it. Thanks, -JB-
Re: [ping] Re: RFR 8049194: com/sun/tools/attach/StartManagementAgent.java start failing after JDK-8048193
On 7/25/14 2:20 PM, Jaroslav Bachorik wrote: On 07/23/2014 02:15 PM, Daniel Fuchs wrote: Hi Jaroslav, It looks reasonable. I imagine you have run testset svc core to verify that there's no regression in other tests that use the library? Daniel, are you ok with this change? I ran JPRT for svc and core testsets and it didn't encounter any new failures when compared to the runs without this patch applied. Oh yes of course, sorry if it wasn't clear. best regards, -- daniel Thanks, -JB- best regards, -- daniel On 7/23/14 1:55 PM, Jaroslav Bachorik wrote: On 07/22/2014 12:39 PM, Jaroslav Bachorik wrote: Hi, might I have a (R)eviewer to take a look at this? The test still fails pretty regularly. Thanks, -JB- On 07/07/2014 12:15 PM, Jaroslav Bachorik wrote: Please, review the following test change Issue : https://bugs.openjdk.java.net/browse/JDK-8049194 Webrev: http://cr.openjdk.java.net/~jbachorik/8049194/webrev.00 jdk.testlibrary.ProcessThread was erroneously starting the external application with timeout of -1 - meaning no waiting for the target application to initialize. What it should have done was to wait for the target application indefinitely and let the harness timeout the test. The change modifies jdk.testlibrary.ProcessTools.startProcess() methods to be explicit about the meaning of -1 and 0 (newly added) timeouts. It also adds a convenient method where you can start a process waiting for the warmup indefinitely without actually providing 0 and a dummy TimeUnit. I also took liberty and encapsulated the test.timeout.factor system property handling into jdk.testlibrary.Utils.adjustTimeou(to) method. The changes to the test library keep the current semantics (eg. the timeout of -1 still means no wait etc.) - they are only enhancing it. Thanks, -JB-
Re: [ping] Re: RFR 8049194: com/sun/tools/attach/StartManagementAgent.java start failing after JDK-8048193
Hi Jaroslav, It looks reasonable. I imagine you have run testset svc core to verify that there's no regression in other tests that use the library? best regards, -- daniel On 7/23/14 1:55 PM, Jaroslav Bachorik wrote: On 07/22/2014 12:39 PM, Jaroslav Bachorik wrote: Hi, might I have a (R)eviewer to take a look at this? The test still fails pretty regularly. Thanks, -JB- On 07/07/2014 12:15 PM, Jaroslav Bachorik wrote: Please, review the following test change Issue : https://bugs.openjdk.java.net/browse/JDK-8049194 Webrev: http://cr.openjdk.java.net/~jbachorik/8049194/webrev.00 jdk.testlibrary.ProcessThread was erroneously starting the external application with timeout of -1 - meaning no waiting for the target application to initialize. What it should have done was to wait for the target application indefinitely and let the harness timeout the test. The change modifies jdk.testlibrary.ProcessTools.startProcess() methods to be explicit about the meaning of -1 and 0 (newly added) timeouts. It also adds a convenient method where you can start a process waiting for the warmup indefinitely without actually providing 0 and a dummy TimeUnit. I also took liberty and encapsulated the test.timeout.factor system property handling into jdk.testlibrary.Utils.adjustTimeou(to) method. The changes to the test library keep the current semantics (eg. the timeout of -1 still means no wait etc.) - they are only enhancing it. Thanks, -JB-
Re: [ping] Re: RFR 8049194: com/sun/tools/attach/StartManagementAgent.java start failing after JDK-8048193
On 07/23/2014 02:15 PM, Daniel Fuchs wrote: Hi Jaroslav, It looks reasonable. I imagine you have run testset svc core to verify that there's no regression in other tests that use the library? Yes, I did. And I will do it again before the push to make sure any newly introduced external changes don't collide with this patch. -JB- best regards, -- daniel On 7/23/14 1:55 PM, Jaroslav Bachorik wrote: On 07/22/2014 12:39 PM, Jaroslav Bachorik wrote: Hi, might I have a (R)eviewer to take a look at this? The test still fails pretty regularly. Thanks, -JB- On 07/07/2014 12:15 PM, Jaroslav Bachorik wrote: Please, review the following test change Issue : https://bugs.openjdk.java.net/browse/JDK-8049194 Webrev: http://cr.openjdk.java.net/~jbachorik/8049194/webrev.00 jdk.testlibrary.ProcessThread was erroneously starting the external application with timeout of -1 - meaning no waiting for the target application to initialize. What it should have done was to wait for the target application indefinitely and let the harness timeout the test. The change modifies jdk.testlibrary.ProcessTools.startProcess() methods to be explicit about the meaning of -1 and 0 (newly added) timeouts. It also adds a convenient method where you can start a process waiting for the warmup indefinitely without actually providing 0 and a dummy TimeUnit. I also took liberty and encapsulated the test.timeout.factor system property handling into jdk.testlibrary.Utils.adjustTimeou(to) method. The changes to the test library keep the current semantics (eg. the timeout of -1 still means no wait etc.) - they are only enhancing it. Thanks, -JB-
Re: RFR 8049194: com/sun/tools/attach/StartManagementAgent.java start failing after JDK-8048193
Hi, might I have a (R)eviewer to take a look at this? The test still fails pretty regularly. Thanks, -JB- On 07/07/2014 12:15 PM, Jaroslav Bachorik wrote: Please, review the following test change Issue : https://bugs.openjdk.java.net/browse/JDK-8049194 Webrev: http://cr.openjdk.java.net/~jbachorik/8049194/webrev.00 jdk.testlibrary.ProcessThread was erroneously starting the external application with timeout of -1 - meaning no waiting for the target application to initialize. What it should have done was to wait for the target application indefinitely and let the harness timeout the test. The change modifies jdk.testlibrary.ProcessTools.startProcess() methods to be explicit about the meaning of -1 and 0 (newly added) timeouts. It also adds a convenient method where you can start a process waiting for the warmup indefinitely without actually providing 0 and a dummy TimeUnit. I also took liberty and encapsulated the test.timeout.factor system property handling into jdk.testlibrary.Utils.adjustTimeou(to) method. The changes to the test library keep the current semantics (eg. the timeout of -1 still means no wait etc.) - they are only enhancing it. Thanks, -JB-
RFR 8049194: com/sun/tools/attach/StartManagementAgent.java start failing after JDK-8048193
Please, review the following test change Issue : https://bugs.openjdk.java.net/browse/JDK-8049194 Webrev: http://cr.openjdk.java.net/~jbachorik/8049194/webrev.00 jdk.testlibrary.ProcessThread was erroneously starting the external application with timeout of -1 - meaning no waiting for the target application to initialize. What it should have done was to wait for the target application indefinitely and let the harness timeout the test. The change modifies jdk.testlibrary.ProcessTools.startProcess() methods to be explicit about the meaning of -1 and 0 (newly added) timeouts. It also adds a convenient method where you can start a process waiting for the warmup indefinitely without actually providing 0 and a dummy TimeUnit. I also took liberty and encapsulated the test.timeout.factor system property handling into jdk.testlibrary.Utils.adjustTimeou(to) method. The changes to the test library keep the current semantics (eg. the timeout of -1 still means no wait etc.) - they are only enhancing it. Thanks, -JB-
Re: RFR 8049194: com/sun/tools/attach/StartManagementAgent.java start failing after JDK-8048193
Looks good Erik Jaroslav Bachorik skrev 2014-07-07 12:15: Please, review the following test change Issue : https://bugs.openjdk.java.net/browse/JDK-8049194 Webrev: http://cr.openjdk.java.net/~jbachorik/8049194/webrev.00 jdk.testlibrary.ProcessThread was erroneously starting the external application with timeout of -1 - meaning no waiting for the target application to initialize. What it should have done was to wait for the target application indefinitely and let the harness timeout the test. The change modifies jdk.testlibrary.ProcessTools.startProcess() methods to be explicit about the meaning of -1 and 0 (newly added) timeouts. It also adds a convenient method where you can start a process waiting for the warmup indefinitely without actually providing 0 and a dummy TimeUnit. I also took liberty and encapsulated the test.timeout.factor system property handling into jdk.testlibrary.Utils.adjustTimeou(to) method. The changes to the test library keep the current semantics (eg. the timeout of -1 still means no wait etc.) - they are only enhancing it. Thanks, -JB-
Re: RFR 8049194: com/sun/tools/attach/StartManagementAgent.java start failing after JDK-8048193
Hi Jaroslav, Thats looks good to me. Olivier. On 07/07/2014 12:15, Jaroslav Bachorik wrote: Please, review the following test change Issue : https://bugs.openjdk.java.net/browse/JDK-8049194 Webrev: http://cr.openjdk.java.net/~jbachorik/8049194/webrev.00 jdk.testlibrary.ProcessThread was erroneously starting the external application with timeout of -1 - meaning no waiting for the target application to initialize. What it should have done was to wait for the target application indefinitely and let the harness timeout the test. The change modifies jdk.testlibrary.ProcessTools.startProcess() methods to be explicit about the meaning of -1 and 0 (newly added) timeouts. It also adds a convenient method where you can start a process waiting for the warmup indefinitely without actually providing 0 and a dummy TimeUnit. I also took liberty and encapsulated the test.timeout.factor system property handling into jdk.testlibrary.Utils.adjustTimeou(to) method. The changes to the test library keep the current semantics (eg. the timeout of -1 still means no wait etc.) - they are only enhancing it. Thanks, -JB-
