Quantcast

RFR(S) : 8180386: remove jdk.testlibrary.TimeLimitedRunner

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

RFR(S) : 8180386: remove jdk.testlibrary.TimeLimitedRunner

Igor Ignatyev
http://cr.openjdk.java.net/~iignatyev//8180386/webrev.00/index.html
>  lines changed: 2 ins; 88 del; 6 mod;

Hi all,

could you please review this small fix which removes TimeLimitedRunner class from jdk testlibrary? we have two TimeLimitedRunner classes one in jdk test library and another in the the toplevel test library, there is no difference b/w these two library classes (besides location and package name), so the tests were simply updated to use TimeLimitedRunner from the toplevel test library.

this fix is a part of ongoing effort on merging and cleaning up our test libraries[1].

webrev: http://cr.openjdk.java.net/~iignatyev//8180386/webrev.00/index.html
JBS: https://bugs.openjdk.java.net/browse/JDK-8180386
testing: affected tests (java/lang/invoke)

[1] https://bugs.openjdk.java.net/browse/JDK-8075327

Thanks,
-- Igor
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(S) : 8180386: remove jdk.testlibrary.TimeLimitedRunner

Igor Ignatyev
Hi,

still looking for a Reviewer.

Cheers,
-- Igor

> On May 15, 2017, at 2:00 PM, Igor Ignatyev <[hidden email]> wrote:
>
> http://cr.openjdk.java.net/~iignatyev//8180386/webrev.00/index.html
>> lines changed: 2 ins; 88 del; 6 mod;
>
> Hi all,
>
> could you please review this small fix which removes TimeLimitedRunner class from jdk testlibrary? we have two TimeLimitedRunner classes one in jdk test library and another in the the toplevel test library, there is no difference b/w these two library classes (besides location and package name), so the tests were simply updated to use TimeLimitedRunner from the toplevel test library.
>
> this fix is a part of ongoing effort on merging and cleaning up our test libraries[1].
>
> webrev: http://cr.openjdk.java.net/~iignatyev//8180386/webrev.00/index.html
> JBS: https://bugs.openjdk.java.net/browse/JDK-8180386
> testing: affected tests (java/lang/invoke)
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8075327
>
> Thanks,
> -- Igor

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(S) : 8180386: remove jdk.testlibrary.TimeLimitedRunner

Mandy Chung
In reply to this post by Igor Ignatyev

> On May 15, 2017, at 2:00 PM, Igor Ignatyev <[hidden email]> wrote:
>
> http://cr.openjdk.java.net/~iignatyev//8180386/webrev.00/index.html
>> lines changed: 2 ins; 88 del; 6 mod;
>
> Hi all,
>
> could you please review this small fix which removes TimeLimitedRunner class from jdk testlibrary? we have two TimeLimitedRunner classes one in jdk test library and another in the the toplevel test library, there is no difference b/w these two library classes (besides location and package name), so the tests were simply updated to use TimeLimitedRunner from the toplevel test library.
>
> this fix is a part of ongoing effort on merging and cleaning up our test libraries[1].
>
> webrev: http://cr.openjdk.java.net/~iignatyev//8180386/webrev.00/index.html

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

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(S) : 8180386: remove jdk.testlibrary.TimeLimitedRunner

Igor Ignatyev

> On May 18, 2017, at 2:34 PM, Mandy Chung <[hidden email]> wrote:
>> On May 15, 2017, at 2:00 PM, Igor Ignatyev <[hidden email]> wrote:
>>
>> http://cr.openjdk.java.net/~iignatyev//8180386/webrev.00/index.html
>>> lines changed: 2 ins; 88 del; 6 mod;
>>
>> Hi all,
>>
>> could you please review this small fix which removes TimeLimitedRunner class from jdk testlibrary? we have two TimeLimitedRunner classes one in jdk test library and another in the the toplevel test library, there is no difference b/w these two library classes (besides location and package name), so the tests were simply updated to use TimeLimitedRunner from the toplevel test library.
>>
>> this fix is a part of ongoing effort on merging and cleaning up our test libraries[1].
>>
>> webrev: http://cr.openjdk.java.net/~iignatyev//8180386/webrev.00/index.html
>
> 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.

-- Igor

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(S) : 8180386: remove jdk.testlibrary.TimeLimitedRunner

Mandy Chung

> On May 18, 2017, at 2:37 PM, Igor Ignatyev <[hidden email]> wrote:
>
>
>> On May 18, 2017, at 2:34 PM, Mandy Chung <[hidden email]> 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?

Mandy

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(S) : 8180386: remove jdk.testlibrary.TimeLimitedRunner

Igor Ignatyev

> On May 18, 2017, at 2:41 PM, Mandy Chung <[hidden email]> wrote:
>
>
>> On May 18, 2017, at 2:37 PM, Igor Ignatyev <[hidden email]> wrote:
>>
>>
>>> On May 18, 2017, at 2:34 PM, Mandy Chung <[hidden email]> 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
Loading...