RFR(S) : 8180386: remove jdk.testlibrary.TimeLimitedRunner
Roger Riggs
Roger.Riggs at Oracle.com
Wed Jun 7 15:31:56 UTC 2017
Hi Igor,
Removing trailing spaces is fine.
Thanks, Roger
On 6/7/2017 11:29 AM, Igor Ignatyev wrote:
> Hi Roger,
>
> please see my comments inline.
>
> -- Igor
>
>> On Jun 7, 2017, at 7:48 AM, Roger Riggs <Roger.Riggs at Oracle.com> wrote:
>>
>> Hi Igor,
>>
>> It is odd that test/TEST.ROOT shows up in the webrev with no changes.
> it seems there were trailing spaces at line#13[1] which my vi removed on closing. if you want, I can revert that before pushing.
>
> [1]
> -# run. Tests that are not headful are "headless."
> +# run. Tests that are not headful are "headless."
>
>
>> Seeing @library with both /lib/testlibrary and /test/lib makes me wonder when that will get resolved?
> this will be resolved that the rest of testlibrary from /lib/testlibrary will be merged w/ library in /test/lib by JDK-8075327[2]. I expect at least 4 other sub-tasks.
>
> [2] https://bugs.openjdk.java.net/browse/JDK-8075327
>> Otherwise, looks fine
> thank you for your review.
>
>> Thanks, Roger
>>
>> On 6/7/2017 2:44 AM, Igor Ignatyev wrote:
>>> still looking for a Reviewer.
>>>
>>> -- Igor
>>>> On May 31, 2017, at 2:57 PM, Igor Ignatyev <igor.ignatyev at oracle.com> wrote:
>>>>
>>>> Hi Mandy,
>>>>
>>>> 8180793[1], which moved j.t.l.wrappers.* to j.t.lib package, has been propagated to jdk10/jdk10. I have updated the patch accordingly -- http://cr.openjdk.java.net/~iignatyev//8180386/webrev.01/index.html
>>>> could you please take a look?
>>>>
>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8180793
>>>>
>>>> Thanks,
>>>> -- Igor
>>>>
>>>>> On May 18, 2017, at 2:49 PM, Igor Ignatyev <igor.ignatyev at oracle.com> wrote:
>>>>>
>>>>>> On May 18, 2017, at 2:41 PM, Mandy Chung <mandy.chung at oracle.com> wrote:
>>>>>>
>>>>>>
>>>>>>> On May 18, 2017, at 2:37 PM, Igor Ignatyev <igor.ignatyev at oracle.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>>> On May 18, 2017, at 2:34 PM, Mandy Chung <mandy.chung at oracle.com> wrote:
>>>>>>>> This comment is not related to the change. The package name `jdk.test.lib.wrappers` is odd. TimeLimitedRunner and InfiniteLoop classes could simply be in the jdk.test.lib package or jdk.test.lib.util package.
>>>>>>>>
>>>>>>>> Mandy
>>>>>>>>
>>>>>>> agree, I'll file an RFE to find a better package for TimeLimitedRunner and InfiniteLoop classes.
>>>>>> Why not doing the rename with this patch?
>>>>> there are hotspot tests which use jdk.test.lib.wrappers.TimeLimitedRunner and InfiniteLoop, so it will require changes in hotspot repo. I'd prefer to have removing classes from jdk testlibrary and renaming classes in top level testlibrary as separate patches for clarity purposes.
>>>>>
>>>>> -- Igor
More information about the core-libs-dev
mailing list