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

[Igor Ignatyev]

+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.

-- Igor

> On Mar 18, 2020, at 9:54 AM, Alexander Scherbatiy <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> 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> 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/serviceability-dev/attachments/20200318/3a35bf05/attachment.htm>