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-
>>>>>>>> bounces at openjdk.java.net] On Behalf Of Leo Korinth
>>>>>>>> Sent: Donnerstag, 14. Juni 2018 18:55
>>>>>>>> To: Hotspot dev runtime <hotspot-runtime-dev at openjdk.java.net>
>>>>>>>> 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