Please review: 7127906: (launcher) convert the launcher regression tests to java

Naoto Sato naoto.sato at oracle.com
Fri Jan 27 23:53:05 UTC 2012


Looks good to me. Thanks.

Naoto

On 1/27/12 3:31 PM, Kumar Srinivasan wrote:
> Hi Naoto,
>
> Here is the incremental webrev since last revision:
> http://cr.openjdk.java.net/~ksrini/7127906/webrev.1/webrev.delta/index.html
>
> Here is the full webrev for reference:
> http://cr.openjdk.java.net/~ksrini/7127906/webrev.1/index.html
>
>
> Thanks
> Kumar
>
>> On 1/27/12 11:36 AM, Kumar Srinivasan wrote:
>>> On 1/27/2012 11:05 AM, Naoto Sato wrote:
>>>> Hi Kumar,
>>>>
>>>> I have a question/suggestion on TestHelper.isEnglishLocale field.
>>>>
>>>> - Is there any chance that the test case changes the default locale by
>>>> Locale.setDefault() during the test execution? If that's the case,
>>>> static initialization of the field may not work.
>>>
>>> Yes, the tests that do change the default locale are UnicodeTest.java
>>> and
>>> I18NJarTest.java, however they execute in their own child Process/VM.
>>>
>>
>> Then I'd prefer to have isEnglishLocale as a simple static method,
>> rather than a field.
>>
>>> I think I should also the save default to a static, such that we can
>>> revert back the locale to the original default.
>>
>> Good point.
>>
>>>
>>>>
>>>> - If the default locale does not change during the test execution,
>>>> then I'd suggest to initialize it with something like:
>>>>
>>>> isEnglishLocale = Locale.getDefault().getLanguage().equals("en");
>>>
>>> What do you suggest ?
>>
>> To replace the current isEnglishLocale logic with the above, which
>> should work for all English language locales.
>>
>> Naoto
>>
>>>
>>> Thanks
>>> Kumar
>>>
>>>>
>>>> Otherwise it won't work with other English locales such as en_GB.
>>>>
>>>> Other than that, it looks good to me.
>>>>
>>>> Naoto
>>>>
>>>> On 1/27/12 10:30 AM, Joe Darcy wrote:
>>>>> Hi Kumar,
>>>>>
>>>>> Looks fine; good to see the shell code go away!
>>>>>
>>>>> -Joe
>>>>>
>>>>> On 01/27/2012 10:03 AM, Kumar Srinivasan wrote:
>>>>>> Hi Joe, Naoto, Sherman,
>>>>>>
>>>>>> Here are the highlights of these changes:
>>>>>>
>>>>>> 1. The TestHelper.java is now extensible, this simplifies the
>>>>>> usage of
>>>>>> static variables
>>>>>> and methods, added a couple of convenience methods and some minor
>>>>>> refactoring.
>>>>>>
>>>>>> 2. Eliminated some shell tests, (few more remaining, saved for a
>>>>>> future project).
>>>>>> a. UnicodeTest.sh, UnicodeCleanup.java merged into UnicodeTest.java
>>>>>> b. i18nTest.sh, deleteI18n.sh, CreatePlatformFile.java merged into
>>>>>> I18NTest.java
>>>>>> c. ChangeDataModel.sh converted to ChangeDataModel.java
>>>>>> d. unresolvedExceptions.sh converted and merged into
>>>>>> UnresolvedExceptions.java
>>>>>>
>>>>>> 3. Arrrghs.java when testing on Solaris with ja_JP.UTF-8 locale, some
>>>>>> of the tests fail
>>>>>> as the launcher messages in java are localized, therefore these tests
>>>>>> are skipped,
>>>>>> under such conditions.
>>>>>>
>>>>>> Tests: Windows English/Japanese, Solaris C/ja_JP.UTF-8 + usual jprt
>>>>>> systems (undergoing).
>>>>>>
>>>>>> webrev: http://cr.openjdk.java.net/~ksrini/7127906/webrev.0/
>>>>>>
>>>>>> Thanks
>>>>>> Kumar
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>




More information about the core-libs-dev mailing list