jmx-dev RFR 8240604: Rewrite sun/management/jmxremote/bootstrap/CustomLauncherTest.java test to make binaries from source file

[Alexander Scherbatiy]

On 18.03.2020 20:02, Igor Ignatyev wrote:

> +import static jdk.test.lib.Utils.TEST_CLASS_PATH;
> I'm not a huge fun of 'import static', yet don't insist on removing it 
> either.
>
> + System.out.println(" libjvm : " + jvmLibDir.toString());
> jvmLibDir doesn't point to libjvm, so you need either update message 
> prefix or use the actual value which will be used as path to libjvm. I 
> personally prefer the latter.
> btw, you don't need to explicitly call toString in string concatenation.
>
   Here is the updated fix where the static import is removed and libjvm 
path is used:

   http://cr.openjdk.java.net/~alexsch/8240604/webrev.03/


   Thanks,

   Alexander.


> -- Igor
>
>> On Mar 18, 2020, at 9:54 AM, Alexander Scherbatiy 
>> <alexander.scherbatiy at bell-sw.com 
>> <mailto:alexander.scherbatiy at bell-sw.com>> wrote:
>>
>> On 18.03.2020 19:00, Igor Ignatyev wrote:
>>
>>> Hi Alexander,
>>>
>>>> I also included TEST_NATIVE_PATH to the Utils lib.
>>> for the sake of clarity and ease of backporting, I'd prefer to have 
>>> it added by a separate bug and commit.
>>
>> Here is the updated fix where TEST_NATIVE_PATH is not added to the 
>> Utils lib.
>>
>> http://cr.openjdk.java.net/~alexsch/8240604/webrev.02/
>>
>>
>> Thanks,
>>
>> Alexander.
>>
>>>> Could I just use "hg remove binary-fie" and run webrev to add the 
>>>> removed binary files into webrev?
>>> IIRC correctly, webrev will just say 'a binary file got removed', in 
>>> any case I'll take it as a 'yes, I'm going to remove these files as 
>>> part of 8240604', so thumbs up.
>>>
>>> -- Igor
>>>
>>>> On Mar 18, 2020, at 4:57 AM, Alexander Scherbatiy 
>>>> <alexander.scherbatiy at bell-sw.com 
>>>> <mailto:alexander.scherbatiy at bell-sw.com>> wrote:
>>>>
>>>> Hello,
>>>>
>>>> Could you review the updated fix:
>>>>
>>>> http://cr.openjdk.java.net/~alexsch/8240604/webrev.01
>>>>
>>>> Utils.TEST_CLASS_PATH, Platform.jvmLibDir(), and /native flag are 
>>>> added to the CustomLauncherTest.java test. I also included 
>>>> TEST_NATIVE_PATH to the Utils lib.
>>>>
>>>> I have not found a history about CustomLauncherTest.sh script in 
>>>> launcher.c so I just updated the comment as "A minature launcher 
>>>> for use by CustomLauncherTest.java test" in the exelauncher.c file.
>>>>
>>>>
>>>> The comment that I had about removing the linux-* and solaris-* 
>>>> binary files I wrote because it is not clear for what is the right 
>>>> way to include removed binary files into webrev.
>>>>
>>>> Could I just use "hg remove binary-fie" and run webrev to add the 
>>>> removed binary files into webrev?
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Alexander.
>>>>
>>>> On 17.03.2020 20:11, Igor Ignatyev wrote:
>>>>> Hi Alexander,
>>>>>
>>>>> overall looks good to me, I have a few comments though:
>>>>>  - you can use Utils.TEST_CLASSPATH instead of 
>>>>> CustomLauncherTest.TEST_CLASSPATH
>>>>> - CustomLauncherTest::findLibjvm can be simplified by use 
>>>>> Platform::jvmLibDir
>>>>> - exelauncher.c has a comment which refers to the test as 
>>>>> CustomLauncherTest.sh, could you please update the comment?
>>>>> - you have to add /native flag to @run action, otherwise jtreg 
>>>>> won't exclude this test from runs w/ test.nativepath being unset
>>>>>
>>>>> I also have a question regarding your statement that
>>>>>>> The changes for obsolete binary files <...> are not included 
>>>>>>> into the webrev. They needs to be removed manually.
>>>>> you are planning to remove these files as part of this patch, right?
>>>>>
>>>>> Thanks,
>>>>> -- Igor
>>>>>
>>>>>
>>>>>> On Mar 5, 2020, at 6:27 AM, Daniel Fuchs <daniel.fuchs at oracle.com 
>>>>>> <mailto:daniel.fuchs at oracle.com>> wrote:
>>>>>>
>>>>>> Hi Alexander,
>>>>>>
>>>>>> Fixes to JMX & management agent are reviewed on the
>>>>>> seviceability-dev (added in to:) these days.
>>>>>>
>>>>>> best regards,
>>>>>>
>>>>>> -- daniel
>>>>>>
>>>>>> On 05/03/2020 13:17, Alexander Scherbatiy wrote:
>>>>>>> Hello,
>>>>>>> Could you review a small enhancement where the test 
>>>>>>> CustomLauncherTest is updated to build binary launcher file from 
>>>>>>> launcher.c file.
>>>>>>> The file launcher.c is renamed to exelauncher.c to follow the 
>>>>>>> name convention for executable test files building by jdk make 
>>>>>>> system.
>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8240604
>>>>>>> Webrev: http://cr.openjdk.java.net/~alexsch/8240604/webrev.00
>>>>>>> The changes for obsolete binary files from 
>>>>>>> sun/management/jmxremote/bootstrap/linux-* and solaris-* are not 
>>>>>>> included into the webrev. They needs to be removed manually.
>>>>>>> The test is passed on Ubuntu 18.04 x86-64, Solaris Sparc v9 
>>>>>>> 11.2, and Solaris x64 11.4 systems.
>>>>>>> The test is excluded from Windows and Mac Os X systems.
>>>>>>> Thanks,
>>>>>>> Alexander.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/jmx-dev/attachments/20200318/366a26b2/attachment.htm>