Quantcast

RFR(9): 8177092: [TESTBUG] JMX test on MinimalVM fails after fix for 8176533

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RFR(9): 8177092: [TESTBUG] JMX test on MinimalVM fails after fix for 8176533

Robbin Ehn
Hi all, please review,

8156537 added a verification that <pid> was an attachable VM.
The verification caused a regression because of an underlying bug in an AttachProvider.
So the verification was removed for pid case in 8176533.
It will be put back when we have fixed: 8176828 (the faulty AttachProvider)

This test was written when we had the verification of VM before blasting SIGQUIT and waiting for socket to open.
Since we do not verify that the process is suppose to be attachable, jcmd will think that socket should open, which leads to timeout in jcmd for all non-attachable processes.

The test is suppose to verify that the minimal VM do not have attach API.
I change the test to just list attachable VM's and verify that this minimal VM is not among them.

Issue: https://bugs.openjdk.java.net/browse/JDK-8177092

Tested locally and with rbt.

Thanks

/Robbin

diff -r 03f4b62f3562 test/runtime/MinimalVM/JMX.java
--- a/test/runtime/MinimalVM/JMX.java Wed Mar 15 18:18:04 2017 -0700
+++ b/test/runtime/MinimalVM/JMX.java Mon Mar 20 15:22:44 2017 +0100
@@ -49,6 +49,5 @@

-        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"), Long.toString(ProcessTools.getProcessId()), "VM.print_threads"});
+        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"), "-l"});
          new OutputAnalyzer(pb.start())
-                .shouldContain("Could not find any processes matching ")
-                .shouldHaveExitValue(1);
+                .shouldNotContain(Long.toString(ProcessTools.getProcessId()));
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(9): 8177092: [TESTBUG] JMX test on MinimalVM fails after fix for 8176533

David Holmes
Hi Robbin,

The approach seems reasonable, but my concern is that you may get a
false match if there is some java process running that happens to have a
numeric argument that matches the target PID, or perhaps more likely
(though still remote) your target PID is a small number and then becomes
a substring of another PID.

Can you actually parse the list and check the PID value for an exact
match? Or perhaps simply use shouldNotMatch with a pattern that only
matches an exact numeric value at the start of a line?

Thanks,
David

On 21/03/2017 12:40 AM, Robbin Ehn wrote:

> Hi all, please review,
>
> 8156537 added a verification that <pid> was an attachable VM.
> The verification caused a regression because of an underlying bug in an
> AttachProvider.
> So the verification was removed for pid case in 8176533.
> It will be put back when we have fixed: 8176828 (the faulty AttachProvider)
>
> This test was written when we had the verification of VM before blasting
> SIGQUIT and waiting for socket to open.
> Since we do not verify that the process is suppose to be attachable,
> jcmd will think that socket should open, which leads to timeout in jcmd
> for all non-attachable processes.
>
> The test is suppose to verify that the minimal VM do not have attach API.
> I change the test to just list attachable VM's and verify that this
> minimal VM is not among them.
>
> Issue: https://bugs.openjdk.java.net/browse/JDK-8177092
>
> Tested locally and with rbt.
>
> Thanks
>
> /Robbin
>
> diff -r 03f4b62f3562 test/runtime/MinimalVM/JMX.java
> --- a/test/runtime/MinimalVM/JMX.java    Wed Mar 15 18:18:04 2017 -0700
> +++ b/test/runtime/MinimalVM/JMX.java    Mon Mar 20 15:22:44 2017 +0100
> @@ -49,6 +49,5 @@
>
> -        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"),
> Long.toString(ProcessTools.getProcessId()), "VM.print_threads"});
> +        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"),
> "-l"});
>          new OutputAnalyzer(pb.start())
> -                .shouldContain("Could not find any processes matching ")
> -                .shouldHaveExitValue(1);
> +
> .shouldNotContain(Long.toString(ProcessTools.getProcessId()));
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(9): 8177092: [TESTBUG] JMX test on MinimalVM fails after fix for 8176533

Robbin Ehn
Hi David, thanks for looking at this!

On 03/21/2017 02:32 AM, David Holmes wrote:
> Hi Robbin,
>
> The approach seems reasonable, but my concern is that you may get a false match if there is some java process running that happens to have a numeric argument that matches
> the target PID, or perhaps more likely (though still remote) your target PID is a small number and then becomes a substring of another PID.

Since I never even see even 3-digits pid, it kinda slip my mind.

>
> Can you actually parse the list and check the PID value for an exact match? Or perhaps simply use shouldNotMatch with a pattern that only matches an exact numeric value at
> the start of a line?

Yes of course!

/Robbin

diff -r 03f4b62f3562 test/runtime/MinimalVM/JMX.java
--- a/test/runtime/MinimalVM/JMX.java Wed Mar 15 18:18:04 2017 -0700
+++ b/test/runtime/MinimalVM/JMX.java Tue Mar 21 10:11:47 2017 +0100
@@ -49,6 +49,5 @@

-        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"), Long.toString(ProcessTools.getProcessId()), "VM.print_threads"});
+        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"), "-l"});
          new OutputAnalyzer(pb.start())
-                .shouldContain("Could not find any processes matching ")
-                .shouldHaveExitValue(1);
+                .shouldNotContain("^" +Long.toString(ProcessTools.getProcessId()) + "\\s+.*$");
      }



>
> Thanks,
> David
>
> On 21/03/2017 12:40 AM, Robbin Ehn wrote:
>> Hi all, please review,
>>
>> 8156537 added a verification that <pid> was an attachable VM.
>> The verification caused a regression because of an underlying bug in an
>> AttachProvider.
>> So the verification was removed for pid case in 8176533.
>> It will be put back when we have fixed: 8176828 (the faulty AttachProvider)
>>
>> This test was written when we had the verification of VM before blasting
>> SIGQUIT and waiting for socket to open.
>> Since we do not verify that the process is suppose to be attachable,
>> jcmd will think that socket should open, which leads to timeout in jcmd
>> for all non-attachable processes.
>>
>> The test is suppose to verify that the minimal VM do not have attach API.
>> I change the test to just list attachable VM's and verify that this
>> minimal VM is not among them.
>>
>> Issue: https://bugs.openjdk.java.net/browse/JDK-8177092
>>
>> Tested locally and with rbt.
>>
>> Thanks
>>
>> /Robbin
>>
>> diff -r 03f4b62f3562 test/runtime/MinimalVM/JMX.java
>> --- a/test/runtime/MinimalVM/JMX.java    Wed Mar 15 18:18:04 2017 -0700
>> +++ b/test/runtime/MinimalVM/JMX.java    Mon Mar 20 15:22:44 2017 +0100
>> @@ -49,6 +49,5 @@
>>
>> -        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"),
>> Long.toString(ProcessTools.getProcessId()), "VM.print_threads"});
>> +        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"),
>> "-l"});
>>          new OutputAnalyzer(pb.start())
>> -                .shouldContain("Could not find any processes matching ")
>> -                .shouldHaveExitValue(1);
>> +
>> .shouldNotContain(Long.toString(ProcessTools.getProcessId()));
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(9): 8177092: [TESTBUG] JMX test on MinimalVM fails after fix for 8176533

Robbin Ehn
 > +                .shouldNotContain("^" + Long.toString(ProcessTools.getProcessId()) + "\\s+.*$");

Missed space after + sign

/Robbin

On 03/21/2017 10:19 AM, Robbin Ehn wrote:

> Hi David, thanks for looking at this!
>
> On 03/21/2017 02:32 AM, David Holmes wrote:
>> Hi Robbin,
>>
>> The approach seems reasonable, but my concern is that you may get a false match if there is some java process running that happens to have a numeric argument that matches
>> the target PID, or perhaps more likely (though still remote) your target PID is a small number and then becomes a substring of another PID.
>
> Since I never even see even 3-digits pid, it kinda slip my mind.
>
>>
>> Can you actually parse the list and check the PID value for an exact match? Or perhaps simply use shouldNotMatch with a pattern that only matches an exact numeric value at
>> the start of a line?
>
> Yes of course!
>
> /Robbin
>
> diff -r 03f4b62f3562 test/runtime/MinimalVM/JMX.java
> --- a/test/runtime/MinimalVM/JMX.java    Wed Mar 15 18:18:04 2017 -0700
> +++ b/test/runtime/MinimalVM/JMX.java    Tue Mar 21 10:11:47 2017 +0100
> @@ -49,6 +49,5 @@
>
> -        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"), Long.toString(ProcessTools.getProcessId()), "VM.print_threads"});
> +        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"), "-l"});
>          new OutputAnalyzer(pb.start())
> -                .shouldContain("Could not find any processes matching ")
> -                .shouldHaveExitValue(1);
> +                .shouldNotContain("^" +Long.toString(ProcessTools.getProcessId()) + "\\s+.*$");
>      }
>
>
>
>>
>> Thanks,
>> David
>>
>> On 21/03/2017 12:40 AM, Robbin Ehn wrote:
>>> Hi all, please review,
>>>
>>> 8156537 added a verification that <pid> was an attachable VM.
>>> The verification caused a regression because of an underlying bug in an
>>> AttachProvider.
>>> So the verification was removed for pid case in 8176533.
>>> It will be put back when we have fixed: 8176828 (the faulty AttachProvider)
>>>
>>> This test was written when we had the verification of VM before blasting
>>> SIGQUIT and waiting for socket to open.
>>> Since we do not verify that the process is suppose to be attachable,
>>> jcmd will think that socket should open, which leads to timeout in jcmd
>>> for all non-attachable processes.
>>>
>>> The test is suppose to verify that the minimal VM do not have attach API.
>>> I change the test to just list attachable VM's and verify that this
>>> minimal VM is not among them.
>>>
>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8177092
>>>
>>> Tested locally and with rbt.
>>>
>>> Thanks
>>>
>>> /Robbin
>>>
>>> diff -r 03f4b62f3562 test/runtime/MinimalVM/JMX.java
>>> --- a/test/runtime/MinimalVM/JMX.java    Wed Mar 15 18:18:04 2017 -0700
>>> +++ b/test/runtime/MinimalVM/JMX.java    Mon Mar 20 15:22:44 2017 +0100
>>> @@ -49,6 +49,5 @@
>>>
>>> -        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"),
>>> Long.toString(ProcessTools.getProcessId()), "VM.print_threads"});
>>> +        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"),
>>> "-l"});
>>>          new OutputAnalyzer(pb.start())
>>> -                .shouldContain("Could not find any processes matching ")
>>> -                .shouldHaveExitValue(1);
>>> +
>>> .shouldNotContain(Long.toString(ProcessTools.getProcessId()));
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(9): 8177092: [TESTBUG] JMX test on MinimalVM fails after fix for 8176533

David Holmes
Hi Robbin,

On 21/03/2017 7:22 PM, Robbin Ehn wrote:
>> +                .shouldNotContain("^" +
> Long.toString(ProcessTools.getProcessId()) + "\\s+.*$");

Maybe I'm looking at the wrong version but OutputAnalyzer.*Contain only
take a string for a substring match, and you need to use  *Match(String
pattern) to pass in a regexp?

David

> Missed space after + sign
>
> /Robbin
>
> On 03/21/2017 10:19 AM, Robbin Ehn wrote:
>> Hi David, thanks for looking at this!
>>
>> On 03/21/2017 02:32 AM, David Holmes wrote:
>>> Hi Robbin,
>>>
>>> The approach seems reasonable, but my concern is that you may get a
>>> false match if there is some java process running that happens to
>>> have a numeric argument that matches
>>> the target PID, or perhaps more likely (though still remote) your
>>> target PID is a small number and then becomes a substring of another
>>> PID.
>>
>> Since I never even see even 3-digits pid, it kinda slip my mind.
>>
>>>
>>> Can you actually parse the list and check the PID value for an exact
>>> match? Or perhaps simply use shouldNotMatch with a pattern that only
>>> matches an exact numeric value at
>>> the start of a line?
>>
>> Yes of course!
>>
>> /Robbin
>>
>> diff -r 03f4b62f3562 test/runtime/MinimalVM/JMX.java
>> --- a/test/runtime/MinimalVM/JMX.java    Wed Mar 15 18:18:04 2017 -0700
>> +++ b/test/runtime/MinimalVM/JMX.java    Tue Mar 21 10:11:47 2017 +0100
>> @@ -49,6 +49,5 @@
>>
>> -        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"),
>> Long.toString(ProcessTools.getProcessId()), "VM.print_threads"});
>> +        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"),
>> "-l"});
>>          new OutputAnalyzer(pb.start())
>> -                .shouldContain("Could not find any processes matching ")
>> -                .shouldHaveExitValue(1);
>> +                .shouldNotContain("^"
>> +Long.toString(ProcessTools.getProcessId()) + "\\s+.*$");
>>      }
>>
>>
>>
>>>
>>> Thanks,
>>> David
>>>
>>> On 21/03/2017 12:40 AM, Robbin Ehn wrote:
>>>> Hi all, please review,
>>>>
>>>> 8156537 added a verification that <pid> was an attachable VM.
>>>> The verification caused a regression because of an underlying bug in an
>>>> AttachProvider.
>>>> So the verification was removed for pid case in 8176533.
>>>> It will be put back when we have fixed: 8176828 (the faulty
>>>> AttachProvider)
>>>>
>>>> This test was written when we had the verification of VM before
>>>> blasting
>>>> SIGQUIT and waiting for socket to open.
>>>> Since we do not verify that the process is suppose to be attachable,
>>>> jcmd will think that socket should open, which leads to timeout in jcmd
>>>> for all non-attachable processes.
>>>>
>>>> The test is suppose to verify that the minimal VM do not have attach
>>>> API.
>>>> I change the test to just list attachable VM's and verify that this
>>>> minimal VM is not among them.
>>>>
>>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8177092
>>>>
>>>> Tested locally and with rbt.
>>>>
>>>> Thanks
>>>>
>>>> /Robbin
>>>>
>>>> diff -r 03f4b62f3562 test/runtime/MinimalVM/JMX.java
>>>> --- a/test/runtime/MinimalVM/JMX.java    Wed Mar 15 18:18:04 2017 -0700
>>>> +++ b/test/runtime/MinimalVM/JMX.java    Mon Mar 20 15:22:44 2017 +0100
>>>> @@ -49,6 +49,5 @@
>>>>
>>>> -        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"),
>>>> Long.toString(ProcessTools.getProcessId()), "VM.print_threads"});
>>>> +        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"),
>>>> "-l"});
>>>>          new OutputAnalyzer(pb.start())
>>>> -                .shouldContain("Could not find any processes
>>>> matching ")
>>>> -                .shouldHaveExitValue(1);
>>>> +
>>>> .shouldNotContain(Long.toString(ProcessTools.getProcessId()));
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(9): 8177092: [TESTBUG] JMX test on MinimalVM fails after fix for 8176533

Robbin Ehn
Hi David,

On 03/21/2017 10:48 AM, David Holmes wrote:
> Hi Robbin,
>
> On 21/03/2017 7:22 PM, Robbin Ehn wrote:
>>> +                .shouldNotContain("^" +
>> Long.toString(ProcessTools.getProcessId()) + "\\s+.*$");
>
> Maybe I'm looking at the wrong version but OutputAnalyzer.*Contain only take a string for a substring match, and you need to use  *Match(String pattern) to pass in a regexp?

No you are looking at the correct version. I was looking at shouldNotMatch, but never changed it, thanks for catching this!

+                .shouldNotMatch("^" + Long.toString(ProcessTools.getProcessId()) + "\\s+.*$");

/Robbin


>
> David
>
>> Missed space after + sign
>>
>> /Robbin
>>
>> On 03/21/2017 10:19 AM, Robbin Ehn wrote:
>>> Hi David, thanks for looking at this!
>>>
>>> On 03/21/2017 02:32 AM, David Holmes wrote:
>>>> Hi Robbin,
>>>>
>>>> The approach seems reasonable, but my concern is that you may get a
>>>> false match if there is some java process running that happens to
>>>> have a numeric argument that matches
>>>> the target PID, or perhaps more likely (though still remote) your
>>>> target PID is a small number and then becomes a substring of another
>>>> PID.
>>>
>>> Since I never even see even 3-digits pid, it kinda slip my mind.
>>>
>>>>
>>>> Can you actually parse the list and check the PID value for an exact
>>>> match? Or perhaps simply use shouldNotMatch with a pattern that only
>>>> matches an exact numeric value at
>>>> the start of a line?
>>>
>>> Yes of course!
>>>
>>> /Robbin
>>>
>>> diff -r 03f4b62f3562 test/runtime/MinimalVM/JMX.java
>>> --- a/test/runtime/MinimalVM/JMX.java    Wed Mar 15 18:18:04 2017 -0700
>>> +++ b/test/runtime/MinimalVM/JMX.java    Tue Mar 21 10:11:47 2017 +0100
>>> @@ -49,6 +49,5 @@
>>>
>>> -        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"),
>>> Long.toString(ProcessTools.getProcessId()), "VM.print_threads"});
>>> +        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"),
>>> "-l"});
>>>          new OutputAnalyzer(pb.start())
>>> -                .shouldContain("Could not find any processes matching ")
>>> -                .shouldHaveExitValue(1);
>>> +                .shouldNotContain("^"
>>> +Long.toString(ProcessTools.getProcessId()) + "\\s+.*$");
>>>      }
>>>
>>>
>>>
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>> On 21/03/2017 12:40 AM, Robbin Ehn wrote:
>>>>> Hi all, please review,
>>>>>
>>>>> 8156537 added a verification that <pid> was an attachable VM.
>>>>> The verification caused a regression because of an underlying bug in an
>>>>> AttachProvider.
>>>>> So the verification was removed for pid case in 8176533.
>>>>> It will be put back when we have fixed: 8176828 (the faulty
>>>>> AttachProvider)
>>>>>
>>>>> This test was written when we had the verification of VM before
>>>>> blasting
>>>>> SIGQUIT and waiting for socket to open.
>>>>> Since we do not verify that the process is suppose to be attachable,
>>>>> jcmd will think that socket should open, which leads to timeout in jcmd
>>>>> for all non-attachable processes.
>>>>>
>>>>> The test is suppose to verify that the minimal VM do not have attach
>>>>> API.
>>>>> I change the test to just list attachable VM's and verify that this
>>>>> minimal VM is not among them.
>>>>>
>>>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8177092
>>>>>
>>>>> Tested locally and with rbt.
>>>>>
>>>>> Thanks
>>>>>
>>>>> /Robbin
>>>>>
>>>>> diff -r 03f4b62f3562 test/runtime/MinimalVM/JMX.java
>>>>> --- a/test/runtime/MinimalVM/JMX.java    Wed Mar 15 18:18:04 2017 -0700
>>>>> +++ b/test/runtime/MinimalVM/JMX.java    Mon Mar 20 15:22:44 2017 +0100
>>>>> @@ -49,6 +49,5 @@
>>>>>
>>>>> -        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"),
>>>>> Long.toString(ProcessTools.getProcessId()), "VM.print_threads"});
>>>>> +        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"),
>>>>> "-l"});
>>>>>          new OutputAnalyzer(pb.start())
>>>>> -                .shouldContain("Could not find any processes
>>>>> matching ")
>>>>> -                .shouldHaveExitValue(1);
>>>>> +
>>>>> .shouldNotContain(Long.toString(ProcessTools.getProcessId()));
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(9): 8177092: [TESTBUG] JMX test on MinimalVM fails after fix for 8176533

David Holmes
On 21/03/2017 7:53 PM, Robbin Ehn wrote:

> Hi David,
>
> On 03/21/2017 10:48 AM, David Holmes wrote:
>> Hi Robbin,
>>
>> On 21/03/2017 7:22 PM, Robbin Ehn wrote:
>>>> +                .shouldNotContain("^" +
>>> Long.toString(ProcessTools.getProcessId()) + "\\s+.*$");
>>
>> Maybe I'm looking at the wrong version but OutputAnalyzer.*Contain
>> only take a string for a substring match, and you need to use
>> *Match(String pattern) to pass in a regexp?
>
> No you are looking at the correct version. I was looking at
> shouldNotMatch, but never changed it, thanks for catching this!
>
> +                .shouldNotMatch("^" +
> Long.toString(ProcessTools.getProcessId()) + "\\s+.*$");

Okay looks good!

Thanks,
David
-----

> /Robbin
>
>
>>
>> David
>>
>>> Missed space after + sign
>>>
>>> /Robbin
>>>
>>> On 03/21/2017 10:19 AM, Robbin Ehn wrote:
>>>> Hi David, thanks for looking at this!
>>>>
>>>> On 03/21/2017 02:32 AM, David Holmes wrote:
>>>>> Hi Robbin,
>>>>>
>>>>> The approach seems reasonable, but my concern is that you may get a
>>>>> false match if there is some java process running that happens to
>>>>> have a numeric argument that matches
>>>>> the target PID, or perhaps more likely (though still remote) your
>>>>> target PID is a small number and then becomes a substring of another
>>>>> PID.
>>>>
>>>> Since I never even see even 3-digits pid, it kinda slip my mind.
>>>>
>>>>>
>>>>> Can you actually parse the list and check the PID value for an exact
>>>>> match? Or perhaps simply use shouldNotMatch with a pattern that only
>>>>> matches an exact numeric value at
>>>>> the start of a line?
>>>>
>>>> Yes of course!
>>>>
>>>> /Robbin
>>>>
>>>> diff -r 03f4b62f3562 test/runtime/MinimalVM/JMX.java
>>>> --- a/test/runtime/MinimalVM/JMX.java    Wed Mar 15 18:18:04 2017 -0700
>>>> +++ b/test/runtime/MinimalVM/JMX.java    Tue Mar 21 10:11:47 2017 +0100
>>>> @@ -49,6 +49,5 @@
>>>>
>>>> -        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"),
>>>> Long.toString(ProcessTools.getProcessId()), "VM.print_threads"});
>>>> +        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"),
>>>> "-l"});
>>>>          new OutputAnalyzer(pb.start())
>>>> -                .shouldContain("Could not find any processes
>>>> matching ")
>>>> -                .shouldHaveExitValue(1);
>>>> +                .shouldNotContain("^"
>>>> +Long.toString(ProcessTools.getProcessId()) + "\\s+.*$");
>>>>      }
>>>>
>>>>
>>>>
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>> On 21/03/2017 12:40 AM, Robbin Ehn wrote:
>>>>>> Hi all, please review,
>>>>>>
>>>>>> 8156537 added a verification that <pid> was an attachable VM.
>>>>>> The verification caused a regression because of an underlying bug
>>>>>> in an
>>>>>> AttachProvider.
>>>>>> So the verification was removed for pid case in 8176533.
>>>>>> It will be put back when we have fixed: 8176828 (the faulty
>>>>>> AttachProvider)
>>>>>>
>>>>>> This test was written when we had the verification of VM before
>>>>>> blasting
>>>>>> SIGQUIT and waiting for socket to open.
>>>>>> Since we do not verify that the process is suppose to be attachable,
>>>>>> jcmd will think that socket should open, which leads to timeout in
>>>>>> jcmd
>>>>>> for all non-attachable processes.
>>>>>>
>>>>>> The test is suppose to verify that the minimal VM do not have attach
>>>>>> API.
>>>>>> I change the test to just list attachable VM's and verify that this
>>>>>> minimal VM is not among them.
>>>>>>
>>>>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8177092
>>>>>>
>>>>>> Tested locally and with rbt.
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>> /Robbin
>>>>>>
>>>>>> diff -r 03f4b62f3562 test/runtime/MinimalVM/JMX.java
>>>>>> --- a/test/runtime/MinimalVM/JMX.java    Wed Mar 15 18:18:04 2017
>>>>>> -0700
>>>>>> +++ b/test/runtime/MinimalVM/JMX.java    Mon Mar 20 15:22:44 2017
>>>>>> +0100
>>>>>> @@ -49,6 +49,5 @@
>>>>>>
>>>>>> -        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"),
>>>>>> Long.toString(ProcessTools.getProcessId()), "VM.print_threads"});
>>>>>> +        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"),
>>>>>> "-l"});
>>>>>>          new OutputAnalyzer(pb.start())
>>>>>> -                .shouldContain("Could not find any processes
>>>>>> matching ")
>>>>>> -                .shouldHaveExitValue(1);
>>>>>> +
>>>>>> .shouldNotContain(Long.toString(ProcessTools.getProcessId()));
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(9): 8177092: [TESTBUG] JMX test on MinimalVM fails after fix for 8176533

Marcus Larsson
Hi,

Looks good to me!

Thanks,
Marcus


On 2017-03-21 11:00, David Holmes wrote:

> On 21/03/2017 7:53 PM, Robbin Ehn wrote:
>> Hi David,
>>
>> On 03/21/2017 10:48 AM, David Holmes wrote:
>>> Hi Robbin,
>>>
>>> On 21/03/2017 7:22 PM, Robbin Ehn wrote:
>>>>> + .shouldNotContain("^" +
>>>> Long.toString(ProcessTools.getProcessId()) + "\\s+.*$");
>>>
>>> Maybe I'm looking at the wrong version but OutputAnalyzer.*Contain
>>> only take a string for a substring match, and you need to use
>>> *Match(String pattern) to pass in a regexp?
>>
>> No you are looking at the correct version. I was looking at
>> shouldNotMatch, but never changed it, thanks for catching this!
>>
>> +                .shouldNotMatch("^" +
>> Long.toString(ProcessTools.getProcessId()) + "\\s+.*$");
>
> Okay looks good!
>
> Thanks,
> David
> -----
>
>> /Robbin
>>
>>
>>>
>>> David
>>>
>>>> Missed space after + sign
>>>>
>>>> /Robbin
>>>>
>>>> On 03/21/2017 10:19 AM, Robbin Ehn wrote:
>>>>> Hi David, thanks for looking at this!
>>>>>
>>>>> On 03/21/2017 02:32 AM, David Holmes wrote:
>>>>>> Hi Robbin,
>>>>>>
>>>>>> The approach seems reasonable, but my concern is that you may get a
>>>>>> false match if there is some java process running that happens to
>>>>>> have a numeric argument that matches
>>>>>> the target PID, or perhaps more likely (though still remote) your
>>>>>> target PID is a small number and then becomes a substring of another
>>>>>> PID.
>>>>>
>>>>> Since I never even see even 3-digits pid, it kinda slip my mind.
>>>>>
>>>>>>
>>>>>> Can you actually parse the list and check the PID value for an exact
>>>>>> match? Or perhaps simply use shouldNotMatch with a pattern that only
>>>>>> matches an exact numeric value at
>>>>>> the start of a line?
>>>>>
>>>>> Yes of course!
>>>>>
>>>>> /Robbin
>>>>>
>>>>> diff -r 03f4b62f3562 test/runtime/MinimalVM/JMX.java
>>>>> --- a/test/runtime/MinimalVM/JMX.java    Wed Mar 15 18:18:04 2017
>>>>> -0700
>>>>> +++ b/test/runtime/MinimalVM/JMX.java    Tue Mar 21 10:11:47 2017
>>>>> +0100
>>>>> @@ -49,6 +49,5 @@
>>>>>
>>>>> -        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"),
>>>>> Long.toString(ProcessTools.getProcessId()), "VM.print_threads"});
>>>>> +        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"),
>>>>> "-l"});
>>>>>          new OutputAnalyzer(pb.start())
>>>>> -                .shouldContain("Could not find any processes
>>>>> matching ")
>>>>> -                .shouldHaveExitValue(1);
>>>>> +                .shouldNotContain("^"
>>>>> +Long.toString(ProcessTools.getProcessId()) + "\\s+.*$");
>>>>>      }
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>>
>>>>>> On 21/03/2017 12:40 AM, Robbin Ehn wrote:
>>>>>>> Hi all, please review,
>>>>>>>
>>>>>>> 8156537 added a verification that <pid> was an attachable VM.
>>>>>>> The verification caused a regression because of an underlying bug
>>>>>>> in an
>>>>>>> AttachProvider.
>>>>>>> So the verification was removed for pid case in 8176533.
>>>>>>> It will be put back when we have fixed: 8176828 (the faulty
>>>>>>> AttachProvider)
>>>>>>>
>>>>>>> This test was written when we had the verification of VM before
>>>>>>> blasting
>>>>>>> SIGQUIT and waiting for socket to open.
>>>>>>> Since we do not verify that the process is suppose to be
>>>>>>> attachable,
>>>>>>> jcmd will think that socket should open, which leads to timeout in
>>>>>>> jcmd
>>>>>>> for all non-attachable processes.
>>>>>>>
>>>>>>> The test is suppose to verify that the minimal VM do not have
>>>>>>> attach
>>>>>>> API.
>>>>>>> I change the test to just list attachable VM's and verify that this
>>>>>>> minimal VM is not among them.
>>>>>>>
>>>>>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8177092
>>>>>>>
>>>>>>> Tested locally and with rbt.
>>>>>>>
>>>>>>> Thanks
>>>>>>>
>>>>>>> /Robbin
>>>>>>>
>>>>>>> diff -r 03f4b62f3562 test/runtime/MinimalVM/JMX.java
>>>>>>> --- a/test/runtime/MinimalVM/JMX.java    Wed Mar 15 18:18:04 2017
>>>>>>> -0700
>>>>>>> +++ b/test/runtime/MinimalVM/JMX.java    Mon Mar 20 15:22:44 2017
>>>>>>> +0100
>>>>>>> @@ -49,6 +49,5 @@
>>>>>>>
>>>>>>> -        pb.command(new String[] {
>>>>>>> JDKToolFinder.getJDKTool("jcmd"),
>>>>>>> Long.toString(ProcessTools.getProcessId()), "VM.print_threads"});
>>>>>>> +        pb.command(new String[] {
>>>>>>> JDKToolFinder.getJDKTool("jcmd"),
>>>>>>> "-l"});
>>>>>>>          new OutputAnalyzer(pb.start())
>>>>>>> -                .shouldContain("Could not find any processes
>>>>>>> matching ")
>>>>>>> -                .shouldHaveExitValue(1);
>>>>>>> +
>>>>>>> .shouldNotContain(Long.toString(ProcessTools.getProcessId()));

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(9): 8177092: [TESTBUG] JMX test on MinimalVM fails after fix for 8176533

Robbin Ehn
Thanks Marcus!

/Robbin

On 03/21/2017 04:36 PM, Marcus Larsson wrote:

> Hi,
>
> Looks good to me!
>
> Thanks,
> Marcus
>
>
> On 2017-03-21 11:00, David Holmes wrote:
>> On 21/03/2017 7:53 PM, Robbin Ehn wrote:
>>> Hi David,
>>>
>>> On 03/21/2017 10:48 AM, David Holmes wrote:
>>>> Hi Robbin,
>>>>
>>>> On 21/03/2017 7:22 PM, Robbin Ehn wrote:
>>>>>> + .shouldNotContain("^" +
>>>>> Long.toString(ProcessTools.getProcessId()) + "\\s+.*$");
>>>>
>>>> Maybe I'm looking at the wrong version but OutputAnalyzer.*Contain
>>>> only take a string for a substring match, and you need to use
>>>> *Match(String pattern) to pass in a regexp?
>>>
>>> No you are looking at the correct version. I was looking at
>>> shouldNotMatch, but never changed it, thanks for catching this!
>>>
>>> +                .shouldNotMatch("^" +
>>> Long.toString(ProcessTools.getProcessId()) + "\\s+.*$");
>>
>> Okay looks good!
>>
>> Thanks,
>> David
>> -----
>>
>>> /Robbin
>>>
>>>
>>>>
>>>> David
>>>>
>>>>> Missed space after + sign
>>>>>
>>>>> /Robbin
>>>>>
>>>>> On 03/21/2017 10:19 AM, Robbin Ehn wrote:
>>>>>> Hi David, thanks for looking at this!
>>>>>>
>>>>>> On 03/21/2017 02:32 AM, David Holmes wrote:
>>>>>>> Hi Robbin,
>>>>>>>
>>>>>>> The approach seems reasonable, but my concern is that you may get a
>>>>>>> false match if there is some java process running that happens to
>>>>>>> have a numeric argument that matches
>>>>>>> the target PID, or perhaps more likely (though still remote) your
>>>>>>> target PID is a small number and then becomes a substring of another
>>>>>>> PID.
>>>>>>
>>>>>> Since I never even see even 3-digits pid, it kinda slip my mind.
>>>>>>
>>>>>>>
>>>>>>> Can you actually parse the list and check the PID value for an exact
>>>>>>> match? Or perhaps simply use shouldNotMatch with a pattern that only
>>>>>>> matches an exact numeric value at
>>>>>>> the start of a line?
>>>>>>
>>>>>> Yes of course!
>>>>>>
>>>>>> /Robbin
>>>>>>
>>>>>> diff -r 03f4b62f3562 test/runtime/MinimalVM/JMX.java
>>>>>> --- a/test/runtime/MinimalVM/JMX.java    Wed Mar 15 18:18:04 2017 -0700
>>>>>> +++ b/test/runtime/MinimalVM/JMX.java    Tue Mar 21 10:11:47 2017 +0100
>>>>>> @@ -49,6 +49,5 @@
>>>>>>
>>>>>> -        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"),
>>>>>> Long.toString(ProcessTools.getProcessId()), "VM.print_threads"});
>>>>>> +        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"),
>>>>>> "-l"});
>>>>>>          new OutputAnalyzer(pb.start())
>>>>>> -                .shouldContain("Could not find any processes
>>>>>> matching ")
>>>>>> -                .shouldHaveExitValue(1);
>>>>>> +                .shouldNotContain("^"
>>>>>> +Long.toString(ProcessTools.getProcessId()) + "\\s+.*$");
>>>>>>      }
>>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>>
>>>>>>> On 21/03/2017 12:40 AM, Robbin Ehn wrote:
>>>>>>>> Hi all, please review,
>>>>>>>>
>>>>>>>> 8156537 added a verification that <pid> was an attachable VM.
>>>>>>>> The verification caused a regression because of an underlying bug
>>>>>>>> in an
>>>>>>>> AttachProvider.
>>>>>>>> So the verification was removed for pid case in 8176533.
>>>>>>>> It will be put back when we have fixed: 8176828 (the faulty
>>>>>>>> AttachProvider)
>>>>>>>>
>>>>>>>> This test was written when we had the verification of VM before
>>>>>>>> blasting
>>>>>>>> SIGQUIT and waiting for socket to open.
>>>>>>>> Since we do not verify that the process is suppose to be attachable,
>>>>>>>> jcmd will think that socket should open, which leads to timeout in
>>>>>>>> jcmd
>>>>>>>> for all non-attachable processes.
>>>>>>>>
>>>>>>>> The test is suppose to verify that the minimal VM do not have attach
>>>>>>>> API.
>>>>>>>> I change the test to just list attachable VM's and verify that this
>>>>>>>> minimal VM is not among them.
>>>>>>>>
>>>>>>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8177092
>>>>>>>>
>>>>>>>> Tested locally and with rbt.
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>>
>>>>>>>> /Robbin
>>>>>>>>
>>>>>>>> diff -r 03f4b62f3562 test/runtime/MinimalVM/JMX.java
>>>>>>>> --- a/test/runtime/MinimalVM/JMX.java    Wed Mar 15 18:18:04 2017
>>>>>>>> -0700
>>>>>>>> +++ b/test/runtime/MinimalVM/JMX.java    Mon Mar 20 15:22:44 2017
>>>>>>>> +0100
>>>>>>>> @@ -49,6 +49,5 @@
>>>>>>>>
>>>>>>>> -        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"),
>>>>>>>> Long.toString(ProcessTools.getProcessId()), "VM.print_threads"});
>>>>>>>> +        pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"),
>>>>>>>> "-l"});
>>>>>>>>          new OutputAnalyzer(pb.start())
>>>>>>>> -                .shouldContain("Could not find any processes
>>>>>>>> matching ")
>>>>>>>> -                .shouldHaveExitValue(1);
>>>>>>>> +
>>>>>>>> .shouldNotContain(Long.toString(ProcessTools.getProcessId()));
>
Loading...