[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 7172176 java/jconsole test/sun/tools/jconsole/ImmutableResourceTest.sh failing
From:       Mandy Chung <mandy.chung () oracle ! com>
Date:       2013-08-29 20:43:01
Message-ID: 521FB255.1050605 () oracle ! com
[Download RAW message or body]

On 8/28/13 9:44 AM, Erik Gahlin wrote:
> Hi,
>
> It took sometime, but here is an updated webrev where the shell script 
> has been modified.
>
> http://cr.openjdk.java.net/~egahlin/7172176_3/

Looks good.  Happy for you to get this pushed as it has been sitting in 
your repo over a year :)

Mandy

>
> Changes:
>
> - added 'Darwin' to the shell script so it now also works on OS X.
> - updated copyright to 2013 ;)
> - changed from HashMap to IdentityHashMap, since same message can be 
> (and is) used by several keys
> - removed test from problem list
>
> Thanks
> Erik
>
> Erik Gahlin skrev 2012-06-08 21:59:
>> Mandy Chung skrev 2012-06-08 21:23:
>>> Erik,
>>>
>>>> Tested in JPRT with "-testset core -onlytests '.*jdk_tools2.*'" on 
>>>> all platforms, 
>>>
>>> Good.  It didn't realize the previous webrev was not verified with 
>>> JPRT.  It's always a good practice to run through shell test fix 
>>> with JPRT and sometimes you will find surprises :)
>>>
>> I ran it on the JPRT Stockholm queue and it doesn't have Macs (so I 
>> excluded that platform, it was after all a Java fix) and of course it 
>> was the only platform it failed on it.
>>
>> Murphy's law ;)
>>
>> Oh, well. I will update the shell script with "Darwin". If you are 
>> okay without a new webrev, please say so and we can both save 
>> ourselves some time :)
>>
>> Otherwise I will post a new webrev on monday
>>
>> Erik
>>
>>>> http://cr.openjdk.java.net/~egahlin/7172176_02/ 
>>>
>>> While I agree that a Java test is preferred to a shell test, I'd 
>>> prefer to keep this fix simple just to add Darwin just because I 
>>> don't have time to re-review this new fix.  There are many shell 
>>> tests in the JDK that have been fixed to add Darwin but this one was 
>>> not updated because it was on the problem list.
>>>
>>> FWIW.  With jigsaw / modules, jconsole.jar is going away and your 
>>> test will fail and require fixing again; on the other hand, java -cp 
>>> jconsole.jar will work in a modular JDK for compatibility reason.   
>>> That's what will be coming for you to consider as well.
>>>
>>> Mandy
>>>
>>> On 6/7/2012 4:47 PM, Erik Gahlin wrote:
>>>> Hi again,
>>>>
>>>> Could you please review an update?
>>>>
>>>> Turns out that the ResourceCheck test didn't work for Mac when I 
>>>> removed
>>>> it from the problems list. There was code in the shell script that 
>>>> needed
>>>> to know all the platform names.
>>>>
>>>> Instead of just adding "Darwin" to the script I decided to make the 
>>>> test pure
>>>> Java, to speed things up and to avoid similar platform problems in the
>>>> future. I couldn't find a way in JTREG to add a test-JDK relative 
>>>> library using
>>>> tags so I loaded the lib/jconsole.jar dynamically within the test.
>>>>
>>>> Changes from previous webrev:
>>>>
>>>> - classes will be loaded using a URLClassLoader 
>>>> (test.jdk/lib/jconsole.jar)
>>>>
>>>> - the resource bundle is loaded from the custom class loader
>>>>
>>>> - the method Resources#getMnemonicInt is now invoked by reflection. 
>>>> Not
>>>>   super clean, but better then having a shell script in my opinion.
>>>>
>>>> - removed failing tests from the ProblemsList.txt
>>>>
>>>> Tested in JPRT with "-testset core -onlytests '.*jdk_tools2.*'" on 
>>>> all platforms,
>>>>
>>>> Here is the webrev:
>>>> http://cr.openjdk.java.net/~egahlin/7172176_02/
>>>>
>>>> Thanks for taking the time to review this.
>>>>
>>>> Erik
>>>>
>>>> Erik Gahlin skrev 2012-05-30 13:40:
>>>>> Hi,
>>>>>
>>>>> Could you please review 7172176, which is a test fix for 7017818 -
>>>>> JConsoleResources.java cannot be handled by translation team.
>>>>>
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~egahlin/7172176_01/
>>>>>
>>>>> Changes:
>>>>>
>>>>> - Removed the ImmutableRequest.java and ImmutableRequest.sh since the
>>>>>   JConsoleResources class was refactored away with 7017818.
>>>>>
>>>>> - Removed hard-wired resource bundle keys in 
>>>>> ResourceCheckTest.java, the keys
>>>>>   are now looked up by reflection on the Messages class. The test 
>>>>> also
>>>>>   checks there is one-to-one mapping between reource bundle keys and
>>>>>   the constants available in the Message class.
>>>>>
>>>>> Michael,
>>>>>
>>>>> I removed sun/tools/jconsole/ResourceCheckTest.sh from the jdk/test/
>>>>> ProblemList.txt. This means the test will break once the 
>>>>> translated files
>>>>> will be checked in. Unused messages were sent for translation and 
>>>>> they
>>>>> need to be removed.
>>>>>
>>>>> Are you ok with that?
>>>>>
>>>>> Thanks!
>>>>> Erik
>>>>
>>
>

[prev in list] [next in list] [prev in thread] [next in thread]