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

[Alexander Scherbatiy]

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.