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

Igor Ignatyev igor.ignatyev at oracle.com
Wed Mar 18 16:00:49 UTC 2020


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.

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



More information about the serviceability-dev mailing list