couple of minor nits I did not see in rev 01.
Copyright,

-        if("C".equals(LC_ALL) || "C".equals(LANG)) {
+        if ("C".equals(LC_ALL) || "C".equals(LANG)) {

Kumar


Looks good. Thank you for fixing this.

Naoto

On 2/25/13 10:53 AM, Brent Christian wrote:
On 2/22/13 10:22 AM, Naoto Sato wrote:
check not only "C" but also other locales that do not use UTF-8
encoding.

I would like to make the test more robust (per 8008761), though my focus
at the moment is to avoid a test failure on our automated test systems,
which all use "C".

I've made a note in 8008761 that we should check for other locales that
do not use UTF-8.

 > Also, "LC_ALL" precedes "LANG" environment variable, so I'd
check "LC_ALL" first, and if it is not exported then check "LANG"
variable.

Good point - I'll check LC_ALL first.

Updated webrev with re-ordered LC_ALL check (and "LC_ALL=" output
change) is here:
http://cr.openjdk.java.net/~bchristi/8006039/webrev.01/

Thanks,
-Brent

On 2/21/13 4:34 PM, Brent Christian wrote:
Hi,

This test started failing after 8003228 [1] was putback (setting
sun.jnu.encoding to UTF-8 on Mac). It tests if java is able to launch a
.jar stored in a directory named with two-byte characters.

The code comments in the test case state that (on Unix) it expects
LC_ALL to be set (to ja_JP.UTF-8/ja_JP.utf8/ja_JP.ujis), though it also
seems to work with en_US.UTF-8).

Our automated build+test Macs have LANG=C, so the test has been
"passing" without actually testing the functionality.

The test case determines if the environment is suitable for testing by
checking if sun.jnu.encoding is either "MS932" or "UTF-8" (on Mac, this
is now always UTF-8).  The test doesn't actually check LC_ALL.

I think the test should also check the LANG & LC_ALL env vars, and go
back to "fake passing" the test if either is set to 'C'. This would
allow the test to continue to run "for real" in the default Mac
environment (LANG=en_US.UTF-8), and stop the test from failing on the
build+test Macs.

Of course it would be even better to update the test so the automated
test machines actually test the intended functionality.  On UNIX at
least, perhaps it could find something suitable to set LC_ALL to before
attempting to launch the .jar.  I can file a separate bug for this.

Webrev is here:
http://cr.openjdk.java.net/~bchristi/8006039/webrev.00/

Thanks,
-Brent

[1]
http://bugs.sun.com/view_bug.do?bug_id=8003228



Reply via email to