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

[Stephen Colebourne]

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
>