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
