[threeten-dev] Please help to review new added test cases for java.time.Chronology

[Stephen Colebourne]

Looks fine to me
Stephen

On 14 May 2013 05:23, Patrick Zhang <patrick.zhang at oracle.com> wrote:
> I have updated the repo and rerun the new test. Now Chronology.of("Hijrah")
> works well.
>
> The new added test cases have been separated into 5 methods as suggestion.
> Please help to review it.
>
>
> webrev:
> http://cr.openjdk.java.net/~pzhang/JSR310/java/time/chrono/Chronology/webrev/
> test result:
> http://cr.openjdk.java.net/~pzhang/JSR310/java/time/chrono/Chronology/TCKChronology.jtr
> http://cr.openjdk.java.net/~pzhang/JSR310/java/time/chrono/Chronology/TCKJapaneseChronology.jtr
>
> Regards
> Patrick
>
>
> On 5/9/13 11:21 PM, roger riggs wrote:
>>
>> Hi Stephen,
>>
>> I'll update the official name to be just "Hijrah"; currently that is an
>> alias
>> because there are variants.  The official names really should not be
>> ambiguous
>> and Hijrah is not specific about the variant.
>> But we did agree that the default/dominant calendar should be Umm AlQura.
>>
>> Roger
>>
>>
>> On 5/9/2013 5:06 AM, Stephen Colebourne wrote:
>>>
>>> TCKChronology
>>> test_dateNow/test_dateYearDay should really be 5 separate test cases
>>> each, as redefining variables in a test case is bad design.
>>>
>>> Sherman/Masayoshi/Roger,
>>> The calendar system display name for Hijrah is tested as
>>> "Hijrah-umalqura". This looks wrong next to the other names, and I
>>> suspect it might be a bug (not stripping the -umalqura suffix before
>>> lookup).
>>>
>>> Stephen
>>>
>>>
>>> On 9 May 2013 09:32, Patrick Zhang <patrick.zhang at oracle.com> wrote:
>>>>
>>>> Hi Team,
>>>>
>>>> Please help to review new added test cases for java.time.Chronology.
>>>> Impacted apis:
>>>> date(Era era, int yearOfEra, int month, int dayOfMonth)
>>>> dateYearDay(Era era, int yearOfEra, int dayOfYear)
>>>> dateNow()
>>>> dateNow(Clock clock)
>>>> dateNow(ZoneId zone)
>>>> getDisplayName()
>>>>
>>>> And in this webrev there is one fix for test code.
>>>> According to
>>>>
>>>> http://sqeweb.us.oracle.com/jsn/users/patrick/threeten/build/linux-x86-normal-server-release/docs/api/,
>>>> 1912-02-19 should belong to Meiji Era nor Taisho. So change the date to
>>>> 1916-02-19 which is in Taisho.
>>>>
>>>> webrev:
>>>>
>>>> http://cr.openjdk.java.net/~pzhang/JSR310/java/time/chrono/Chronology/webrev/
>>>> test result:
>>>>
>>>> http://cr.openjdk.java.net/~pzhang/JSR310/java/time/chrono/Chronology/TCKChronology.jtr
>>>>
>>>> Regards
>>>> Patrick
>>>>
>>
>