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

[Stephen Colebourne]

On 20 January 2013 06:31, Patrick Zhang <patrick.zhang at oracle.com> wrote:
> Description:
> 1. Use minusYears(amount) to rewrite existing test about minus(amout,
> YEARS). plusYears(amout) has been covered and we do not touch it. And we do
> same thing to other TemporalUnit, for example, MONTHS, DAYS, HOURS, MINUTES,
> SECONDS and NANOS.
> Then below missed methods are covered:
> minusYears(amount)
> minusMonths(amount)
> minusDays(amount)
> minusHourss(amount)
> minusMinutess(amount)
> minusSeconds(amount)
> minusNanos(amount)

As a general rule, the test case method names for non-static methods should be:

public void test_methodName_arguments_whatMakesTheTestSpecial()

So,
 public void test_minus_Years
should be
 public void test_minus_longTemporalUnit_Years

(not all existing tests match this pattern)

The changes you've made do successfully test what needs testing,
however they would be more scalable as DataProvider tests. That way
one set of data could test both types of method. (DataProvider tests
are much more scalable in general, so feel free to refactor tests to
DataProvider in the future.)

Its up to you as to whether you make these recommended changes for
this webrev, or future ones.


> 2. Add one new data provider and related test case for toDateOnly() and
> toTimeOnly().

Looks good
Stephen