RFR: 8205054: Could not find "lsof" on test machine

classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR: 8205054: Could not find "lsof" on test machine

Leo Korinth
Hi,

Unfortunately some test machines does not have "lsof" installed, or it
can not be found.

I do not know how to find "lsof" on the test machine where the test
fails; the command might not be installed. Here is a fix to make the
test case succeed if "lsof" can not be found. I believe it is the best
option (at least for now). I am sorry for all the noise I am generating.

Bug:
https://bugs.openjdk.java.net/browse/JDK-8205054

Webrev:
http://cr.openjdk.java.net/~lkorinth/8205054/00/

Testing:
I have tested this on my machine, and I have a mach5 test job running.

Thanks,
Leo
Reply | Threaded
Open this post in threaded view
|

RE: 8205054: Could not find "lsof" on test machine

Lindenmaier, Goetz
Hi Leo,

the fix looks good.  It makes sense to skip a test if the infrastructure is
insufficient to run it.

Could you please put the long comment at the end of line 179
into a line of it's own?

Also, please take the chance and move the Copyright message
to the beginning of the file.

Best regards,
  Goetz.


> -----Original Message-----
> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
> [hidden email]] On Behalf Of Leo Korinth
> Sent: Donnerstag, 14. Juni 2018 18:55
> To: Hotspot dev runtime <[hidden email]>
> Subject: RFR: 8205054: Could not find "lsof" on test machine
>
> Hi,
>
> Unfortunately some test machines does not have "lsof" installed, or it
> can not be found.
>
> I do not know how to find "lsof" on the test machine where the test
> fails; the command might not be installed. Here is a fix to make the
> test case succeed if "lsof" can not be found. I believe it is the best
> option (at least for now). I am sorry for all the noise I am generating.
>
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8205054
>
> Webrev:
> http://cr.openjdk.java.net/~lkorinth/8205054/00/
>
> Testing:
> I have tested this on my machine, and I have a mach5 test job running.
>
> Thanks,
> Leo
Reply | Threaded
Open this post in threaded view
|

Re: 8205054: Could not find "lsof" on test machine

Leo Korinth


On 14/06/18 19:10, Lindenmaier, Goetz wrote:
> Hi Leo,
>
> the fix looks good.  It makes sense to skip a test if the infrastructure is
> insufficient to run it.
>
> Could you please put the long comment at the end of line 179
> into a line of it's own?

Fixed

> Also, please take the chance and move the Copyright message
> to the beginning of the file.

Fixed

> Best regards,
>    Goetz.
>

Thanks for the fast review; I need a second reviewer and someone to help
me push (I am not a commiter).

New webrevs:
http://cr.openjdk.java.net/~lkorinth/8205054/00_01/ (incremental)
http://cr.openjdk.java.net/~lkorinth/8205054/01/    (full)

Thanks,
Leo

>> -----Original Message-----
>> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
>> [hidden email]] On Behalf Of Leo Korinth
>> Sent: Donnerstag, 14. Juni 2018 18:55
>> To: Hotspot dev runtime <[hidden email]>
>> Subject: RFR: 8205054: Could not find "lsof" on test machine
>>
>> Hi,
>>
>> Unfortunately some test machines does not have "lsof" installed, or it
>> can not be found.
>>
>> I do not know how to find "lsof" on the test machine where the test
>> fails; the command might not be installed. Here is a fix to make the
>> test case succeed if "lsof" can not be found. I believe it is the best
>> option (at least for now). I am sorry for all the noise I am generating.
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8205054
>>
>> Webrev:
>> http://cr.openjdk.java.net/~lkorinth/8205054/00/
>>
>> Testing:
>> I have tested this on my machine, and I have a mach5 test job running.
>>
>> Thanks,
>> Leo
Reply | Threaded
Open this post in threaded view
|

Re: 8205054: Could not find "lsof" on test machine

David Holmes
Hi Leo,

This is what I was concerned about - and yes the CI testing did shake it
out in a few cycles.

I agree with the fix and will sponsor it for you.

Thanks,
David

On 15/06/2018 3:31 AM, Leo Korinth wrote:

>
>
> On 14/06/18 19:10, Lindenmaier, Goetz wrote:
>> Hi Leo,
>>
>> the fix looks good.  It makes sense to skip a test if the
>> infrastructure is
>> insufficient to run it.
>>
>> Could you please put the long comment at the end of line 179
>> into a line of it's own?
>
> Fixed
>
>> Also, please take the chance and move the Copyright message
>> to the beginning of the file.
>
> Fixed
>
>> Best regards,
>>    Goetz.
>>
>
> Thanks for the fast review; I need a second reviewer and someone to help
> me push (I am not a commiter).
>
> New webrevs:
> http://cr.openjdk.java.net/~lkorinth/8205054/00_01/ (incremental)
> http://cr.openjdk.java.net/~lkorinth/8205054/01/    (full)
>
> Thanks,
> Leo
>
>>> -----Original Message-----
>>> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
>>> [hidden email]] On Behalf Of Leo Korinth
>>> Sent: Donnerstag, 14. Juni 2018 18:55
>>> To: Hotspot dev runtime <[hidden email]>
>>> Subject: RFR: 8205054: Could not find "lsof" on test machine
>>>
>>> Hi,
>>>
>>> Unfortunately some test machines does not have "lsof" installed, or it
>>> can not be found.
>>>
>>> I do not know how to find "lsof" on the test machine where the test
>>> fails; the command might not be installed. Here is a fix to make the
>>> test case succeed if "lsof" can not be found. I believe it is the best
>>> option (at least for now). I am sorry for all the noise I am generating.
>>>
>>> Bug:
>>> https://bugs.openjdk.java.net/browse/JDK-8205054
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~lkorinth/8205054/00/
>>>
>>> Testing:
>>> I have tested this on my machine, and I have a mach5 test job running.
>>>
>>> Thanks,
>>> Leo
Reply | Threaded
Open this post in threaded view
|

Re: 8205054: Could not find "lsof" on test machine

David Holmes
Sorry after doing more testing and looking at the output I think we need
to do better. For a machine with no lsof we see:

using command: <not found>
(Second VM) Open file descriptors:

using command: <not found>
(Third VM) Open file descriptors:

VM RESULT => RETAINS FD
VM RESULT => VM EXIT

Log file was not inherited by third VM
---

That looks like an error is occurring to me and that we pass by
accident. I think we need to terminate the test, cleanly of course, as
soon as we can't find the tool and report that the test can't run
because of that.

Thanks,
David

On 15/06/2018 11:21 AM, David Holmes wrote:

> Hi Leo,
>
> This is what I was concerned about - and yes the CI testing did shake it
> out in a few cycles.
>
> I agree with the fix and will sponsor it for you.
>
> Thanks,
> David
>
> On 15/06/2018 3:31 AM, Leo Korinth wrote:
>>
>>
>> On 14/06/18 19:10, Lindenmaier, Goetz wrote:
>>> Hi Leo,
>>>
>>> the fix looks good.  It makes sense to skip a test if the
>>> infrastructure is
>>> insufficient to run it.
>>>
>>> Could you please put the long comment at the end of line 179
>>> into a line of it's own?
>>
>> Fixed
>>
>>> Also, please take the chance and move the Copyright message
>>> to the beginning of the file.
>>
>> Fixed
>>
>>> Best regards,
>>>    Goetz.
>>>
>>
>> Thanks for the fast review; I need a second reviewer and someone to
>> help me push (I am not a commiter).
>>
>> New webrevs:
>> http://cr.openjdk.java.net/~lkorinth/8205054/00_01/ (incremental)
>> http://cr.openjdk.java.net/~lkorinth/8205054/01/    (full)
>>
>> Thanks,
>> Leo
>>
>>>> -----Original Message-----
>>>> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
>>>> [hidden email]] On Behalf Of Leo Korinth
>>>> Sent: Donnerstag, 14. Juni 2018 18:55
>>>> To: Hotspot dev runtime <[hidden email]>
>>>> Subject: RFR: 8205054: Could not find "lsof" on test machine
>>>>
>>>> Hi,
>>>>
>>>> Unfortunately some test machines does not have "lsof" installed, or it
>>>> can not be found.
>>>>
>>>> I do not know how to find "lsof" on the test machine where the test
>>>> fails; the command might not be installed. Here is a fix to make the
>>>> test case succeed if "lsof" can not be found. I believe it is the best
>>>> option (at least for now). I am sorry for all the noise I am
>>>> generating.
>>>>
>>>> Bug:
>>>> https://bugs.openjdk.java.net/browse/JDK-8205054
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~lkorinth/8205054/00/
>>>>
>>>> Testing:
>>>> I have tested this on my machine, and I have a mach5 test job running.
>>>>
>>>> Thanks,
>>>> Leo
Reply | Threaded
Open this post in threaded view
|

Re: 8205054: Could not find "lsof" on test machine

Leo Korinth
Hi David.

On 15/06/18 04:08, David Holmes wrote:

> Sorry after doing more testing and looking at the output I think we need
> to do better. For a machine with no lsof we see:
>
> using command: <not found>
> (Second VM) Open file descriptors:
>
> using command: <not found>
> (Third VM) Open file descriptors:
>
> VM RESULT => RETAINS FD
> VM RESULT => VM EXIT
>
> Log file was not inherited by third VM
> ---
>
> That looks like an error is occurring to me and that we pass by
> accident. I think we need to terminate the test, cleanly of course, as
> soon as we can't find the tool and report that the test can't run
> because of that.
>
> Thanks,
> David

I could change the output to "using command: <not found, terminating
test as successful as we can not finish it in a meaningful way>" or
maybe something else that you prefer that is better.

I do believe this is the easy way to terminate the test cleanly without
leaving running JVMs. We need to communicate back to the first JVM that
the test was successful. Is it the string "<not found>" that looks like
an error and that we pass by accident? Or is it something in the test
case code that looks fishy?

Or maybe the comment should be improved?
   // if command can not be found return list without log file (some
machines does not have "lsof" in the expected place). (add this->)The
empty list will make the test exit successfully.(<-add this)

> On 15/06/2018 11:21 AM, David Holmes wrote:
>> Hi Leo,
>>
>> This is what I was concerned about - and yes the CI testing did shake
>> it out in a few cycles.
>>
>> I agree with the fix and will sponsor it for you.

Thanks David.
/Leo

>>
>> Thanks,
>> David
>>
>> On 15/06/2018 3:31 AM, Leo Korinth wrote:
>>>
>>>
>>> On 14/06/18 19:10, Lindenmaier, Goetz wrote:
>>>> Hi Leo,
>>>>
>>>> the fix looks good.  It makes sense to skip a test if the
>>>> infrastructure is
>>>> insufficient to run it.
>>>>
>>>> Could you please put the long comment at the end of line 179
>>>> into a line of it's own?
>>>
>>> Fixed
>>>
>>>> Also, please take the chance and move the Copyright message
>>>> to the beginning of the file.
>>>
>>> Fixed
>>>
>>>> Best regards,
>>>>    Goetz.
>>>>
>>>
>>> Thanks for the fast review; I need a second reviewer and someone to
>>> help me push (I am not a commiter).
>>>
>>> New webrevs:
>>> http://cr.openjdk.java.net/~lkorinth/8205054/00_01/ (incremental)
>>> http://cr.openjdk.java.net/~lkorinth/8205054/01/    (full)
>>>
>>> Thanks,
>>> Leo
>>>
>>>>> -----Original Message-----
>>>>> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
>>>>> [hidden email]] On Behalf Of Leo Korinth
>>>>> Sent: Donnerstag, 14. Juni 2018 18:55
>>>>> To: Hotspot dev runtime <[hidden email]>
>>>>> Subject: RFR: 8205054: Could not find "lsof" on test machine
>>>>>
>>>>> Hi,
>>>>>
>>>>> Unfortunately some test machines does not have "lsof" installed, or it
>>>>> can not be found.
>>>>>
>>>>> I do not know how to find "lsof" on the test machine where the test
>>>>> fails; the command might not be installed. Here is a fix to make the
>>>>> test case succeed if "lsof" can not be found. I believe it is the best
>>>>> option (at least for now). I am sorry for all the noise I am
>>>>> generating.
>>>>>
>>>>> Bug:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8205054
>>>>>
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~lkorinth/8205054/00/
>>>>>
>>>>> Testing:
>>>>> I have tested this on my machine, and I have a mach5 test job running.
>>>>>
>>>>> Thanks,
>>>>> Leo
Reply | Threaded
Open this post in threaded view
|

Re: 8205054: Could not find "lsof" on test machine

David Holmes
Hi Leo,

Inine ...

On 15/06/2018 6:06 PM, Leo Korinth wrote:
> Hi David.
>
> On 15/06/18 04:08, David Holmes wrote:
>> Sorry after doing more testing and looking at the output I think we
>> need to do better. For a machine with no lsof we see:
>>
>> using command: <not found>

That looks like an error ...

>> (Second VM) Open file descriptors:
>>
>> using command: <not found>

That looks like an error too - but why did we keep going after the first
error?
>> (Third VM) Open file descriptors:
>>
>> VM RESULT => RETAINS FD
>> VM RESULT => VM EXIT
>>
>> Log file was not inherited by third VM

But that's not true - we couldn't determine anything! So this looks like
an accidental pass rather than an deliverate skip.

>> ---
>>
>> That looks like an error is occurring to me and that we pass by
>> accident. I think we need to terminate the test, cleanly of course, as
>> soon as we can't find the tool and report that the test can't run
>> because of that.
>>
>> Thanks,
>> David
>
> I could change the output to "using command: <not found, terminating
> test as successful as we can not finish it in a meaningful way>" or
> maybe something else that you prefer that is better.
>
> I do believe this is the easy way to terminate the test cleanly without
> leaving running JVMs. We need to communicate back to the first JVM that
> the test was successful. Is it the string "<not found>" that looks like
> an error and that we pass by accident? Or is it something in the test
> case code that looks fishy?

See above.

I'd suggest locating the command as one of the first things in the test
in the first VM and then bail out if not found.

Thanks,
David
------

> Or maybe the comment should be improved?
>    // if command can not be found return list without log file (some
> machines does not have "lsof" in the expected place). (add this->)The
> empty list will make the test exit successfully.(<-add this)
>
>> On 15/06/2018 11:21 AM, David Holmes wrote:
>>> Hi Leo,
>>>
>>> This is what I was concerned about - and yes the CI testing did shake
>>> it out in a few cycles.
>>>
>>> I agree with the fix and will sponsor it for you.
>
> Thanks David.
> /Leo
>
>>>
>>> Thanks,
>>> David
>>>
>>> On 15/06/2018 3:31 AM, Leo Korinth wrote:
>>>>
>>>>
>>>> On 14/06/18 19:10, Lindenmaier, Goetz wrote:
>>>>> Hi Leo,
>>>>>
>>>>> the fix looks good.  It makes sense to skip a test if the
>>>>> infrastructure is
>>>>> insufficient to run it.
>>>>>
>>>>> Could you please put the long comment at the end of line 179
>>>>> into a line of it's own?
>>>>
>>>> Fixed
>>>>
>>>>> Also, please take the chance and move the Copyright message
>>>>> to the beginning of the file.
>>>>
>>>> Fixed
>>>>
>>>>> Best regards,
>>>>>    Goetz.
>>>>>
>>>>
>>>> Thanks for the fast review; I need a second reviewer and someone to
>>>> help me push (I am not a commiter).
>>>>
>>>> New webrevs:
>>>> http://cr.openjdk.java.net/~lkorinth/8205054/00_01/ (incremental)
>>>> http://cr.openjdk.java.net/~lkorinth/8205054/01/    (full)
>>>>
>>>> Thanks,
>>>> Leo
>>>>
>>>>>> -----Original Message-----
>>>>>> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
>>>>>> [hidden email]] On Behalf Of Leo Korinth
>>>>>> Sent: Donnerstag, 14. Juni 2018 18:55
>>>>>> To: Hotspot dev runtime <[hidden email]>
>>>>>> Subject: RFR: 8205054: Could not find "lsof" on test machine
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Unfortunately some test machines does not have "lsof" installed,
>>>>>> or it
>>>>>> can not be found.
>>>>>>
>>>>>> I do not know how to find "lsof" on the test machine where the test
>>>>>> fails; the command might not be installed. Here is a fix to make the
>>>>>> test case succeed if "lsof" can not be found. I believe it is the
>>>>>> best
>>>>>> option (at least for now). I am sorry for all the noise I am
>>>>>> generating.
>>>>>>
>>>>>> Bug:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8205054
>>>>>>
>>>>>> Webrev:
>>>>>> http://cr.openjdk.java.net/~lkorinth/8205054/00/
>>>>>>
>>>>>> Testing:
>>>>>> I have tested this on my machine, and I have a mach5 test job
>>>>>> running.
>>>>>>
>>>>>> Thanks,
>>>>>> Leo
Reply | Threaded
Open this post in threaded view
|

Re: 8205054: Could not find "lsof" on test machine

Leo Korinth
Hi David,

Thanks for the input, it makes the test case _much_ better.
Unfortunately it is easy to get blind when you are in a hurry and
responsible for a failing test case. It is of course much better to look
for the command at start.

New output:

     STDOUT:
     Could not find lsof like command
     Exit test case as successful though it could not verify anything
     STDERR:

     JavaTest Message:  Test complete.

Webrev:
http://cr.openjdk.java.net/~lkorinth/8205054/01_02/ (incremental)
http://cr.openjdk.java.net/~lkorinth/8205054/02/    (full)

Testing:
Tested on my GNU/Linux machine and currently running a mach5 test
(mostly for verifying the windows part)

Thanks,
Leo


On 15/06/18 10:40, David Holmes wrote:

> Hi Leo,
>
> Inine ...
>
> On 15/06/2018 6:06 PM, Leo Korinth wrote:
>> Hi David.
>>
>> On 15/06/18 04:08, David Holmes wrote:
>>> Sorry after doing more testing and looking at the output I think we
>>> need to do better. For a machine with no lsof we see:
>>>
>>> using command: <not found>
>
> That looks like an error ...
>
>>> (Second VM) Open file descriptors:
>>>
>>> using command: <not found>
>
> That looks like an error too - but why did we keep going after the first
> error?
>>> (Third VM) Open file descriptors:
>>>
>>> VM RESULT => RETAINS FD
>>> VM RESULT => VM EXIT
>>>
>>> Log file was not inherited by third VM
>
> But that's not true - we couldn't determine anything! So this looks like
> an accidental pass rather than an deliverate skip.
>
>>> ---
>>>
>>> That looks like an error is occurring to me and that we pass by
>>> accident. I think we need to terminate the test, cleanly of course,
>>> as soon as we can't find the tool and report that the test can't run
>>> because of that.
>>>
>>> Thanks,
>>> David
>>
>> I could change the output to "using command: <not found, terminating
>> test as successful as we can not finish it in a meaningful way>" or
>> maybe something else that you prefer that is better.
>>
>> I do believe this is the easy way to terminate the test cleanly
>> without leaving running JVMs. We need to communicate back to the first
>> JVM that the test was successful. Is it the string "<not found>" that
>> looks like an error and that we pass by accident? Or is it something
>> in the test case code that looks fishy?
>
> See above.
>
> I'd suggest locating the command as one of the first things in the test
> in the first VM and then bail out if not found.
>
> Thanks,
> David
> ------
>
>> Or maybe the comment should be improved?
>>    // if command can not be found return list without log file (some
>> machines does not have "lsof" in the expected place). (add this->)The
>> empty list will make the test exit successfully.(<-add this)
>>
>>> On 15/06/2018 11:21 AM, David Holmes wrote:
>>>> Hi Leo,
>>>>
>>>> This is what I was concerned about - and yes the CI testing did
>>>> shake it out in a few cycles.
>>>>
>>>> I agree with the fix and will sponsor it for you.
>>
>> Thanks David.
>> /Leo
>>
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>> On 15/06/2018 3:31 AM, Leo Korinth wrote:
>>>>>
>>>>>
>>>>> On 14/06/18 19:10, Lindenmaier, Goetz wrote:
>>>>>> Hi Leo,
>>>>>>
>>>>>> the fix looks good.  It makes sense to skip a test if the
>>>>>> infrastructure is
>>>>>> insufficient to run it.
>>>>>>
>>>>>> Could you please put the long comment at the end of line 179
>>>>>> into a line of it's own?
>>>>>
>>>>> Fixed
>>>>>
>>>>>> Also, please take the chance and move the Copyright message
>>>>>> to the beginning of the file.
>>>>>
>>>>> Fixed
>>>>>
>>>>>> Best regards,
>>>>>>    Goetz.
>>>>>>
>>>>>
>>>>> Thanks for the fast review; I need a second reviewer and someone to
>>>>> help me push (I am not a commiter).
>>>>>
>>>>> New webrevs:
>>>>> http://cr.openjdk.java.net/~lkorinth/8205054/00_01/ (incremental)
>>>>> http://cr.openjdk.java.net/~lkorinth/8205054/01/    (full)
>>>>>
>>>>> Thanks,
>>>>> Leo
>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
>>>>>>> [hidden email]] On Behalf Of Leo Korinth
>>>>>>> Sent: Donnerstag, 14. Juni 2018 18:55
>>>>>>> To: Hotspot dev runtime <[hidden email]>
>>>>>>> Subject: RFR: 8205054: Could not find "lsof" on test machine
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> Unfortunately some test machines does not have "lsof" installed,
>>>>>>> or it
>>>>>>> can not be found.
>>>>>>>
>>>>>>> I do not know how to find "lsof" on the test machine where the test
>>>>>>> fails; the command might not be installed. Here is a fix to make the
>>>>>>> test case succeed if "lsof" can not be found. I believe it is the
>>>>>>> best
>>>>>>> option (at least for now). I am sorry for all the noise I am
>>>>>>> generating.
>>>>>>>
>>>>>>> Bug:
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8205054
>>>>>>>
>>>>>>> Webrev:
>>>>>>> http://cr.openjdk.java.net/~lkorinth/8205054/00/
>>>>>>>
>>>>>>> Testing:
>>>>>>> I have tested this on my machine, and I have a mach5 test job
>>>>>>> running.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Leo
Reply | Threaded
Open this post in threaded view
|

Re: 8205054: Could not find "lsof" on test machine

David Holmes
Hi Leo,
On 15/06/2018 7:37 PM, Leo Korinth wrote:

> Hi David,
>
> Thanks for the input, it makes the test case _much_ better.
> Unfortunately it is easy to get blind when you are in a hurry and
> responsible for a failing test case. It is of course much better to look
> for the command at start.
>
> New output:
>
>      STDOUT:
>      Could not find lsof like command
>      Exit test case as successful though it could not verify anything
>      STDERR:
>
>      JavaTest Message:  Test complete.

Much clearer - thanks!

> Webrev:
> http://cr.openjdk.java.net/~lkorinth/8205054/01_02/ (incremental)
> http://cr.openjdk.java.net/~lkorinth/8205054/02/    (full)

Generally looks good. Only nit is that the lsofCommand() result should
be cached in a field so you don't need to recalculate it for the actual
execution.

Thanks,
David

> Testing:
> Tested on my GNU/Linux machine and currently running a mach5 test
> (mostly for verifying the windows part)
>
> Thanks,
> Leo
>
>
> On 15/06/18 10:40, David Holmes wrote:
>> Hi Leo,
>>
>> Inine ...
>>
>> On 15/06/2018 6:06 PM, Leo Korinth wrote:
>>> Hi David.
>>>
>>> On 15/06/18 04:08, David Holmes wrote:
>>>> Sorry after doing more testing and looking at the output I think we
>>>> need to do better. For a machine with no lsof we see:
>>>>
>>>> using command: <not found>
>>
>> That looks like an error ...
>>
>>>> (Second VM) Open file descriptors:
>>>>
>>>> using command: <not found>
>>
>> That looks like an error too - but why did we keep going after the
>> first error?
>>>> (Third VM) Open file descriptors:
>>>>
>>>> VM RESULT => RETAINS FD
>>>> VM RESULT => VM EXIT
>>>>
>>>> Log file was not inherited by third VM
>>
>> But that's not true - we couldn't determine anything! So this looks
>> like an accidental pass rather than an deliverate skip.
>>
>>>> ---
>>>>
>>>> That looks like an error is occurring to me and that we pass by
>>>> accident. I think we need to terminate the test, cleanly of course,
>>>> as soon as we can't find the tool and report that the test can't run
>>>> because of that.
>>>>
>>>> Thanks,
>>>> David
>>>
>>> I could change the output to "using command: <not found, terminating
>>> test as successful as we can not finish it in a meaningful way>" or
>>> maybe something else that you prefer that is better.
>>>
>>> I do believe this is the easy way to terminate the test cleanly
>>> without leaving running JVMs. We need to communicate back to the
>>> first JVM that the test was successful. Is it the string "<not
>>> found>" that looks like an error and that we pass by accident? Or is
>>> it something in the test case code that looks fishy?
>>
>> See above.
>>
>> I'd suggest locating the command as one of the first things in the
>> test in the first VM and then bail out if not found.
>>
>> Thanks,
>> David
>> ------
>>
>>> Or maybe the comment should be improved?
>>>    // if command can not be found return list without log file (some
>>> machines does not have "lsof" in the expected place). (add this->)The
>>> empty list will make the test exit successfully.(<-add this)
>>>
>>>> On 15/06/2018 11:21 AM, David Holmes wrote:
>>>>> Hi Leo,
>>>>>
>>>>> This is what I was concerned about - and yes the CI testing did
>>>>> shake it out in a few cycles.
>>>>>
>>>>> I agree with the fix and will sponsor it for you.
>>>
>>> Thanks David.
>>> /Leo
>>>
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>> On 15/06/2018 3:31 AM, Leo Korinth wrote:
>>>>>>
>>>>>>
>>>>>> On 14/06/18 19:10, Lindenmaier, Goetz wrote:
>>>>>>> Hi Leo,
>>>>>>>
>>>>>>> the fix looks good.  It makes sense to skip a test if the
>>>>>>> infrastructure is
>>>>>>> insufficient to run it.
>>>>>>>
>>>>>>> Could you please put the long comment at the end of line 179
>>>>>>> into a line of it's own?
>>>>>>
>>>>>> Fixed
>>>>>>
>>>>>>> Also, please take the chance and move the Copyright message
>>>>>>> to the beginning of the file.
>>>>>>
>>>>>> Fixed
>>>>>>
>>>>>>> Best regards,
>>>>>>>    Goetz.
>>>>>>>
>>>>>>
>>>>>> Thanks for the fast review; I need a second reviewer and someone
>>>>>> to help me push (I am not a commiter).
>>>>>>
>>>>>> New webrevs:
>>>>>> http://cr.openjdk.java.net/~lkorinth/8205054/00_01/ (incremental)
>>>>>> http://cr.openjdk.java.net/~lkorinth/8205054/01/    (full)
>>>>>>
>>>>>> Thanks,
>>>>>> Leo
>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
>>>>>>>> [hidden email]] On Behalf Of Leo Korinth
>>>>>>>> Sent: Donnerstag, 14. Juni 2018 18:55
>>>>>>>> To: Hotspot dev runtime <[hidden email]>
>>>>>>>> Subject: RFR: 8205054: Could not find "lsof" on test machine
>>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Unfortunately some test machines does not have "lsof" installed,
>>>>>>>> or it
>>>>>>>> can not be found.
>>>>>>>>
>>>>>>>> I do not know how to find "lsof" on the test machine where the test
>>>>>>>> fails; the command might not be installed. Here is a fix to make
>>>>>>>> the
>>>>>>>> test case succeed if "lsof" can not be found. I believe it is
>>>>>>>> the best
>>>>>>>> option (at least for now). I am sorry for all the noise I am
>>>>>>>> generating.
>>>>>>>>
>>>>>>>> Bug:
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8205054
>>>>>>>>
>>>>>>>> Webrev:
>>>>>>>> http://cr.openjdk.java.net/~lkorinth/8205054/00/
>>>>>>>>
>>>>>>>> Testing:
>>>>>>>> I have tested this on my machine, and I have a mach5 test job
>>>>>>>> running.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Leo
Reply | Threaded
Open this post in threaded view
|

Re: 8205054: Could not find "lsof" on test machine

Leo Korinth
Hi David and Goetz.

I have added the last caching thing David asked for. Are you also okay
with the changes Goetz?

Webrev:
http://cr.openjdk.java.net/~lkorinth/8205054/02_03/ (incremental)
http://cr.openjdk.java.net/~lkorinth/8205054/03/    (full)

I also noticed that my test case unfortunately runs in many tiers. But
that is nothing that is special to my test case but is shared between
most test cases that are selected by being under "runtime/". I do not
feel qualified in doing changes there, and maybe there is a reason for it.

Thanks,
Leo

On 15/06/18 11:56, David Holmes wrote:

> Hi Leo,
> On 15/06/2018 7:37 PM, Leo Korinth wrote:
>> Hi David,
>>
>> Thanks for the input, it makes the test case _much_ better.
>> Unfortunately it is easy to get blind when you are in a hurry and
>> responsible for a failing test case. It is of course much better to
>> look for the command at start.
>>
>> New output:
>>
>>      STDOUT:
>>      Could not find lsof like command
>>      Exit test case as successful though it could not verify anything
>>      STDERR:
>>
>>      JavaTest Message:  Test complete.
>
> Much clearer - thanks!
>
>> Webrev:
>> http://cr.openjdk.java.net/~lkorinth/8205054/01_02/ (incremental)
>> http://cr.openjdk.java.net/~lkorinth/8205054/02/    (full)
>
> Generally looks good. Only nit is that the lsofCommand() result should
> be cached in a field so you don't need to recalculate it for the actual
> execution.
>
> Thanks,
> David
>
>> Testing:
>> Tested on my GNU/Linux machine and currently running a mach5 test
>> (mostly for verifying the windows part)
>>
>> Thanks,
>> Leo
>>
>>
>> On 15/06/18 10:40, David Holmes wrote:
>>> Hi Leo,
>>>
>>> Inine ...
>>>
>>> On 15/06/2018 6:06 PM, Leo Korinth wrote:
>>>> Hi David.
>>>>
>>>> On 15/06/18 04:08, David Holmes wrote:
>>>>> Sorry after doing more testing and looking at the output I think we
>>>>> need to do better. For a machine with no lsof we see:
>>>>>
>>>>> using command: <not found>
>>>
>>> That looks like an error ...
>>>
>>>>> (Second VM) Open file descriptors:
>>>>>
>>>>> using command: <not found>
>>>
>>> That looks like an error too - but why did we keep going after the
>>> first error?
>>>>> (Third VM) Open file descriptors:
>>>>>
>>>>> VM RESULT => RETAINS FD
>>>>> VM RESULT => VM EXIT
>>>>>
>>>>> Log file was not inherited by third VM
>>>
>>> But that's not true - we couldn't determine anything! So this looks
>>> like an accidental pass rather than an deliverate skip.
>>>
>>>>> ---
>>>>>
>>>>> That looks like an error is occurring to me and that we pass by
>>>>> accident. I think we need to terminate the test, cleanly of course,
>>>>> as soon as we can't find the tool and report that the test can't
>>>>> run because of that.
>>>>>
>>>>> Thanks,
>>>>> David
>>>>
>>>> I could change the output to "using command: <not found, terminating
>>>> test as successful as we can not finish it in a meaningful way>" or
>>>> maybe something else that you prefer that is better.
>>>>
>>>> I do believe this is the easy way to terminate the test cleanly
>>>> without leaving running JVMs. We need to communicate back to the
>>>> first JVM that the test was successful. Is it the string "<not
>>>> found>" that looks like an error and that we pass by accident? Or is
>>>> it something in the test case code that looks fishy?
>>>
>>> See above.
>>>
>>> I'd suggest locating the command as one of the first things in the
>>> test in the first VM and then bail out if not found.
>>>
>>> Thanks,
>>> David
>>> ------
>>>
>>>> Or maybe the comment should be improved?
>>>>    // if command can not be found return list without log file (some
>>>> machines does not have "lsof" in the expected place). (add
>>>> this->)The empty list will make the test exit successfully.(<-add this)
>>>>
>>>>> On 15/06/2018 11:21 AM, David Holmes wrote:
>>>>>> Hi Leo,
>>>>>>
>>>>>> This is what I was concerned about - and yes the CI testing did
>>>>>> shake it out in a few cycles.
>>>>>>
>>>>>> I agree with the fix and will sponsor it for you.
>>>>
>>>> Thanks David.
>>>> /Leo
>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>>
>>>>>> On 15/06/2018 3:31 AM, Leo Korinth wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 14/06/18 19:10, Lindenmaier, Goetz wrote:
>>>>>>>> Hi Leo,
>>>>>>>>
>>>>>>>> the fix looks good.  It makes sense to skip a test if the
>>>>>>>> infrastructure is
>>>>>>>> insufficient to run it.
>>>>>>>>
>>>>>>>> Could you please put the long comment at the end of line 179
>>>>>>>> into a line of it's own?
>>>>>>>
>>>>>>> Fixed
>>>>>>>
>>>>>>>> Also, please take the chance and move the Copyright message
>>>>>>>> to the beginning of the file.
>>>>>>>
>>>>>>> Fixed
>>>>>>>
>>>>>>>> Best regards,
>>>>>>>>    Goetz.
>>>>>>>>
>>>>>>>
>>>>>>> Thanks for the fast review; I need a second reviewer and someone
>>>>>>> to help me push (I am not a commiter).
>>>>>>>
>>>>>>> New webrevs:
>>>>>>> http://cr.openjdk.java.net/~lkorinth/8205054/00_01/ (incremental)
>>>>>>> http://cr.openjdk.java.net/~lkorinth/8205054/01/    (full)
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Leo
>>>>>>>
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
>>>>>>>>> [hidden email]] On Behalf Of Leo Korinth
>>>>>>>>> Sent: Donnerstag, 14. Juni 2018 18:55
>>>>>>>>> To: Hotspot dev runtime <[hidden email]>
>>>>>>>>> Subject: RFR: 8205054: Could not find "lsof" on test machine
>>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> Unfortunately some test machines does not have "lsof"
>>>>>>>>> installed, or it
>>>>>>>>> can not be found.
>>>>>>>>>
>>>>>>>>> I do not know how to find "lsof" on the test machine where the
>>>>>>>>> test
>>>>>>>>> fails; the command might not be installed. Here is a fix to
>>>>>>>>> make the
>>>>>>>>> test case succeed if "lsof" can not be found. I believe it is
>>>>>>>>> the best
>>>>>>>>> option (at least for now). I am sorry for all the noise I am
>>>>>>>>> generating.
>>>>>>>>>
>>>>>>>>> Bug:
>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8205054
>>>>>>>>>
>>>>>>>>> Webrev:
>>>>>>>>> http://cr.openjdk.java.net/~lkorinth/8205054/00/
>>>>>>>>>
>>>>>>>>> Testing:
>>>>>>>>> I have tested this on my machine, and I have a mach5 test job
>>>>>>>>> running.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Leo
Reply | Threaded
Open this post in threaded view
|

Re: 8205054: Could not find "lsof" on test machine

David Holmes
On 15/06/2018 9:53 PM, Leo Korinth wrote:
> Hi David and Goetz.
>
> I have added the last caching thing David asked for. Are you also okay
> with the changes Goetz?
>
> Webrev:
> http://cr.openjdk.java.net/~lkorinth/8205054/02_03/ (incremental)
> http://cr.openjdk.java.net/~lkorinth/8205054/03/    (full)

That looks fine to me.

> I also noticed that my test case unfortunately runs in many tiers. But
> that is nothing that is special to my test case but is shared between
> most test cases that are selected by being under "runtime/". I do not
> feel qualified in doing changes there, and maybe there is a reason for it.

Different tiers can involve different build variations and also
different runtime flags. So tests get run in different ways.

David

> Thanks,
> Leo
>
> On 15/06/18 11:56, David Holmes wrote:
>> Hi Leo,
>> On 15/06/2018 7:37 PM, Leo Korinth wrote:
>>> Hi David,
>>>
>>> Thanks for the input, it makes the test case _much_ better.
>>> Unfortunately it is easy to get blind when you are in a hurry and
>>> responsible for a failing test case. It is of course much better to
>>> look for the command at start.
>>>
>>> New output:
>>>
>>>      STDOUT:
>>>      Could not find lsof like command
>>>      Exit test case as successful though it could not verify anything
>>>      STDERR:
>>>
>>>      JavaTest Message:  Test complete.
>>
>> Much clearer - thanks!
>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~lkorinth/8205054/01_02/ (incremental)
>>> http://cr.openjdk.java.net/~lkorinth/8205054/02/    (full)
>>
>> Generally looks good. Only nit is that the lsofCommand() result should
>> be cached in a field so you don't need to recalculate it for the
>> actual execution.
>>
>> Thanks,
>> David
>>
>>> Testing:
>>> Tested on my GNU/Linux machine and currently running a mach5 test
>>> (mostly for verifying the windows part)
>>>
>>> Thanks,
>>> Leo
>>>
>>>
>>> On 15/06/18 10:40, David Holmes wrote:
>>>> Hi Leo,
>>>>
>>>> Inine ...
>>>>
>>>> On 15/06/2018 6:06 PM, Leo Korinth wrote:
>>>>> Hi David.
>>>>>
>>>>> On 15/06/18 04:08, David Holmes wrote:
>>>>>> Sorry after doing more testing and looking at the output I think
>>>>>> we need to do better. For a machine with no lsof we see:
>>>>>>
>>>>>> using command: <not found>
>>>>
>>>> That looks like an error ...
>>>>
>>>>>> (Second VM) Open file descriptors:
>>>>>>
>>>>>> using command: <not found>
>>>>
>>>> That looks like an error too - but why did we keep going after the
>>>> first error?
>>>>>> (Third VM) Open file descriptors:
>>>>>>
>>>>>> VM RESULT => RETAINS FD
>>>>>> VM RESULT => VM EXIT
>>>>>>
>>>>>> Log file was not inherited by third VM
>>>>
>>>> But that's not true - we couldn't determine anything! So this looks
>>>> like an accidental pass rather than an deliverate skip.
>>>>
>>>>>> ---
>>>>>>
>>>>>> That looks like an error is occurring to me and that we pass by
>>>>>> accident. I think we need to terminate the test, cleanly of
>>>>>> course, as soon as we can't find the tool and report that the test
>>>>>> can't run because of that.
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>
>>>>> I could change the output to "using command: <not found,
>>>>> terminating test as successful as we can not finish it in a
>>>>> meaningful way>" or maybe something else that you prefer that is
>>>>> better.
>>>>>
>>>>> I do believe this is the easy way to terminate the test cleanly
>>>>> without leaving running JVMs. We need to communicate back to the
>>>>> first JVM that the test was successful. Is it the string "<not
>>>>> found>" that looks like an error and that we pass by accident? Or
>>>>> is it something in the test case code that looks fishy?
>>>>
>>>> See above.
>>>>
>>>> I'd suggest locating the command as one of the first things in the
>>>> test in the first VM and then bail out if not found.
>>>>
>>>> Thanks,
>>>> David
>>>> ------
>>>>
>>>>> Or maybe the comment should be improved?
>>>>>    // if command can not be found return list without log file
>>>>> (some machines does not have "lsof" in the expected place). (add
>>>>> this->)The empty list will make the test exit successfully.(<-add
>>>>> this)
>>>>>
>>>>>> On 15/06/2018 11:21 AM, David Holmes wrote:
>>>>>>> Hi Leo,
>>>>>>>
>>>>>>> This is what I was concerned about - and yes the CI testing did
>>>>>>> shake it out in a few cycles.
>>>>>>>
>>>>>>> I agree with the fix and will sponsor it for you.
>>>>>
>>>>> Thanks David.
>>>>> /Leo
>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>>
>>>>>>> On 15/06/2018 3:31 AM, Leo Korinth wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 14/06/18 19:10, Lindenmaier, Goetz wrote:
>>>>>>>>> Hi Leo,
>>>>>>>>>
>>>>>>>>> the fix looks good.  It makes sense to skip a test if the
>>>>>>>>> infrastructure is
>>>>>>>>> insufficient to run it.
>>>>>>>>>
>>>>>>>>> Could you please put the long comment at the end of line 179
>>>>>>>>> into a line of it's own?
>>>>>>>>
>>>>>>>> Fixed
>>>>>>>>
>>>>>>>>> Also, please take the chance and move the Copyright message
>>>>>>>>> to the beginning of the file.
>>>>>>>>
>>>>>>>> Fixed
>>>>>>>>
>>>>>>>>> Best regards,
>>>>>>>>>    Goetz.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks for the fast review; I need a second reviewer and someone
>>>>>>>> to help me push (I am not a commiter).
>>>>>>>>
>>>>>>>> New webrevs:
>>>>>>>> http://cr.openjdk.java.net/~lkorinth/8205054/00_01/ (incremental)
>>>>>>>> http://cr.openjdk.java.net/~lkorinth/8205054/01/    (full)
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Leo
>>>>>>>>
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
>>>>>>>>>> [hidden email]] On Behalf Of Leo Korinth
>>>>>>>>>> Sent: Donnerstag, 14. Juni 2018 18:55
>>>>>>>>>> To: Hotspot dev runtime <[hidden email]>
>>>>>>>>>> Subject: RFR: 8205054: Could not find "lsof" on test machine
>>>>>>>>>>
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> Unfortunately some test machines does not have "lsof"
>>>>>>>>>> installed, or it
>>>>>>>>>> can not be found.
>>>>>>>>>>
>>>>>>>>>> I do not know how to find "lsof" on the test machine where the
>>>>>>>>>> test
>>>>>>>>>> fails; the command might not be installed. Here is a fix to
>>>>>>>>>> make the
>>>>>>>>>> test case succeed if "lsof" can not be found. I believe it is
>>>>>>>>>> the best
>>>>>>>>>> option (at least for now). I am sorry for all the noise I am
>>>>>>>>>> generating.
>>>>>>>>>>
>>>>>>>>>> Bug:
>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8205054
>>>>>>>>>>
>>>>>>>>>> Webrev:
>>>>>>>>>> http://cr.openjdk.java.net/~lkorinth/8205054/00/
>>>>>>>>>>
>>>>>>>>>> Testing:
>>>>>>>>>> I have tested this on my machine, and I have a mach5 test job
>>>>>>>>>> running.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Leo
Reply | Threaded
Open this post in threaded view
|

Re: 8205054: Could not find "lsof" on test machine

Mikael Vidstedt-3
In reply to this post by Leo Korinth

Two minor things which can be done as follow-ups if we want to get the fix pushed quickly:


+        if (isWindows() == false && lsofCommand().isPresent() == false) {

Booleans are not normally compared with explicit values, so for consistency with the other checks in the code I’d suggest changing that to:

if (!isWindows() && !lsofCommand().isPresent()) {


Also, there is already an isWindows() method in test/lib/jdk/test/lib/Platform.java which would probably be better to (re-)use.


Apart from that it looks good!

Cheers,
Mikael


> On Jun 15, 2018, at 4:53 AM, Leo Korinth <[hidden email]> wrote:
>
> Hi David and Goetz.
>
> I have added the last caching thing David asked for. Are you also okay with the changes Goetz?
>
> Webrev:
> http://cr.openjdk.java.net/~lkorinth/8205054/02_03/ (incremental)
> http://cr.openjdk.java.net/~lkorinth/8205054/03/    (full)
>
> I also noticed that my test case unfortunately runs in many tiers. But that is nothing that is special to my test case but is shared between most test cases that are selected by being under "runtime/". I do not feel qualified in doing changes there, and maybe there is a reason for it.
>
> Thanks,
> Leo
>
> On 15/06/18 11:56, David Holmes wrote:
>> Hi Leo,
>> On 15/06/2018 7:37 PM, Leo Korinth wrote:
>>> Hi David,
>>>
>>> Thanks for the input, it makes the test case _much_ better. Unfortunately it is easy to get blind when you are in a hurry and responsible for a failing test case. It is of course much better to look for the command at start.
>>>
>>> New output:
>>>
>>>      STDOUT:
>>>      Could not find lsof like command
>>>      Exit test case as successful though it could not verify anything
>>>      STDERR:
>>>
>>>      JavaTest Message:  Test complete.
>> Much clearer - thanks!
>>> Webrev:
>>> http://cr.openjdk.java.net/~lkorinth/8205054/01_02/ (incremental)
>>> http://cr.openjdk.java.net/~lkorinth/8205054/02/    (full)
>> Generally looks good. Only nit is that the lsofCommand() result should be cached in a field so you don't need to recalculate it for the actual execution.
>> Thanks,
>> David
>>> Testing:
>>> Tested on my GNU/Linux machine and currently running a mach5 test (mostly for verifying the windows part)
>>>
>>> Thanks,
>>> Leo
>>>
>>>
>>> On 15/06/18 10:40, David Holmes wrote:
>>>> Hi Leo,
>>>>
>>>> Inine ...
>>>>
>>>> On 15/06/2018 6:06 PM, Leo Korinth wrote:
>>>>> Hi David.
>>>>>
>>>>> On 15/06/18 04:08, David Holmes wrote:
>>>>>> Sorry after doing more testing and looking at the output I think we need to do better. For a machine with no lsof we see:
>>>>>>
>>>>>> using command: <not found>
>>>>
>>>> That looks like an error ...
>>>>
>>>>>> (Second VM) Open file descriptors:
>>>>>>
>>>>>> using command: <not found>
>>>>
>>>> That looks like an error too - but why did we keep going after the first error?
>>>>>> (Third VM) Open file descriptors:
>>>>>>
>>>>>> VM RESULT => RETAINS FD
>>>>>> VM RESULT => VM EXIT
>>>>>>
>>>>>> Log file was not inherited by third VM
>>>>
>>>> But that's not true - we couldn't determine anything! So this looks like an accidental pass rather than an deliverate skip.
>>>>
>>>>>> ---
>>>>>>
>>>>>> That looks like an error is occurring to me and that we pass by accident. I think we need to terminate the test, cleanly of course, as soon as we can't find the tool and report that the test can't run because of that.
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>
>>>>> I could change the output to "using command: <not found, terminating test as successful as we can not finish it in a meaningful way>" or maybe something else that you prefer that is better.
>>>>>
>>>>> I do believe this is the easy way to terminate the test cleanly without leaving running JVMs. We need to communicate back to the first JVM that the test was successful. Is it the string "<not found>" that looks like an error and that we pass by accident? Or is it something in the test case code that looks fishy?
>>>>
>>>> See above.
>>>>
>>>> I'd suggest locating the command as one of the first things in the test in the first VM and then bail out if not found.
>>>>
>>>> Thanks,
>>>> David
>>>> ------
>>>>
>>>>> Or maybe the comment should be improved?
>>>>>    // if command can not be found return list without log file (some machines does not have "lsof" in the expected place). (add this->)The empty list will make the test exit successfully.(<-add this)
>>>>>
>>>>>> On 15/06/2018 11:21 AM, David Holmes wrote:
>>>>>>> Hi Leo,
>>>>>>>
>>>>>>> This is what I was concerned about - and yes the CI testing did shake it out in a few cycles.
>>>>>>>
>>>>>>> I agree with the fix and will sponsor it for you.
>>>>>
>>>>> Thanks David.
>>>>> /Leo
>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>>
>>>>>>> On 15/06/2018 3:31 AM, Leo Korinth wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 14/06/18 19:10, Lindenmaier, Goetz wrote:
>>>>>>>>> Hi Leo,
>>>>>>>>>
>>>>>>>>> the fix looks good.  It makes sense to skip a test if the infrastructure is
>>>>>>>>> insufficient to run it.
>>>>>>>>>
>>>>>>>>> Could you please put the long comment at the end of line 179
>>>>>>>>> into a line of it's own?
>>>>>>>>
>>>>>>>> Fixed
>>>>>>>>
>>>>>>>>> Also, please take the chance and move the Copyright message
>>>>>>>>> to the beginning of the file.
>>>>>>>>
>>>>>>>> Fixed
>>>>>>>>
>>>>>>>>> Best regards,
>>>>>>>>>    Goetz.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Thanks for the fast review; I need a second reviewer and someone to help me push (I am not a commiter).
>>>>>>>>
>>>>>>>> New webrevs:
>>>>>>>> http://cr.openjdk.java.net/~lkorinth/8205054/00_01/ (incremental)
>>>>>>>> http://cr.openjdk.java.net/~lkorinth/8205054/01/    (full)
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Leo
>>>>>>>>
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
>>>>>>>>>> [hidden email]] On Behalf Of Leo Korinth
>>>>>>>>>> Sent: Donnerstag, 14. Juni 2018 18:55
>>>>>>>>>> To: Hotspot dev runtime <[hidden email]>
>>>>>>>>>> Subject: RFR: 8205054: Could not find "lsof" on test machine
>>>>>>>>>>
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> Unfortunately some test machines does not have "lsof" installed, or it
>>>>>>>>>> can not be found.
>>>>>>>>>>
>>>>>>>>>> I do not know how to find "lsof" on the test machine where the test
>>>>>>>>>> fails; the command might not be installed. Here is a fix to make the
>>>>>>>>>> test case succeed if "lsof" can not be found. I believe it is the best
>>>>>>>>>> option (at least for now). I am sorry for all the noise I am generating.
>>>>>>>>>>
>>>>>>>>>> Bug:
>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8205054
>>>>>>>>>>
>>>>>>>>>> Webrev:
>>>>>>>>>> http://cr.openjdk.java.net/~lkorinth/8205054/00/
>>>>>>>>>>
>>>>>>>>>> Testing:
>>>>>>>>>> I have tested this on my machine, and I have a mach5 test job running.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Leo

Reply | Threaded
Open this post in threaded view
|

Re: 8205054: Could not find "lsof" on test machine

David Holmes
Hi Mikael,

On 19/06/2018 9:43 AM, Mikael Vidstedt wrote:
>
> Two minor things which can be done as follow-ups if we want to get the fix pushed quickly:
>
>
> +        if (isWindows() == false && lsofCommand().isPresent() == false) {
>
> Booleans are not normally compared with explicit values, so for consistency with the other checks in the code I’d suggest changing that to:
>
> if (!isWindows() && !lsofCommand().isPresent()) {

Oops - good catch.
> Also, there is already an isWindows() method in test/lib/jdk/test/lib/Platform.java which would probably be better to (re-)use.

Also good point.

> Apart from that it looks good!

Thanks. We had been awaiting further word from Goetz before I pushed
this for Leo, but I understand he is, or was, away for a couple of days.
In the interests of getting the CI testing cleaned up I'll push this
with your suggested cosmetic changes applied. (Leo I hope you don't mind
:) )

Thanks,
David

> Cheers,
> Mikael
>
>
>> On Jun 15, 2018, at 4:53 AM, Leo Korinth <[hidden email]> wrote:
>>
>> Hi David and Goetz.
>>
>> I have added the last caching thing David asked for. Are you also okay with the changes Goetz?
>>
>> Webrev:
>> http://cr.openjdk.java.net/~lkorinth/8205054/02_03/ (incremental)
>> http://cr.openjdk.java.net/~lkorinth/8205054/03/    (full)
>>
>> I also noticed that my test case unfortunately runs in many tiers. But that is nothing that is special to my test case but is shared between most test cases that are selected by being under "runtime/". I do not feel qualified in doing changes there, and maybe there is a reason for it.
>>
>> Thanks,
>> Leo
>>
>> On 15/06/18 11:56, David Holmes wrote:
>>> Hi Leo,
>>> On 15/06/2018 7:37 PM, Leo Korinth wrote:
>>>> Hi David,
>>>>
>>>> Thanks for the input, it makes the test case _much_ better. Unfortunately it is easy to get blind when you are in a hurry and responsible for a failing test case. It is of course much better to look for the command at start.
>>>>
>>>> New output:
>>>>
>>>>       STDOUT:
>>>>       Could not find lsof like command
>>>>       Exit test case as successful though it could not verify anything
>>>>       STDERR:
>>>>
>>>>       JavaTest Message:  Test complete.
>>> Much clearer - thanks!
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~lkorinth/8205054/01_02/ (incremental)
>>>> http://cr.openjdk.java.net/~lkorinth/8205054/02/    (full)
>>> Generally looks good. Only nit is that the lsofCommand() result should be cached in a field so you don't need to recalculate it for the actual execution.
>>> Thanks,
>>> David
>>>> Testing:
>>>> Tested on my GNU/Linux machine and currently running a mach5 test (mostly for verifying the windows part)
>>>>
>>>> Thanks,
>>>> Leo
>>>>
>>>>
>>>> On 15/06/18 10:40, David Holmes wrote:
>>>>> Hi Leo,
>>>>>
>>>>> Inine ...
>>>>>
>>>>> On 15/06/2018 6:06 PM, Leo Korinth wrote:
>>>>>> Hi David.
>>>>>>
>>>>>> On 15/06/18 04:08, David Holmes wrote:
>>>>>>> Sorry after doing more testing and looking at the output I think we need to do better. For a machine with no lsof we see:
>>>>>>>
>>>>>>> using command: <not found>
>>>>>
>>>>> That looks like an error ...
>>>>>
>>>>>>> (Second VM) Open file descriptors:
>>>>>>>
>>>>>>> using command: <not found>
>>>>>
>>>>> That looks like an error too - but why did we keep going after the first error?
>>>>>>> (Third VM) Open file descriptors:
>>>>>>>
>>>>>>> VM RESULT => RETAINS FD
>>>>>>> VM RESULT => VM EXIT
>>>>>>>
>>>>>>> Log file was not inherited by third VM
>>>>>
>>>>> But that's not true - we couldn't determine anything! So this looks like an accidental pass rather than an deliverate skip.
>>>>>
>>>>>>> ---
>>>>>>>
>>>>>>> That looks like an error is occurring to me and that we pass by accident. I think we need to terminate the test, cleanly of course, as soon as we can't find the tool and report that the test can't run because of that.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>
>>>>>> I could change the output to "using command: <not found, terminating test as successful as we can not finish it in a meaningful way>" or maybe something else that you prefer that is better.
>>>>>>
>>>>>> I do believe this is the easy way to terminate the test cleanly without leaving running JVMs. We need to communicate back to the first JVM that the test was successful. Is it the string "<not found>" that looks like an error and that we pass by accident? Or is it something in the test case code that looks fishy?
>>>>>
>>>>> See above.
>>>>>
>>>>> I'd suggest locating the command as one of the first things in the test in the first VM and then bail out if not found.
>>>>>
>>>>> Thanks,
>>>>> David
>>>>> ------
>>>>>
>>>>>> Or maybe the comment should be improved?
>>>>>>     // if command can not be found return list without log file (some machines does not have "lsof" in the expected place). (add this->)The empty list will make the test exit successfully.(<-add this)
>>>>>>
>>>>>>> On 15/06/2018 11:21 AM, David Holmes wrote:
>>>>>>>> Hi Leo,
>>>>>>>>
>>>>>>>> This is what I was concerned about - and yes the CI testing did shake it out in a few cycles.
>>>>>>>>
>>>>>>>> I agree with the fix and will sponsor it for you.
>>>>>>
>>>>>> Thanks David.
>>>>>> /Leo
>>>>>>
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> David
>>>>>>>>
>>>>>>>> On 15/06/2018 3:31 AM, Leo Korinth wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 14/06/18 19:10, Lindenmaier, Goetz wrote:
>>>>>>>>>> Hi Leo,
>>>>>>>>>>
>>>>>>>>>> the fix looks good.  It makes sense to skip a test if the infrastructure is
>>>>>>>>>> insufficient to run it.
>>>>>>>>>>
>>>>>>>>>> Could you please put the long comment at the end of line 179
>>>>>>>>>> into a line of it's own?
>>>>>>>>>
>>>>>>>>> Fixed
>>>>>>>>>
>>>>>>>>>> Also, please take the chance and move the Copyright message
>>>>>>>>>> to the beginning of the file.
>>>>>>>>>
>>>>>>>>> Fixed
>>>>>>>>>
>>>>>>>>>> Best regards,
>>>>>>>>>>     Goetz.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks for the fast review; I need a second reviewer and someone to help me push (I am not a commiter).
>>>>>>>>>
>>>>>>>>> New webrevs:
>>>>>>>>> http://cr.openjdk.java.net/~lkorinth/8205054/00_01/ (incremental)
>>>>>>>>> http://cr.openjdk.java.net/~lkorinth/8205054/01/    (full)
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Leo
>>>>>>>>>
>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
>>>>>>>>>>> [hidden email]] On Behalf Of Leo Korinth
>>>>>>>>>>> Sent: Donnerstag, 14. Juni 2018 18:55
>>>>>>>>>>> To: Hotspot dev runtime <[hidden email]>
>>>>>>>>>>> Subject: RFR: 8205054: Could not find "lsof" on test machine
>>>>>>>>>>>
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> Unfortunately some test machines does not have "lsof" installed, or it
>>>>>>>>>>> can not be found.
>>>>>>>>>>>
>>>>>>>>>>> I do not know how to find "lsof" on the test machine where the test
>>>>>>>>>>> fails; the command might not be installed. Here is a fix to make the
>>>>>>>>>>> test case succeed if "lsof" can not be found. I believe it is the best
>>>>>>>>>>> option (at least for now). I am sorry for all the noise I am generating.
>>>>>>>>>>>
>>>>>>>>>>> Bug:
>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8205054
>>>>>>>>>>>
>>>>>>>>>>> Webrev:
>>>>>>>>>>> http://cr.openjdk.java.net/~lkorinth/8205054/00/
>>>>>>>>>>>
>>>>>>>>>>> Testing:
>>>>>>>>>>> I have tested this on my machine, and I have a mach5 test job running.
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Leo
>
Reply | Threaded
Open this post in threaded view
|

Re: 8205054: Could not find "lsof" on test machine

Leo Korinth
Thanks David,
Leo

On 19/06/18 01:58, David Holmes wrote:

> Hi Mikael,
>
> On 19/06/2018 9:43 AM, Mikael Vidstedt wrote:
>>
>> Two minor things which can be done as follow-ups if we want to get the
>> fix pushed quickly:
>>
>>
>> +        if (isWindows() == false && lsofCommand().isPresent() ==
>> false) {
>>
>> Booleans are not normally compared with explicit values, so for
>> consistency with the other checks in the code I’d suggest changing
>> that to:
>>
>> if (!isWindows() && !lsofCommand().isPresent()) {
>
> Oops - good catch.
>> Also, there is already an isWindows() method in
>> test/lib/jdk/test/lib/Platform.java which would probably be better to
>> (re-)use.
>
> Also good point.
>
>> Apart from that it looks good!
>
> Thanks. We had been awaiting further word from Goetz before I pushed
> this for Leo, but I understand he is, or was, away for a couple of days.
> In the interests of getting the CI testing cleaned up I'll push this
> with your suggested cosmetic changes applied. (Leo I hope you don't mind
> :) )
>
> Thanks,
> David
>
>> Cheers,
>> Mikael
>>
>>
>>> On Jun 15, 2018, at 4:53 AM, Leo Korinth <[hidden email]> wrote:
>>>
>>> Hi David and Goetz.
>>>
>>> I have added the last caching thing David asked for. Are you also
>>> okay with the changes Goetz?
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~lkorinth/8205054/02_03/ (incremental)
>>> http://cr.openjdk.java.net/~lkorinth/8205054/03/    (full)
>>>
>>> I also noticed that my test case unfortunately runs in many tiers.
>>> But that is nothing that is special to my test case but is shared
>>> between most test cases that are selected by being under "runtime/".
>>> I do not feel qualified in doing changes there, and maybe there is a
>>> reason for it.
>>>
>>> Thanks,
>>> Leo
>>>
>>> On 15/06/18 11:56, David Holmes wrote:
>>>> Hi Leo,
>>>> On 15/06/2018 7:37 PM, Leo Korinth wrote:
>>>>> Hi David,
>>>>>
>>>>> Thanks for the input, it makes the test case _much_ better.
>>>>> Unfortunately it is easy to get blind when you are in a hurry and
>>>>> responsible for a failing test case. It is of course much better to
>>>>> look for the command at start.
>>>>>
>>>>> New output:
>>>>>
>>>>>       STDOUT:
>>>>>       Could not find lsof like command
>>>>>       Exit test case as successful though it could not verify anything
>>>>>       STDERR:
>>>>>
>>>>>       JavaTest Message:  Test complete.
>>>> Much clearer - thanks!
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~lkorinth/8205054/01_02/ (incremental)
>>>>> http://cr.openjdk.java.net/~lkorinth/8205054/02/    (full)
>>>> Generally looks good. Only nit is that the lsofCommand() result
>>>> should be cached in a field so you don't need to recalculate it for
>>>> the actual execution.
>>>> Thanks,
>>>> David
>>>>> Testing:
>>>>> Tested on my GNU/Linux machine and currently running a mach5 test
>>>>> (mostly for verifying the windows part)
>>>>>
>>>>> Thanks,
>>>>> Leo
>>>>>
>>>>>
>>>>> On 15/06/18 10:40, David Holmes wrote:
>>>>>> Hi Leo,
>>>>>>
>>>>>> Inine ...
>>>>>>
>>>>>> On 15/06/2018 6:06 PM, Leo Korinth wrote:
>>>>>>> Hi David.
>>>>>>>
>>>>>>> On 15/06/18 04:08, David Holmes wrote:
>>>>>>>> Sorry after doing more testing and looking at the output I think
>>>>>>>> we need to do better. For a machine with no lsof we see:
>>>>>>>>
>>>>>>>> using command: <not found>
>>>>>>
>>>>>> That looks like an error ...
>>>>>>
>>>>>>>> (Second VM) Open file descriptors:
>>>>>>>>
>>>>>>>> using command: <not found>
>>>>>>
>>>>>> That looks like an error too - but why did we keep going after the
>>>>>> first error?
>>>>>>>> (Third VM) Open file descriptors:
>>>>>>>>
>>>>>>>> VM RESULT => RETAINS FD
>>>>>>>> VM RESULT => VM EXIT
>>>>>>>>
>>>>>>>> Log file was not inherited by third VM
>>>>>>
>>>>>> But that's not true - we couldn't determine anything! So this
>>>>>> looks like an accidental pass rather than an deliverate skip.
>>>>>>
>>>>>>>> ---
>>>>>>>>
>>>>>>>> That looks like an error is occurring to me and that we pass by
>>>>>>>> accident. I think we need to terminate the test, cleanly of
>>>>>>>> course, as soon as we can't find the tool and report that the
>>>>>>>> test can't run because of that.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> David
>>>>>>>
>>>>>>> I could change the output to "using command: <not found,
>>>>>>> terminating test as successful as we can not finish it in a
>>>>>>> meaningful way>" or maybe something else that you prefer that is
>>>>>>> better.
>>>>>>>
>>>>>>> I do believe this is the easy way to terminate the test cleanly
>>>>>>> without leaving running JVMs. We need to communicate back to the
>>>>>>> first JVM that the test was successful. Is it the string "<not
>>>>>>> found>" that looks like an error and that we pass by accident? Or
>>>>>>> is it something in the test case code that looks fishy?
>>>>>>
>>>>>> See above.
>>>>>>
>>>>>> I'd suggest locating the command as one of the first things in the
>>>>>> test in the first VM and then bail out if not found.
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>> ------
>>>>>>
>>>>>>> Or maybe the comment should be improved?
>>>>>>>     // if command can not be found return list without log file
>>>>>>> (some machines does not have "lsof" in the expected place). (add
>>>>>>> this->)The empty list will make the test exit successfully.(<-add
>>>>>>> this)
>>>>>>>
>>>>>>>> On 15/06/2018 11:21 AM, David Holmes wrote:
>>>>>>>>> Hi Leo,
>>>>>>>>>
>>>>>>>>> This is what I was concerned about - and yes the CI testing did
>>>>>>>>> shake it out in a few cycles.
>>>>>>>>>
>>>>>>>>> I agree with the fix and will sponsor it for you.
>>>>>>>
>>>>>>> Thanks David.
>>>>>>> /Leo
>>>>>>>
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> David
>>>>>>>>>
>>>>>>>>> On 15/06/2018 3:31 AM, Leo Korinth wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 14/06/18 19:10, Lindenmaier, Goetz wrote:
>>>>>>>>>>> Hi Leo,
>>>>>>>>>>>
>>>>>>>>>>> the fix looks good.  It makes sense to skip a test if the
>>>>>>>>>>> infrastructure is
>>>>>>>>>>> insufficient to run it.
>>>>>>>>>>>
>>>>>>>>>>> Could you please put the long comment at the end of line 179
>>>>>>>>>>> into a line of it's own?
>>>>>>>>>>
>>>>>>>>>> Fixed
>>>>>>>>>>
>>>>>>>>>>> Also, please take the chance and move the Copyright message
>>>>>>>>>>> to the beginning of the file.
>>>>>>>>>>
>>>>>>>>>> Fixed
>>>>>>>>>>
>>>>>>>>>>> Best regards,
>>>>>>>>>>>     Goetz.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks for the fast review; I need a second reviewer and
>>>>>>>>>> someone to help me push (I am not a commiter).
>>>>>>>>>>
>>>>>>>>>> New webrevs:
>>>>>>>>>> http://cr.openjdk.java.net/~lkorinth/8205054/00_01/ (incremental)
>>>>>>>>>> http://cr.openjdk.java.net/~lkorinth/8205054/01/    (full)
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Leo
>>>>>>>>>>
>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
>>>>>>>>>>>> [hidden email]] On Behalf Of Leo Korinth
>>>>>>>>>>>> Sent: Donnerstag, 14. Juni 2018 18:55
>>>>>>>>>>>> To: Hotspot dev runtime <[hidden email]>
>>>>>>>>>>>> Subject: RFR: 8205054: Could not find "lsof" on test machine
>>>>>>>>>>>>
>>>>>>>>>>>> Hi,
>>>>>>>>>>>>
>>>>>>>>>>>> Unfortunately some test machines does not have "lsof"
>>>>>>>>>>>> installed, or it
>>>>>>>>>>>> can not be found.
>>>>>>>>>>>>
>>>>>>>>>>>> I do not know how to find "lsof" on the test machine where
>>>>>>>>>>>> the test
>>>>>>>>>>>> fails; the command might not be installed. Here is a fix to
>>>>>>>>>>>> make the
>>>>>>>>>>>> test case succeed if "lsof" can not be found. I believe it
>>>>>>>>>>>> is the best
>>>>>>>>>>>> option (at least for now). I am sorry for all the noise I am
>>>>>>>>>>>> generating.
>>>>>>>>>>>>
>>>>>>>>>>>> Bug:
>>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8205054
>>>>>>>>>>>>
>>>>>>>>>>>> Webrev:
>>>>>>>>>>>> http://cr.openjdk.java.net/~lkorinth/8205054/00/
>>>>>>>>>>>>
>>>>>>>>>>>> Testing:
>>>>>>>>>>>> I have tested this on my machine, and I have a mach5 test
>>>>>>>>>>>> job running.
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> Leo
>>