[threeten-dev] Please help to review new test code for java.time.calendars.MinguoChronology

[patrick zhang]

Thanks. I will separate it into 3 different test methods in future webrevs.

Regards
Patrick

On 2013-1-30 18:47, Stephen Colebourne wrote:
> I've fixed the Javadoc error
>
> In general you shouldn't put too much in one test. For example
>  @Test(dataProvider="prolepticYear", groups={"tck"})
>  public void test_prolepticYear(int eraValue, Era<MinguoChronology>
> era, int yearOfEra, int expectedProlepticYear, boolean isLeapYear) {
>      Era<MinguoChronology> eraObj = MinguoChronology.INSTANCE.eraOf(eraValue) ;
>      assertTrue(MinguoChronology.INSTANCE.eras().contains(eraObj));
>      assertEquals(eraObj, era);
>      assertEquals(MinguoChronology.INSTANCE.prolepticYear(era,
> yearOfEra), expectedProlepticYear);
>      assertEquals(MinguoChronology.INSTANCE.isLeapYear(expectedProlepticYear),
> isLeapYear) ;
>      assertEquals(MinguoChronology.INSTANCE.isLeapYear(expectedProlepticYear),
> Year.of(expectedProlepticYear + YDIFF).isLeap()) ;
>  }
>
> If testing at least three different things - the validity of the era,
> the proleptic year method and the leap year method.
>
> I would recommend in future testing all three separately. The data
> provider can be re-used in multiple tests if necessary.
>
> thanks
> Stephen
>
>
> On 30 January 2013 09:35, patrick zhang <patrick.zhang at oracle.com> wrote:
>   
>> The text of webrev/result link looks good. But it seems it points to
>> incorrect url. Please copy it to browser then open it. Or click below
>> updated link directly.
>>
>> webrev:
>>
>> http://cr.openjdk.java.net/~pzhang/JSR310/java/time/calendars/MinguoChronology/
>>
>> test result:
>> http://cr.openjdk.java.net/~pzhang/JSR310/java/time/calendars/MinguoChronology/TestMinguoChronology.jtr
>>
>> Regards
>> Patrick
>>
>> On 2013-1-30 15:12, Patrick Zhang wrote:
>>
>>
>> Hi Team,
>>
>> Please help to review below new added test case:
>> http://cr.openjdk.java.net/~pzhang/JSR310/java/time/calendars/MinguoChronology/
>>
>> test result:
>> http://cr.openjdk.java.net/~pzhang/JSR310/java/time/calendars/MinguoChronology/TestMinguoChronology.jtr
>>
>> Description:
>> The new added test cases are used for prolepticYear(), datYearDay() and
>> isLeapYear().
>> 1. As positive test case, some leapYear/02/29 are verified with according
>> ISO date. And we use both dateYearDay(year, doy) and date(era, year, month,
>> dom) to construct same ISO date to confirm it works well.
>> As negative test case, some non-leapYear/02/29 are added in "badDates" data
>> provider and it should throw DateTimeException.
>>
>> 2. According to description of javadoc, we can confirm below formula should
>> work:
>>     prolepticYear(ERA_ROC, a) = a
>>     prolepticYear(ERA_BEFORE_ROC, a) = 1-a
>>     prolepticYear(ERA_ROC, a) = "ISO  prolepticYear" - 1911
>>
>> And we can confirm isLeapYear(prolepticYear) should return same value with
>> Year.of(prolepticYear + 1911).isLeap() safely.
>>
>> Above logic has been verified in test_prolepticYear().
>>
>> 3. It looks there is one minor typo in javadoc of MinguoChronology:
>>     " Dates are aligned such that 0001-01-01 (Minguo) is 1911-01-01 (ISO). "
>>     Actually it should be 1912-01-01(ISO), all existing test, and my new
>> added test, obey it. It is only one error in javadoc.
>>
>> Regards
>> Patrick
>>
>>