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

[patrick zhang]

Hi Stephen,
Thanks for your suggestion. I will take care of it in future webrev. 
More dataprovider pattern will be introduced in future test cases.

Hi Sherman,
Could you help to push it?
http://cr.openjdk.java.net/~sherman/jdk8_threeten/patrick/period/webrev/
 
Regards
Patrick

On 2013-1-21 7:46, Stephen Colebourne wrote:
> 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
>