[threeten-dev] Please help to review new test code for java.time.temporal.TemporalAdjuster.adjustInto()

[Stephen Colebourne]

Looks good to me. Is Sherman pushing for you?
Stephen

On 25 February 2013 02:56, Patrick Zhang <patrick.zhang at oracle.com> wrote:
> Hi Stephen,
>
> Do you think the updated webrev is ok?
>
> Regards
> Patrick
>
>
> On 2/19/13 8:57 PM, Patrick Zhang wrote:
>>
>> Hi Stephen,
>>
>> webrev and test result have been updated.
>>
>>
>> webrev:
>> http://cr.openjdk.java.net/~pzhang/JSR310/java/time/adjustInto/webrev/
>>
>> test result:
>>
>> http://cr.openjdk.java.net/~pzhang/JSR310/java/time/adjustInto/TCKLocalDateTime.jtr
>>
>> http://cr.openjdk.java.net/~pzhang/JSR310/java/time/adjustInto/TCKLocalTime.jtr
>>
>> http://cr.openjdk.java.net/~pzhang/JSR310/java/time/adjustInto/TCKOffsetDateTime.jtr
>>
>> http://cr.openjdk.java.net/~pzhang/JSR310/java/time/adjustInto/TCKOffsetTime.jtr
>>
>> Regards
>> Patrick
>>
>> On 2/19/13 6:45 PM, Stephen Colebourne wrote:
>>>
>>> Overall, looks good.
>>>
>>> Code like this:
>>>   OffsetDateTime.of(LocalDateTime.of(2012, 3, 4, 23, 5), ZoneOffset.UTC)
>>> can be replaced by
>>>   OffsetDateTime.of(2012, 3, 4, 23, 5, 0, ZoneOffset.UTC)
>>>
>>> Code like this:
>>>   ZonedDateTime.of(LocalDateTime.of(2012, 3, 4, 23, 5), ZONE_PARIS)
>>> can be replaced by
>>>   ZonedDateTime.of(2012, 3, 4, 23, 5, 0, ZONE_PARIS)
>>>
>>> Code like this:
>>>   OffsetTime.of(LocalTime.of(22, 3, 0), ZoneOffset.UTC)
>>> can be replaced by
>>>   OffsetTime.of(22, 3, 0, 0, ZoneOffset.UTC)
>>>
>>> Stephen
>>>
>>>
>>> On 19 February 2013 09:20, Patrick Zhang<patrick.zhang at oracle.com>
>>> wrote:
>>>>
>>>> Hi Team,
>>>>
>>>> The new added test code is used to checking compabality of adjustInto()
>>>> for below 5 classes related with date-time, which have implement
>>>> java.time.temporal.TemporalAdjuster interface.
>>>>
>>>> java.time.LocalDate
>>>> java.time.LocalDateTime
>>>> java.time.LocalTime
>>>> java.time.OffsetDateTime
>>>> java.time.OffsetTime
>>>>
>>>> And we have another general data-time object, java.time.ZonedDateTime,
>>>> which do NOT implement TemporalAdjuster interface.
>>>> But we still put it in checking list since it supoorts most of data-time
>>>> field.
>>>>
>>>>  From the description for A.adjustInto(B), it just invokes
>>>> A.with(temporalField, value) multi times to copy some temporalField
>>>> value in
>>>> A to overwrite same temporalField in B. Then A.adjustInto(B) should
>>>> always
>>>> works well if B contains all temporalField of A.
>>>>
>>>> Since we can get all supported  temporalField list from description of
>>>> isSupported() method, we can conclude below "adjust" matrix easily.
>>>>
>>>> A.adjustInto(B)
>>>> B: LocalDate B: LocalDateTime B: LocalTime B: OffsetDateTime B:
>>>> OffsetTime
>>>> B: ZonedDateTime
>>>> A: LocalDate Y
>>>> Y
>>>> NA
>>>> Y
>>>> NA
>>>> Y
>>>> A: LocalDateTime NA
>>>> Y
>>>> NA
>>>> Y
>>>> NA
>>>> Y
>>>> A: LocalTime NA
>>>> Y
>>>> Y
>>>> Y
>>>> Y
>>>> Y
>>>> A: OffsetDateTime NA
>>>> NA
>>>> NA
>>>> Y
>>>> NA
>>>> Y
>>>> A: OffsetTime NA
>>>> NA
>>>> NA
>>>> Y
>>>> Y
>>>> Y
>>>>
>>>>
>>>> Above logic has been implemented in below 4 test classes. I do not touch
>>>> TCKLocalDate.java since it has included similar logic.
>>>>
>>>> test/java/time/tck/java/time/TCKLocalDateTime.java
>>>> test/java/time/tck/java/time/TCKLocalTime.java
>>>> test/java/time/tck/java/time/TCKOffsetDateTime.java
>>>> test/java/time/tck/java/time/TCKOffsetTime.java
>>>>
>>>>
>>>> For more detail, please check webrev:
>>>> http://cr.openjdk.java.net/~pzhang/JSR310/java/time/adjustInto/webrev/
>>>>
>>>> test result:
>>>>
>>>>
>>>> http://cr.openjdk.java.net/~pzhang/JSR310/java/time/adjustInto/TCKLocalDateTime.jtr
>>>>
>>>>
>>>> http://cr.openjdk.java.net/~pzhang/JSR310/java/time/adjustInto/TCKLocalTime.jtr
>>>>
>>>>
>>>> http://cr.openjdk.java.net/~pzhang/JSR310/java/time/adjustInto/TCKOffsetDateTime.jtr
>>>>
>>>>
>>>> http://cr.openjdk.java.net/~pzhang/JSR310/java/time/adjustInto/TCKOffsetTime.jtr
>>>>
>>>> Regards
>>>> Patrick
>>>>
>