[threeten-dev] Hijrah Tests Review Request

[roger riggs]

On 6/19/2013 5:15 AM, Stephen Colebourne wrote:
> The TCK test involves a lot of deletion. Was that deliberate?
Since they had calendar data dependencies, they are not valid as TCK tests.
I asked that they be moved to the UmmAlQura tests.

Roger

>
> The TestUmmAlQuraChronology class (non TCK) has date creation
> indirectly via lines this this:
> {1318, 01, 01,   1900, 04, 30},
> but the "01" and "04" are octal numbers, not "normal" constants. Octal
> numbers may look pretty for dates, but they don't work for "08" and
> "09" and thus confuse users browsing this code (looking for example
> API usage).
>
> The comment sections (grouping similar tests) are incomplete. For
> example. you could do with a dashed line or comment header at line 655
> as the tests below there don't refer to the previous comment section
> header at line 623. Alternately, remove all the comment sections.
>
> The tests themselves look pretty comprehensive, although I haven't
> checked the actual dates or logic are correct.
>
> Stephen
>
>
> On 18 June 2013 21:45, Chand Basha <chand.basha at oracle.com> wrote:
>> Hi,
>>
>> I have added few new tests to TestUmmAlQuraChronology.java and
>> removed/cleaned up existing TCKHijrahChronology.java files. Roger graciously
>> created a review for me
>> http://cr.openjdk.java.net/~rriggs/webrev-hijrah-calendar-tests-8016898.
>> Thanks a lot Roger.
>>
>> I would appreciate if you could review it and let me know if there are any
>> questions/concerns to commit the files.
>>
>> Thanks,
>> - Basha