RFR: 7172176 java/jconsole test/sun/tools/jconsole/ImmutableResourceTest.sh failing

[Erik Gahlin]

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