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

[Mandy Chung]

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

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