[threeten-dev] Hijrah Tests Review Request

[Stephen Colebourne]

The TCK test involves a lot of deletion. Was that deliberate?

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