Re: RFR: 8006039 : test/tools/launcher/I18NJarTest.java fails on Mac w/ LANG=C, LC_ALL=C
Hi Brent, Ok I tested on Windows with JA installed and your patch works, also tested on Solaris. On other nit, Copyright on I18NJarTest.java needs updating. Looks good, go for it. Thanks Kumar On 2/21/13 7:33 PM, Kumar Srinivasan wrote: A nit - LC_ALL + LC_ALL + \n + + LC_ALL= + LC_ALL + \n + Done, thanks. If Mac is the only system affected by this, should we make the checks for LC* and LANG specific to Macs since other platforms don't have this issue ? I am concerned this might pass vacuously on systems configured correctly. LANG/LC_ALL=C is not a correct configuration for running this test. It ought to pass vacuously in this case (and does, AFAIK, on other platforms). What is Mac-specific is that sun.jnu.encoding is now always set to UTF-8, even under LANG/LC_ALL=C. The test sees this, and tries (and fails) to run the real test instead of passing vacuously. I don't think it does any harm to check for LANG/LC_ALL != 'C' on other platforms. But it's only on Mac that it's required in order to detect a mis-configured test environment. Of course it would be even better to update the test so the automated test machines actually test the intended functionality. ... I can file a separate bug for this. I've filed 8008761 (test/tools/launcher/I18NJarTest.java should automatically setup its environment) to cover this. To be clear, my short-term goal is to take I18NJarTest back off the ProblemList. The test does correctly test the intended functionality under the default Mac environment (LANG=en_US.UTF-8). So I want to get it running again for engineers using the jdk_tools test target on their local Mac (without causing nightly test failure noise). Later, as time permits, I can tackle 8008761, and get the test to configure its own environment. Thanks, -Brent
Re: RFR: 8006039 : test/tools/launcher/I18NJarTest.java fails on Mac w/ LANG=C, LC_ALL=C
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
Re: RFR: 8006039 : test/tools/launcher/I18NJarTest.java fails on Mac w/ LANG=C, LC_ALL=C
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
Re: RFR: 8006039 : test/tools/launcher/I18NJarTest.java fails on Mac w/ LANG=C, LC_ALL=C
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
Re: RFR: 8006039 : test/tools/launcher/I18NJarTest.java fails on Mac w/ LANG=C, LC_ALL=C
Thanks for the review, guys.
Think I've got it all in:
http://cr.openjdk.java.net/~bchristi/8006039/webrev.02/
For the Copyright, do I just need to change the year(s) to 2012, 2013,?
Also, thanks a lot for running the Windows testing, Kumar.
Can someone push this for me? I can send a patch (or bundle) file...
Thanks,
-Brent
On 2/25/13 11:09 AM, Kumar Srinivasan wrote:
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
Re: RFR: 8006039 : test/tools/launcher/I18NJarTest.java fails on Mac w/ LANG=C, LC_ALL=C
I can take care of it.
Kumar
Thanks for the review, guys.
Think I've got it all in:
http://cr.openjdk.java.net/~bchristi/8006039/webrev.02/
For the Copyright, do I just need to change the year(s) to 2012, 2013,?
Also, thanks a lot for running the Windows testing, Kumar.
Can someone push this for me? I can send a patch (or bundle) file...
Thanks,
-Brent
On 2/25/13 11:09 AM, Kumar Srinivasan wrote:
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
Re: RFR: 8006039 : test/tools/launcher/I18NJarTest.java fails on Mac w/ LANG=C, LC_ALL=C
Hi Brent, I think the condition needs to check if it is on Unix environment, and check not only C but also other locales that do not use UTF-8 encoding. Also, LC_ALL precedes LANG environment variable, so I'd check LC_ALL first, and if it is not exported then check LANG variable. Naoto 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
Re: RFR: 8006039 : test/tools/launcher/I18NJarTest.java fails on Mac w/ LANG=C, LC_ALL=C
On 2/21/13 7:33 PM, Kumar Srinivasan wrote: A nit - LC_ALL + LC_ALL + \n + + LC_ALL= + LC_ALL + \n + Done, thanks. If Mac is the only system affected by this, should we make the checks for LC* and LANG specific to Macs since other platforms don't have this issue ? I am concerned this might pass vacuously on systems configured correctly. LANG/LC_ALL=C is not a correct configuration for running this test. It ought to pass vacuously in this case (and does, AFAIK, on other platforms). What is Mac-specific is that sun.jnu.encoding is now always set to UTF-8, even under LANG/LC_ALL=C. The test sees this, and tries (and fails) to run the real test instead of passing vacuously. I don't think it does any harm to check for LANG/LC_ALL != 'C' on other platforms. But it's only on Mac that it's required in order to detect a mis-configured test environment. Of course it would be even better to update the test so the automated test machines actually test the intended functionality. ... I can file a separate bug for this. I've filed 8008761 (test/tools/launcher/I18NJarTest.java should automatically setup its environment) to cover this. To be clear, my short-term goal is to take I18NJarTest back off the ProblemList. The test does correctly test the intended functionality under the default Mac environment (LANG=en_US.UTF-8). So I want to get it running again for engineers using the jdk_tools test target on their local Mac (without causing nightly test failure noise). Later, as time permits, I can tackle 8008761, and get the test to configure its own environment. Thanks, -Brent
Re: RFR: 8006039 : test/tools/launcher/I18NJarTest.java fails on Mac w/ LANG=C, LC_ALL=C
Hi Brent,
I am not an expert here, and I am hoping someone from I18N
also reviews this.
A nit
- LC_ALL + LC_ALL + \n +
+ LC_ALL= + LC_ALL + \n +
If Mac is the only system affected by this, should we make the checks for
LC* and LANG specific to Macs since other platforms don't have this issue ?
I am concerned this might pass vacuously on systems configured correctly.
Since the test extends TestHelper all you have to do is:
-if(C.equals(LANG) || C.equals(LC_ALL)) {
+if (isMacOSX (C.equals(LANG) || C.equals(LC_ALL))) {
Kumar
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
