RFR(M) : 8177507 : line number sensitive tests for jdi should be unified

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

RFR(M) : 8177507 : line number sensitive tests for jdi should be unified

Igor Ignatyev
http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00
> 295 lines changed: 176 ins; 15 del; 104 mod;

Hi all,

could you please review this fix for 8177507?

due to their nature, some of jdi tests are line number sensitive. unfortunately different tests indicate that differently, so it's quite easy to overlook that and incidentally break tests, for example by changing module dependency declaration or license modification. this fix unifies the way line number sensitivity is indicated and also improves readability/maintainability of some tests by using constant fields instead of magic numbers.

some of line number sensitive tests have been unexpectedly removed from execution because they had @test/nodynamiccopyright/ instead of @test tag. this changeset fixes and returns them to regular execution.  

webrev: http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00
JBS: https://bugs.openjdk.java.net/browse/JDK-8177507
testing: test/com/sun/jdi

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

Re: RFR(M) : 8177507 : line number sensitive tests for jdi should be unified

Igor Ignatyev
Hi,

still looking for a reviewer, anyone?

-- Igor

> On Mar 24, 2017, at 1:56 PM, Igor Ignatyev <[hidden email]> wrote:
>
> http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00
>> 295 lines changed: 176 ins; 15 del; 104 mod;
>
> Hi all,
>
> could you please review this fix for 8177507?
>
> due to their nature, some of jdi tests are line number sensitive. unfortunately different tests indicate that differently, so it's quite easy to overlook that and incidentally break tests, for example by changing module dependency declaration or license modification. this fix unifies the way line number sensitivity is indicated and also improves readability/maintainability of some tests by using constant fields instead of magic numbers.
>
> some of line number sensitive tests have been unexpectedly removed from execution because they had @test/nodynamiccopyright/ instead of @test tag. this changeset fixes and returns them to regular execution.  
>
> webrev: http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00
> JBS: https://bugs.openjdk.java.net/browse/JDK-8177507
> testing: test/com/sun/jdi
>
> Thanks,
> -- Igor

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

Re: RFR(M) : 8177507 : line number sensitive tests for jdi should be unified

David Holmes
Hi Igor,

This cleanup looks good to me! One query:

test/com/sun/jdi/LineNumberOnBraceTest.java

The old version seems to have the wrong line numbers - was it failing?

Two nits:
- test/com/sun/jdi/FetchLocals.java
- test/com/sun/jdi/LambdaBreakpointTest.java

Second copyright year should be 2017.

Thanks,
David
-----

On 29/03/2017 10:42 AM, Igor Ignatyev wrote:

> Hi,
>
> still looking for a reviewer, anyone?
>
> -- Igor
>> On Mar 24, 2017, at 1:56 PM, Igor Ignatyev <[hidden email]> wrote:
>>
>> http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00
>>> 295 lines changed: 176 ins; 15 del; 104 mod;
>>
>> Hi all,
>>
>> could you please review this fix for 8177507?
>>
>> due to their nature, some of jdi tests are line number sensitive. unfortunately different tests indicate that differently, so it's quite easy to overlook that and incidentally break tests, for example by changing module dependency declaration or license modification. this fix unifies the way line number sensitivity is indicated and also improves readability/maintainability of some tests by using constant fields instead of magic numbers.
>>
>> some of line number sensitive tests have been unexpectedly removed from execution because they had @test/nodynamiccopyright/ instead of @test tag. this changeset fixes and returns them to regular execution.
>>
>> webrev: http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8177507
>> testing: test/com/sun/jdi
>>
>> Thanks,
>> -- Igor
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(M) : 8177507 : line number sensitive tests for jdi should be unified

Igor Ignatyev
Hi David,

thank you for review. please see my answers inlined.

> On Mar 28, 2017, at 6:37 PM, David Holmes <[hidden email]> wrote:
>
> Hi Igor,
>
> This cleanup looks good to me! One query:
>
> test/com/sun/jdi/LineNumberOnBraceTest.java
>
> The old version seems to have the wrong line numbers - was it failing?
it didn't run, because it had @test/nodynamiccopyright/ instead of @test
>
> Two nits:
> - test/com/sun/jdi/FetchLocals.java
> - test/com/sun/jdi/LambdaBreakpointTest.java
>
> Second copyright year should be 2017.
sure, I'll update it before push.

-- Igor

>
> Thanks,
> David
> -----
>
> On 29/03/2017 10:42 AM, Igor Ignatyev wrote:
>> Hi,
>>
>> still looking for a reviewer, anyone?
>>
>> -- Igor
>>> On Mar 24, 2017, at 1:56 PM, Igor Ignatyev <[hidden email]> wrote:
>>>
>>> http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00
>>>> 295 lines changed: 176 ins; 15 del; 104 mod;
>>>
>>> Hi all,
>>>
>>> could you please review this fix for 8177507?
>>>
>>> due to their nature, some of jdi tests are line number sensitive. unfortunately different tests indicate that differently, so it's quite easy to overlook that and incidentally break tests, for example by changing module dependency declaration or license modification. this fix unifies the way line number sensitivity is indicated and also improves readability/maintainability of some tests by using constant fields instead of magic numbers.
>>>
>>> some of line number sensitive tests have been unexpectedly removed from execution because they had @test/nodynamiccopyright/ instead of @test tag. this changeset fixes and returns them to regular execution.
>>>
>>> webrev: http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8177507
>>> testing: test/com/sun/jdi
>>>
>>> Thanks,
>>> -- Igor
>>

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

Re: RFR(M) : 8177507 : line number sensitive tests for jdi should be unified

Mikhailo Seledtsov
In reply to this post by Igor Ignatyev
Hi Igor,

   Looks good. One style nit:

LineNumberOnBraceTarg:
    All other Java tests use style of CAP_UNDERSCORE (e.g. STOP_LINE) for line number variables, but this test uses 'stopLine'.
Consider changing it to STOP_LINE (and STOP_LINE_2) to be uniform.

The rest looks good to me (+ David's comments)


Thank you,
Misha



On 3/28/17, 5:42 PM, Igor Ignatyev wrote:

> Hi,
>
> still looking for a reviewer, anyone?
>
> -- Igor
>> On Mar 24, 2017, at 1:56 PM, Igor Ignatyev<[hidden email]>  wrote:
>>
>> http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00
>>> 295 lines changed: 176 ins; 15 del; 104 mod;
>> Hi all,
>>
>> could you please review this fix for 8177507?
>>
>> due to their nature, some of jdi tests are line number sensitive. unfortunately different tests indicate that differently, so it's quite easy to overlook that and incidentally break tests, for example by changing module dependency declaration or license modification. this fix unifies the way line number sensitivity is indicated and also improves readability/maintainability of some tests by using constant fields instead of magic numbers.
>>
>> some of line number sensitive tests have been unexpectedly removed from execution because they had @test/nodynamiccopyright/ instead of @test tag. this changeset fixes and returns them to regular execution.
>>
>> webrev: http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8177507
>> testing: test/com/sun/jdi
>>
>> Thanks,
>> -- Igor
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(M) : 8177507 : line number sensitive tests for jdi should be unified

serguei.spitsyn@oracle.com
Hi Igor,

+1 to the Mikhailo's comment.


This one also does not look unified:
http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00/test/com/sun/jdi/LambdaBreakpointTest.java.udiff.html

But I do not have a strong opinion on it and leave it up to you.


Thanks,
Serguei


On 3/29/17 11:08, Mikhailo Seledtsov wrote:

> Hi Igor,
>
>   Looks good. One style nit:
>
> LineNumberOnBraceTarg:
>    All other Java tests use style of CAP_UNDERSCORE (e.g. STOP_LINE)
> for line number variables, but this test uses 'stopLine'.
> Consider changing it to STOP_LINE (and STOP_LINE_2) to be uniform.
>
> The rest looks good to me (+ David's comments)
>
>
> Thank you,
> Misha
>
>
>
> On 3/28/17, 5:42 PM, Igor Ignatyev wrote:
>> Hi,
>>
>> still looking for a reviewer, anyone?
>>
>> -- Igor
>>> On Mar 24, 2017, at 1:56 PM, Igor
>>> Ignatyev<[hidden email]>  wrote:
>>>
>>> http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00
>>>> 295 lines changed: 176 ins; 15 del; 104 mod;
>>> Hi all,
>>>
>>> could you please review this fix for 8177507?
>>>
>>> due to their nature, some of jdi tests are line number sensitive.
>>> unfortunately different tests indicate that differently, so it's
>>> quite easy to overlook that and incidentally break tests, for
>>> example by changing module dependency declaration or license
>>> modification. this fix unifies the way line number sensitivity is
>>> indicated and also improves readability/maintainability of some
>>> tests by using constant fields instead of magic numbers.
>>>
>>> some of line number sensitive tests have been unexpectedly removed
>>> from execution because they had @test/nodynamiccopyright/ instead of
>>> @test tag. this changeset fixes and returns them to regular execution.
>>>
>>> webrev: http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8177507
>>> testing: test/com/sun/jdi
>>>
>>> Thanks,
>>> -- Igor

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

Re: RFR(M) : 8177507 : line number sensitive tests for jdi should be unified

Igor Ignatyev
Hi,

http://cr.openjdk.java.net/~iignatyev//8177507/webrev.01 <http://cr.openjdk.java.net/~iignatyev//8177507/webrev.01> is the next iteration w/ copyright years fixed, LineNumberOnBraceTarg and LambdaBreakpointTest made more unified w/ the rest.

Thanks,
-- Igor

> On Mar 29, 2017, at 12:57 PM, [hidden email] wrote:
>
> This one also does not look unified:
> http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00/test/com/sun/jdi/LambdaBreakpointTest.java.udiff.html

> On 3/29/17 11:08, Mikhailo Seledtsov wrote:
>
> One style nit:
>
> LineNumberOnBraceTarg:
>   All other Java tests use style of CAP_UNDERSCORE (e.g. STOP_LINE) for line number variables, but this test uses 'stopLine'.
> Consider changing it to STOP_LINE (and STOP_LINE_2) to be uniform.

> On Mar 28, 2017, at 6:37 PM, David Holmes <[hidden email]> wrote:
>
> Two nits:
> - test/com/sun/jdi/FetchLocals.java
> - test/com/sun/jdi/LambdaBreakpointTest.java
>
> Second copyright year should be 2017.

> On Mar 24, 2017, at 1:56 PM, Igor Ignatyev <[hidden email]> wrote:

> Hi all,
>
> could you please review this fix for 8177507?
>
> due to their nature, some of jdi tests are line number sensitive. unfortunately different tests indicate that differently, so it's quite easy to overlook that and incidentally break tests, for example by changing module dependency declaration or license modification. this fix unifies the way line number sensitivity is indicated and also improves readability/maintainability of some tests by using constant fields instead of magic numbers.
>
> some of line number sensitive tests have been unexpectedly removed from execution because they had @test/nodynamiccopyright/ instead of @test tag. this changeset fixes and returns them to regular execution.  
>
> webrev: http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00 <http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00>
> JBS: https://bugs.openjdk.java.net/browse/JDK-8177507 <https://bugs.openjdk.java.net/browse/JDK-8177507>
> testing: test/com/sun/jdi
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(M) : 8177507 : line number sensitive tests for jdi should be unified

David Holmes
Hi Igor,

On 3/04/2017 3:09 AM, Igor Ignatyev wrote:
> Hi,
>
> http://cr.openjdk.java.net/~iignatyev//8177507/webrev.01 is the next
> iteration w/ copyright years fixed,

test/com/sun/jdi/BreakpointTest.java

also needs fixing - says 2015.

> LineNumberOnBraceTarg and

test/com/sun/jdi/LineNumberOnBraceTest.java - Ok.

> LambdaBreakpointTest made more unified w/ the rest.

I don't see any change here.

Thanks,
David
-----

> Thanks,
> -- Igor
>
>> On Mar 29, 2017, at 12:57 PM, [hidden email]
>> <mailto:[hidden email]> wrote:
>>
>> This one also does not look unified:
>> http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00/test/com/sun/jdi/LambdaBreakpointTest.java.udiff.html
>
>> On 3/29/17 11:08, Mikhailo Seledtsov wrote:
>>
>> One style nit:
>>
>> LineNumberOnBraceTarg:
>>   All other Java tests use style of CAP_UNDERSCORE (e.g. STOP_LINE)
>> for line number variables, but this test uses 'stopLine'.
>> Consider changing it to STOP_LINE (and STOP_LINE_2) to be uniform.
>
>> On Mar 28, 2017, at 6:37 PM, David Holmes <[hidden email]
>> <mailto:[hidden email]>> wrote:
>>
>> Two nits:
>> - test/com/sun/jdi/FetchLocals.java
>> - test/com/sun/jdi/LambdaBreakpointTest.java
>>
>> Second copyright year should be 2017.
>
>> On Mar 24, 2017, at 1:56 PM, Igor Ignatyev <[hidden email]
>> <mailto:[hidden email]>> wrote:
>> Hi all,
>>
>> could you please review this fix for 8177507?
>>
>> due to their nature, some of jdi tests are line number sensitive.
>> unfortunately different tests indicate that differently, so it's quite
>> easy to overlook that and incidentally break tests, for example by
>> changing module dependency declaration or license modification. this
>> fix unifies the way line number sensitivity is indicated and also
>> improves readability/maintainability of some tests by using constant
>> fields instead of magic numbers.
>>
>> some of line number sensitive tests have been unexpectedly removed
>> from execution because they had @test/nodynamiccopyright/ instead of
>> @test tag. this changeset fixes and returns them to regular execution.
>>
>> webrev: http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00
>> JBS: https://bugs.openjdk.java.net/browse/JDK-8177507
>> testing: test/com/sun/jdi
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(M) : 8177507 : line number sensitive tests for jdi should be unified

Igor Ignatyev
looks like I have uploaded the webrev before I saved files... uploaded a new webrev w/ all mentioned changes: http://cr.openjdk.java.net/~iignatyev/8177507/webrev.02 <http://cr.openjdk.java.net/~iignatyev/8177507/webrev.02>

-- Igor

> On Apr 2, 2017, at 1:31 PM, David Holmes <[hidden email] <mailto:[hidden email]>> wrote:
>
> Hi Igor,
>
> On 3/04/2017 3:09 AM, Igor Ignatyev wrote:
>> Hi,
>>
>> http://cr.openjdk.java.net/~iignatyev//8177507/webrev.01 <http://cr.openjdk.java.net/~iignatyev//8177507/webrev.01> is the next
>> iteration w/ copyright years fixed,
>
> test/com/sun/jdi/BreakpointTest.java
>
> also needs fixing - says 2015.
>
>> LineNumberOnBraceTarg and
>
> test/com/sun/jdi/LineNumberOnBraceTest.java - Ok.
>
>> LambdaBreakpointTest made more unified w/ the rest.
>
> I don't see any change here.
>
> Thanks,
> David
> -----
>
>> Thanks,
>> -- Igor
>>
>>> On Mar 29, 2017, at 12:57 PM, [hidden email] <mailto:[hidden email]>
>>> <mailto:[hidden email] <mailto:[hidden email]>> wrote:
>>>
>>> This one also does not look unified:
>>> http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00/test/com/sun/jdi/LambdaBreakpointTest.java.udiff.html <http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00/test/com/sun/jdi/LambdaBreakpointTest.java.udiff.html>
>>
>>> On 3/29/17 11:08, Mikhailo Seledtsov wrote:
>>>
>>> One style nit:
>>>
>>> LineNumberOnBraceTarg:
>>>  All other Java tests use style of CAP_UNDERSCORE (e.g. STOP_LINE)
>>> for line number variables, but this test uses 'stopLine'.
>>> Consider changing it to STOP_LINE (and STOP_LINE_2) to be uniform.
>>
>>> On Mar 28, 2017, at 6:37 PM, David Holmes <[hidden email] <mailto:[hidden email]>
>>> <mailto:[hidden email] <mailto:[hidden email]>>> wrote:
>>>
>>> Two nits:
>>> - test/com/sun/jdi/FetchLocals.java
>>> - test/com/sun/jdi/LambdaBreakpointTest.java
>>>
>>> Second copyright year should be 2017.
>>
>>> On Mar 24, 2017, at 1:56 PM, Igor Ignatyev <[hidden email] <mailto:[hidden email]>
>>> <mailto:[hidden email] <mailto:[hidden email]>>> wrote:
>>> Hi all,
>>>
>>> could you please review this fix for 8177507?
>>>
>>> due to their nature, some of jdi tests are line number sensitive.
>>> unfortunately different tests indicate that differently, so it's quite
>>> easy to overlook that and incidentally break tests, for example by
>>> changing module dependency declaration or license modification. this
>>> fix unifies the way line number sensitivity is indicated and also
>>> improves readability/maintainability of some tests by using constant
>>> fields instead of magic numbers.
>>>
>>> some of line number sensitive tests have been unexpectedly removed
>>> from execution because they had @test/nodynamiccopyright/ instead of
>>> @test tag. this changeset fixes and returns them to regular execution.
>>>
>>> webrev: http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00 <http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00>
>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8177507 <https://bugs.openjdk.java.net/browse/JDK-8177507>
>>> testing: test/com/sun/jdi

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

Re: RFR(M) : 8177507 : line number sensitive tests for jdi should be unified

serguei.spitsyn@oracle.com
Hi Igor,

It looks good.
Thank you for the update!

Thanks,
Serguei


On 4/3/17 09:46, Igor Ignatyev wrote:

> looks like I have uploaded the webrev before I saved files... uploaded
> a new webrev w/ all mentioned changes:
> http://cr.openjdk.java.net/~iignatyev/8177507/webrev.02 
> <http://cr.openjdk.java.net/%7Eiignatyev/8177507/webrev.02>
>
> -- Igor
>
>> On Apr 2, 2017, at 1:31 PM, David Holmes <[hidden email]
>> <mailto:[hidden email]>> wrote:
>>
>> Hi Igor,
>>
>> On 3/04/2017 3:09 AM, Igor Ignatyev wrote:
>>> Hi,
>>>
>>> http://cr.openjdk.java.net/~iignatyev//8177507/webrev.01 
>>> <http://cr.openjdk.java.net/%7Eiignatyev//8177507/webrev.01> is the next
>>> iteration w/ copyright years fixed,
>>
>> test/com/sun/jdi/BreakpointTest.java
>>
>> also needs fixing - says 2015.
>>
>>> LineNumberOnBraceTarg and
>>
>> test/com/sun/jdi/LineNumberOnBraceTest.java - Ok.
>>
>>> LambdaBreakpointTest made more unified w/ the rest.
>>
>> I don't see any change here.
>>
>> Thanks,
>> David
>> -----
>>
>>> Thanks,
>>> -- Igor
>>>
>>>> On Mar 29, 2017, at 12:57 PM, [hidden email]
>>>> <mailto:[hidden email]>
>>>> <mailto:[hidden email]> wrote:
>>>>
>>>> This one also does not look unified:
>>>> http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00/test/com/sun/jdi/LambdaBreakpointTest.java.udiff.html 
>>>> <http://cr.openjdk.java.net/%7Eiignatyev/8177507/webrev.00/test/com/sun/jdi/LambdaBreakpointTest.java.udiff.html>
>>>
>>>> On 3/29/17 11:08, Mikhailo Seledtsov wrote:
>>>>
>>>> One style nit:
>>>>
>>>> LineNumberOnBraceTarg:
>>>>  All other Java tests use style of CAP_UNDERSCORE (e.g. STOP_LINE)
>>>> for line number variables, but this test uses 'stopLine'.
>>>> Consider changing it to STOP_LINE (and STOP_LINE_2) to be uniform.
>>>
>>>> On Mar 28, 2017, at 6:37 PM, David Holmes <[hidden email]
>>>> <mailto:[hidden email]>
>>>> <mailto:[hidden email]>> wrote:
>>>>
>>>> Two nits:
>>>> - test/com/sun/jdi/FetchLocals.java
>>>> - test/com/sun/jdi/LambdaBreakpointTest.java
>>>>
>>>> Second copyright year should be 2017.
>>>
>>>> On Mar 24, 2017, at 1:56 PM, Igor Ignatyev
>>>> <[hidden email] <mailto:[hidden email]>
>>>> <mailto:[hidden email]>> wrote:
>>>> Hi all,
>>>>
>>>> could you please review this fix for 8177507?
>>>>
>>>> due to their nature, some of jdi tests are line number sensitive.
>>>> unfortunately different tests indicate that differently, so it's quite
>>>> easy to overlook that and incidentally break tests, for example by
>>>> changing module dependency declaration or license modification. this
>>>> fix unifies the way line number sensitivity is indicated and also
>>>> improves readability/maintainability of some tests by using constant
>>>> fields instead of magic numbers.
>>>>
>>>> some of line number sensitive tests have been unexpectedly removed
>>>> from execution because they had @test/nodynamiccopyright/ instead of
>>>> @test tag. this changeset fixes and returns them to regular execution.
>>>>
>>>> webrev: http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00 
>>>> <http://cr.openjdk.java.net/%7Eiignatyev/8177507/webrev.00>
>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8177507
>>>> testing: test/com/sun/jdi
>

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

Re: RFR(M) : 8177507 : line number sensitive tests for jdi should be unified

David Holmes
In reply to this post by Igor Ignatyev
On 4/04/2017 2:46 AM, Igor Ignatyev wrote:
> looks like I have uploaded the webrev before I saved files... uploaded a
> new webrev w/ all mentioned changes:
> http://cr.openjdk.java.net/~iignatyev/8177507/webrev.02

I don't see how the change to LambdaBreakpointTest can possibly work.
The new test steps to each line in source code order, but that is not
the execution order!

David
-----

> -- Igor
>
>> On Apr 2, 2017, at 1:31 PM, David Holmes <[hidden email]
>> <mailto:[hidden email]>> wrote:
>>
>> Hi Igor,
>>
>> On 3/04/2017 3:09 AM, Igor Ignatyev wrote:
>>> Hi,
>>>
>>> http://cr.openjdk.java.net/~iignatyev//8177507/webrev.01 is the next
>>> iteration w/ copyright years fixed,
>>
>> test/com/sun/jdi/BreakpointTest.java
>>
>> also needs fixing - says 2015.
>>
>>> LineNumberOnBraceTarg and
>>
>> test/com/sun/jdi/LineNumberOnBraceTest.java - Ok.
>>
>>> LambdaBreakpointTest made more unified w/ the rest.
>>
>> I don't see any change here.
>>
>> Thanks,
>> David
>> -----
>>
>>> Thanks,
>>> -- Igor
>>>
>>>> On Mar 29, 2017, at 12:57 PM, [hidden email]
>>>> <mailto:[hidden email]>
>>>> <mailto:[hidden email]> wrote:
>>>>
>>>> This one also does not look unified:
>>>> http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00/test/com/sun/jdi/LambdaBreakpointTest.java.udiff.html
>>>
>>>> On 3/29/17 11:08, Mikhailo Seledtsov wrote:
>>>>
>>>> One style nit:
>>>>
>>>> LineNumberOnBraceTarg:
>>>>  All other Java tests use style of CAP_UNDERSCORE (e.g. STOP_LINE)
>>>> for line number variables, but this test uses 'stopLine'.
>>>> Consider changing it to STOP_LINE (and STOP_LINE_2) to be uniform.
>>>
>>>> On Mar 28, 2017, at 6:37 PM, David Holmes <[hidden email]
>>>> <mailto:[hidden email]>
>>>> <mailto:[hidden email]>> wrote:
>>>>
>>>> Two nits:
>>>> - test/com/sun/jdi/FetchLocals.java
>>>> - test/com/sun/jdi/LambdaBreakpointTest.java
>>>>
>>>> Second copyright year should be 2017.
>>>
>>>> On Mar 24, 2017, at 1:56 PM, Igor Ignatyev <[hidden email]
>>>> <mailto:[hidden email]>
>>>> <mailto:[hidden email]>> wrote:
>>>> Hi all,
>>>>
>>>> could you please review this fix for 8177507?
>>>>
>>>> due to their nature, some of jdi tests are line number sensitive.
>>>> unfortunately different tests indicate that differently, so it's quite
>>>> easy to overlook that and incidentally break tests, for example by
>>>> changing module dependency declaration or license modification. this
>>>> fix unifies the way line number sensitivity is indicated and also
>>>> improves readability/maintainability of some tests by using constant
>>>> fields instead of magic numbers.
>>>>
>>>> some of line number sensitive tests have been unexpectedly removed
>>>> from execution because they had @test/nodynamiccopyright/ instead of
>>>> @test tag. this changeset fixes and returns them to regular execution.
>>>>
>>>> webrev: http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00
>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8177507
>>>> testing: test/com/sun/jdi
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(M) : 8177507 : line number sensitive tests for jdi should be unified

serguei.spitsyn@oracle.com
On 4/3/17 13:53, David Holmes wrote:
> On 4/04/2017 2:46 AM, Igor Ignatyev wrote:
>> looks like I have uploaded the webrev before I saved files... uploaded a
>> new webrev w/ all mentioned changes:
>> http://cr.openjdk.java.net/~iignatyev/8177507/webrev.02
>
> I don't see how the change to LambdaBreakpointTest can possibly work.
> The new test steps to each line in source code order, but that is not
> the execution order!

Nice catch!

   62     private static void test() {
63 Runnable r = () -> { // B1: L62
64 String from = "lambda"; // B3: L63
65 System.out.println("Hello from " + from); // B4: L64
66 }; // B5: L65
67 r.run(); // B2: L66
68 System.out.println("Goodbye."); // B6: L67
   69     }


The B2 breakpoint is at the L66, but B3 is at L63.

Thanks,
Serguei



>
> David
> -----
>
>> -- Igor
>>
>>> On Apr 2, 2017, at 1:31 PM, David Holmes <[hidden email]
>>> <mailto:[hidden email]>> wrote:
>>>
>>> Hi Igor,
>>>
>>> On 3/04/2017 3:09 AM, Igor Ignatyev wrote:
>>>> Hi,
>>>>
>>>> http://cr.openjdk.java.net/~iignatyev//8177507/webrev.01 is the next
>>>> iteration w/ copyright years fixed,
>>>
>>> test/com/sun/jdi/BreakpointTest.java
>>>
>>> also needs fixing - says 2015.
>>>
>>>> LineNumberOnBraceTarg and
>>>
>>> test/com/sun/jdi/LineNumberOnBraceTest.java - Ok.
>>>
>>>> LambdaBreakpointTest made more unified w/ the rest.
>>>
>>> I don't see any change here.
>>>
>>> Thanks,
>>> David
>>> -----
>>>
>>>> Thanks,
>>>> -- Igor
>>>>
>>>>> On Mar 29, 2017, at 12:57 PM, [hidden email]
>>>>> <mailto:[hidden email]>
>>>>> <mailto:[hidden email]> wrote:
>>>>>
>>>>> This one also does not look unified:
>>>>> http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00/test/com/sun/jdi/LambdaBreakpointTest.java.udiff.html 
>>>>>
>>>>
>>>>> On 3/29/17 11:08, Mikhailo Seledtsov wrote:
>>>>>
>>>>> One style nit:
>>>>>
>>>>> LineNumberOnBraceTarg:
>>>>>  All other Java tests use style of CAP_UNDERSCORE (e.g. STOP_LINE)
>>>>> for line number variables, but this test uses 'stopLine'.
>>>>> Consider changing it to STOP_LINE (and STOP_LINE_2) to be uniform.
>>>>
>>>>> On Mar 28, 2017, at 6:37 PM, David Holmes <[hidden email]
>>>>> <mailto:[hidden email]>
>>>>> <mailto:[hidden email]>> wrote:
>>>>>
>>>>> Two nits:
>>>>> - test/com/sun/jdi/FetchLocals.java
>>>>> - test/com/sun/jdi/LambdaBreakpointTest.java
>>>>>
>>>>> Second copyright year should be 2017.
>>>>
>>>>> On Mar 24, 2017, at 1:56 PM, Igor Ignatyev <[hidden email]
>>>>> <mailto:[hidden email]>
>>>>> <mailto:[hidden email]>> wrote:
>>>>> Hi all,
>>>>>
>>>>> could you please review this fix for 8177507?
>>>>>
>>>>> due to their nature, some of jdi tests are line number sensitive.
>>>>> unfortunately different tests indicate that differently, so it's
>>>>> quite
>>>>> easy to overlook that and incidentally break tests, for example by
>>>>> changing module dependency declaration or license modification. this
>>>>> fix unifies the way line number sensitivity is indicated and also
>>>>> improves readability/maintainability of some tests by using constant
>>>>> fields instead of magic numbers.
>>>>>
>>>>> some of line number sensitive tests have been unexpectedly removed
>>>>> from execution because they had @test/nodynamiccopyright/ instead of
>>>>> @test tag. this changeset fixes and returns them to regular
>>>>> execution.
>>>>>
>>>>> webrev: http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00
>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8177507
>>>>> testing: test/com/sun/jdi
>>

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

Re: RFR(M) : 8177507 : line number sensitive tests for jdi should be unified

Igor Ignatyev
In reply to this post by David Holmes
yes, David you are right. it's so embarrassing ... besides uploading webrev w/o all changes, I have also tested it w/o all changes, I should have noticed that. Thank you for catching that.

I have fixed that, tested and uploaded new webrev -- http://cr.openjdk.java.net/~iignatyev/8177507/webrev.03 <http://cr.openjdk.java.net/~iignatyev/8177507/webrev.03>

Thanks,
-- Igor

> On Apr 3, 2017, at 1:53 PM, David Holmes <[hidden email]> wrote:
>
> On 4/04/2017 2:46 AM, Igor Ignatyev wrote:
>> looks like I have uploaded the webrev before I saved files... uploaded a
>> new webrev w/ all mentioned changes:
>> http://cr.openjdk.java.net/~iignatyev/8177507/webrev.02 <http://cr.openjdk.java.net/~iignatyev/8177507/webrev.02>
>
> I don't see how the change to LambdaBreakpointTest can possibly work. The new test steps to each line in source code order, but that is not the execution order!
>
> David
> -----
>
>> -- Igor
>>
>>> On Apr 2, 2017, at 1:31 PM, David Holmes <[hidden email] <mailto:[hidden email]>
>>> <mailto:[hidden email] <mailto:[hidden email]>>> wrote:
>>>
>>> Hi Igor,
>>>
>>> On 3/04/2017 3:09 AM, Igor Ignatyev wrote:
>>>> Hi,
>>>>
>>>> http://cr.openjdk.java.net/~iignatyev//8177507/webrev.01 is the next
>>>> iteration w/ copyright years fixed,
>>>
>>> test/com/sun/jdi/BreakpointTest.java
>>>
>>> also needs fixing - says 2015.
>>>
>>>> LineNumberOnBraceTarg and
>>>
>>> test/com/sun/jdi/LineNumberOnBraceTest.java - Ok.
>>>
>>>> LambdaBreakpointTest made more unified w/ the rest.
>>>
>>> I don't see any change here.
>>>
>>> Thanks,
>>> David
>>> -----
>>>
>>>> Thanks,
>>>> -- Igor
>>>>
>>>>> On Mar 29, 2017, at 12:57 PM, [hidden email] <mailto:[hidden email]>
>>>>> <mailto:[hidden email] <mailto:[hidden email]>>
>>>>> <mailto:[hidden email] <mailto:[hidden email]>> wrote:
>>>>>
>>>>> This one also does not look unified:
>>>>> http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00/test/com/sun/jdi/LambdaBreakpointTest.java.udiff.html <http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00/test/com/sun/jdi/LambdaBreakpointTest.java.udiff.html>
>>>>
>>>>> On 3/29/17 11:08, Mikhailo Seledtsov wrote:
>>>>>
>>>>> One style nit:
>>>>>
>>>>> LineNumberOnBraceTarg:
>>>>> All other Java tests use style of CAP_UNDERSCORE (e.g. STOP_LINE)
>>>>> for line number variables, but this test uses 'stopLine'.
>>>>> Consider changing it to STOP_LINE (and STOP_LINE_2) to be uniform.
>>>>
>>>>> On Mar 28, 2017, at 6:37 PM, David Holmes <[hidden email] <mailto:[hidden email]>
>>>>> <mailto:[hidden email] <mailto:[hidden email]>>
>>>>> <mailto:[hidden email] <mailto:[hidden email]>>> wrote:
>>>>>
>>>>> Two nits:
>>>>> - test/com/sun/jdi/FetchLocals.java
>>>>> - test/com/sun/jdi/LambdaBreakpointTest.java
>>>>>
>>>>> Second copyright year should be 2017.
>>>>
>>>>> On Mar 24, 2017, at 1:56 PM, Igor Ignatyev <[hidden email] <mailto:[hidden email]>
>>>>> <mailto:[hidden email] <mailto:[hidden email]>>
>>>>> <mailto:[hidden email]>> wrote:
>>>>> Hi all,
>>>>>
>>>>> could you please review this fix for 8177507?
>>>>>
>>>>> due to their nature, some of jdi tests are line number sensitive.
>>>>> unfortunately different tests indicate that differently, so it's quite
>>>>> easy to overlook that and incidentally break tests, for example by
>>>>> changing module dependency declaration or license modification. this
>>>>> fix unifies the way line number sensitivity is indicated and also
>>>>> improves readability/maintainability of some tests by using constant
>>>>> fields instead of magic numbers.
>>>>>
>>>>> some of line number sensitive tests have been unexpectedly removed
>>>>> from execution because they had @test/nodynamiccopyright/ instead of
>>>>> @test tag. this changeset fixes and returns them to regular execution.
>>>>>
>>>>> webrev: http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00
>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8177507
>>>>> testing: test/com/sun/jdi

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

Re: RFR(M) : 8177507 : line number sensitive tests for jdi should be unified

serguei.spitsyn@oracle.com
It is good now.

Thanks,
Serguei



On 4/3/17 16:09, Igor Ignatyev wrote:

> yes, David you are right. it's so embarrassing ... besides uploading
> webrev w/o all changes, I have also tested it w/o all changes, I
> should have noticed that. Thank you for catching that.
>
> I have fixed that, tested and uploaded new webrev --
> _http://cr.openjdk.java.net/~iignatyev/8177507/webrev.03
> <http://cr.openjdk.java.net/%7Eiignatyev/8177507/webrev.03>_
>
> Thanks,
> -- Igor
>> On Apr 3, 2017, at 1:53 PM, David Holmes <[hidden email]
>> <mailto:[hidden email]>> wrote:
>>
>> On 4/04/2017 2:46 AM, Igor Ignatyev wrote:
>>> looks like I have uploaded the webrev before I saved files... uploaded a
>>> new webrev w/ all mentioned changes:
>>> http://cr.openjdk.java.net/~iignatyev/8177507/webrev.02 
>>> <http://cr.openjdk.java.net/%7Eiignatyev/8177507/webrev.02>
>>
>> I don't see how the change to LambdaBreakpointTest can possibly work.
>> The new test steps to each line in source code order, but that is not
>> the execution order!
>>
>> David
>> -----
>>
>>> -- Igor
>>>
>>>> On Apr 2, 2017, at 1:31 PM, David Holmes <[hidden email]
>>>> <mailto:[hidden email]>
>>>> <mailto:[hidden email]>> wrote:
>>>>
>>>> Hi Igor,
>>>>
>>>> On 3/04/2017 3:09 AM, Igor Ignatyev wrote:
>>>>> Hi,
>>>>>
>>>>> http://cr.openjdk.java.net/~iignatyev//8177507/webrev.01 
>>>>> <http://cr.openjdk.java.net/%7Eiignatyev//8177507/webrev.01> is
>>>>> the next
>>>>> iteration w/ copyright years fixed,
>>>>
>>>> test/com/sun/jdi/BreakpointTest.java
>>>>
>>>> also needs fixing - says 2015.
>>>>
>>>>> LineNumberOnBraceTarg and
>>>>
>>>> test/com/sun/jdi/LineNumberOnBraceTest.java - Ok.
>>>>
>>>>> LambdaBreakpointTest made more unified w/ the rest.
>>>>
>>>> I don't see any change here.
>>>>
>>>> Thanks,
>>>> David
>>>> -----
>>>>
>>>>> Thanks,
>>>>> -- Igor
>>>>>
>>>>>> On Mar 29, 2017, at 12:57 PM,[hidden email]
>>>>>> <mailto:[hidden email]>
>>>>>> <mailto:[hidden email]>
>>>>>> <mailto:[hidden email]> wrote:
>>>>>>
>>>>>> This one also does not look unified:
>>>>>> http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00/test/com/sun/jdi/LambdaBreakpointTest.java.udiff.html 
>>>>>> <http://cr.openjdk.java.net/%7Eiignatyev/8177507/webrev.00/test/com/sun/jdi/LambdaBreakpointTest.java.udiff.html>
>>>>>
>>>>>> On 3/29/17 11:08, Mikhailo Seledtsov wrote:
>>>>>>
>>>>>> One style nit:
>>>>>>
>>>>>> LineNumberOnBraceTarg:
>>>>>> All other Java tests use style of CAP_UNDERSCORE (e.g. STOP_LINE)
>>>>>> for line number variables, but this test uses 'stopLine'.
>>>>>> Consider changing it to STOP_LINE (and STOP_LINE_2) to be uniform.
>>>>>
>>>>>> On Mar 28, 2017, at 6:37 PM, David Holmes
>>>>>> <[hidden email] <mailto:[hidden email]>
>>>>>> <mailto:[hidden email]>
>>>>>> <mailto:[hidden email]>> wrote:
>>>>>>
>>>>>> Two nits:
>>>>>> - test/com/sun/jdi/FetchLocals.java
>>>>>> - test/com/sun/jdi/LambdaBreakpointTest.java
>>>>>>
>>>>>> Second copyright year should be 2017.
>>>>>
>>>>>> On Mar 24, 2017, at 1:56 PM, Igor Ignatyev
>>>>>> <[hidden email] <mailto:[hidden email]>
>>>>>> <mailto:[hidden email]>
>>>>>> <mailto:[hidden email]>> wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> could you please review this fix for 8177507?
>>>>>>
>>>>>> due to their nature, some of jdi tests are line number sensitive.
>>>>>> unfortunately different tests indicate that differently, so it's
>>>>>> quite
>>>>>> easy to overlook that and incidentally break tests, for example by
>>>>>> changing module dependency declaration or license modification. this
>>>>>> fix unifies the way line number sensitivity is indicated and also
>>>>>> improves readability/maintainability of some tests by using constant
>>>>>> fields instead of magic numbers.
>>>>>>
>>>>>> some of line number sensitive tests have been unexpectedly removed
>>>>>> from execution because they had @test/nodynamiccopyright/ instead of
>>>>>> @test tag. this changeset fixes and returns them to regular
>>>>>> execution.
>>>>>>
>>>>>> webrev: http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00 
>>>>>> <http://cr.openjdk.java.net/%7Eiignatyev/8177507/webrev.00>
>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8177507
>>>>>> testing: test/com/sun/jdi
>

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

Re: RFR(M) : 8177507 : line number sensitive tests for jdi should be unified

David Holmes
In reply to this post by Igor Ignatyev
On 4/04/2017 9:09 AM, Igor Ignatyev wrote:
> yes, David you are right. it's so embarrassing ... besides uploading
> webrev w/o all changes, I have also tested it w/o all changes, I should
> have noticed that. Thank you for catching that.
>
> I have fixed that, tested and uploaded new webrev --
> _http://cr.openjdk.java.net/~iignatyev/8177507/webrev.03_

Looks fine now.

Thanks,
David

> Thanks,
> -- Igor
>> On Apr 3, 2017, at 1:53 PM, David Holmes <[hidden email]
>> <mailto:[hidden email]>> wrote:
>>
>> On 4/04/2017 2:46 AM, Igor Ignatyev wrote:
>>> looks like I have uploaded the webrev before I saved files... uploaded a
>>> new webrev w/ all mentioned changes:
>>> http://cr.openjdk.java.net/~iignatyev/8177507/webrev.02
>>
>> I don't see how the change to LambdaBreakpointTest can possibly work.
>> The new test steps to each line in source code order, but that is not
>> the execution order!
>>
>> David
>> -----
>>
>>> -- Igor
>>>
>>>> On Apr 2, 2017, at 1:31 PM, David Holmes <[hidden email]
>>>> <mailto:[hidden email]>
>>>> <mailto:[hidden email]>> wrote:
>>>>
>>>> Hi Igor,
>>>>
>>>> On 3/04/2017 3:09 AM, Igor Ignatyev wrote:
>>>>> Hi,
>>>>>
>>>>> http://cr.openjdk.java.net/~iignatyev//8177507/webrev.01 is the next
>>>>> iteration w/ copyright years fixed,
>>>>
>>>> test/com/sun/jdi/BreakpointTest.java
>>>>
>>>> also needs fixing - says 2015.
>>>>
>>>>> LineNumberOnBraceTarg and
>>>>
>>>> test/com/sun/jdi/LineNumberOnBraceTest.java - Ok.
>>>>
>>>>> LambdaBreakpointTest made more unified w/ the rest.
>>>>
>>>> I don't see any change here.
>>>>
>>>> Thanks,
>>>> David
>>>> -----
>>>>
>>>>> Thanks,
>>>>> -- Igor
>>>>>
>>>>>> On Mar 29, 2017, at 12:57 PM, [hidden email]
>>>>>> <mailto:[hidden email]>
>>>>>> <mailto:[hidden email]>
>>>>>> <mailto:[hidden email]> wrote:
>>>>>>
>>>>>> This one also does not look unified:
>>>>>> http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00/test/com/sun/jdi/LambdaBreakpointTest.java.udiff.html
>>>>>
>>>>>> On 3/29/17 11:08, Mikhailo Seledtsov wrote:
>>>>>>
>>>>>> One style nit:
>>>>>>
>>>>>> LineNumberOnBraceTarg:
>>>>>> All other Java tests use style of CAP_UNDERSCORE (e.g. STOP_LINE)
>>>>>> for line number variables, but this test uses 'stopLine'.
>>>>>> Consider changing it to STOP_LINE (and STOP_LINE_2) to be uniform.
>>>>>
>>>>>> On Mar 28, 2017, at 6:37 PM, David Holmes <[hidden email]
>>>>>> <mailto:[hidden email]>
>>>>>> <mailto:[hidden email]>
>>>>>> <mailto:[hidden email]>> wrote:
>>>>>>
>>>>>> Two nits:
>>>>>> - test/com/sun/jdi/FetchLocals.java
>>>>>> - test/com/sun/jdi/LambdaBreakpointTest.java
>>>>>>
>>>>>> Second copyright year should be 2017.
>>>>>
>>>>>> On Mar 24, 2017, at 1:56 PM, Igor Ignatyev
>>>>>> <[hidden email] <mailto:[hidden email]>
>>>>>> <mailto:[hidden email]>
>>>>>> <mailto:[hidden email]>> wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> could you please review this fix for 8177507?
>>>>>>
>>>>>> due to their nature, some of jdi tests are line number sensitive.
>>>>>> unfortunately different tests indicate that differently, so it's quite
>>>>>> easy to overlook that and incidentally break tests, for example by
>>>>>> changing module dependency declaration or license modification. this
>>>>>> fix unifies the way line number sensitivity is indicated and also
>>>>>> improves readability/maintainability of some tests by using constant
>>>>>> fields instead of magic numbers.
>>>>>>
>>>>>> some of line number sensitive tests have been unexpectedly removed
>>>>>> from execution because they had @test/nodynamiccopyright/ instead of
>>>>>> @test tag. this changeset fixes and returns them to regular execution.
>>>>>>
>>>>>> webrev: http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00
>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8177507
>>>>>> testing: test/com/sun/jdi
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(M) : 8177507 : line number sensitive tests for jdi should be unified

Mikhailo Seledtsov
Looks good to me.

Misha

On 4/3/17, 4:54 PM, David Holmes wrote:

> On 4/04/2017 9:09 AM, Igor Ignatyev wrote:
>> yes, David you are right. it's so embarrassing ... besides uploading
>> webrev w/o all changes, I have also tested it w/o all changes, I should
>> have noticed that. Thank you for catching that.
>>
>> I have fixed that, tested and uploaded new webrev --
>> _http://cr.openjdk.java.net/~iignatyev/8177507/webrev.03_
>
> Looks fine now.
>
> Thanks,
> David
>
>> Thanks,
>> -- Igor
>>> On Apr 3, 2017, at 1:53 PM, David Holmes <[hidden email]
>>> <mailto:[hidden email]>> wrote:
>>>
>>> On 4/04/2017 2:46 AM, Igor Ignatyev wrote:
>>>> looks like I have uploaded the webrev before I saved files...
>>>> uploaded a
>>>> new webrev w/ all mentioned changes:
>>>> http://cr.openjdk.java.net/~iignatyev/8177507/webrev.02
>>>
>>> I don't see how the change to LambdaBreakpointTest can possibly work.
>>> The new test steps to each line in source code order, but that is not
>>> the execution order!
>>>
>>> David
>>> -----
>>>
>>>> -- Igor
>>>>
>>>>> On Apr 2, 2017, at 1:31 PM, David Holmes <[hidden email]
>>>>> <mailto:[hidden email]>
>>>>> <mailto:[hidden email]>> wrote:
>>>>>
>>>>> Hi Igor,
>>>>>
>>>>> On 3/04/2017 3:09 AM, Igor Ignatyev wrote:
>>>>>> Hi,
>>>>>>
>>>>>> http://cr.openjdk.java.net/~iignatyev//8177507/webrev.01 is the next
>>>>>> iteration w/ copyright years fixed,
>>>>>
>>>>> test/com/sun/jdi/BreakpointTest.java
>>>>>
>>>>> also needs fixing - says 2015.
>>>>>
>>>>>> LineNumberOnBraceTarg and
>>>>>
>>>>> test/com/sun/jdi/LineNumberOnBraceTest.java - Ok.
>>>>>
>>>>>> LambdaBreakpointTest made more unified w/ the rest.
>>>>>
>>>>> I don't see any change here.
>>>>>
>>>>> Thanks,
>>>>> David
>>>>> -----
>>>>>
>>>>>> Thanks,
>>>>>> -- Igor
>>>>>>
>>>>>>> On Mar 29, 2017, at 12:57 PM, [hidden email]
>>>>>>> <mailto:[hidden email]>
>>>>>>> <mailto:[hidden email]>
>>>>>>> <mailto:[hidden email]> wrote:
>>>>>>>
>>>>>>> This one also does not look unified:
>>>>>>> http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00/test/com/sun/jdi/LambdaBreakpointTest.java.udiff.html 
>>>>>>>
>>>>>>
>>>>>>> On 3/29/17 11:08, Mikhailo Seledtsov wrote:
>>>>>>>
>>>>>>> One style nit:
>>>>>>>
>>>>>>> LineNumberOnBraceTarg:
>>>>>>> All other Java tests use style of CAP_UNDERSCORE (e.g. STOP_LINE)
>>>>>>> for line number variables, but this test uses 'stopLine'.
>>>>>>> Consider changing it to STOP_LINE (and STOP_LINE_2) to be uniform.
>>>>>>
>>>>>>> On Mar 28, 2017, at 6:37 PM, David Holmes <[hidden email]
>>>>>>> <mailto:[hidden email]>
>>>>>>> <mailto:[hidden email]>
>>>>>>> <mailto:[hidden email]>> wrote:
>>>>>>>
>>>>>>> Two nits:
>>>>>>> - test/com/sun/jdi/FetchLocals.java
>>>>>>> - test/com/sun/jdi/LambdaBreakpointTest.java
>>>>>>>
>>>>>>> Second copyright year should be 2017.
>>>>>>
>>>>>>> On Mar 24, 2017, at 1:56 PM, Igor Ignatyev
>>>>>>> <[hidden email] <mailto:[hidden email]>
>>>>>>> <mailto:[hidden email]>
>>>>>>> <mailto:[hidden email]>> wrote:
>>>>>>> Hi all,
>>>>>>>
>>>>>>> could you please review this fix for 8177507?
>>>>>>>
>>>>>>> due to their nature, some of jdi tests are line number sensitive.
>>>>>>> unfortunately different tests indicate that differently, so it's
>>>>>>> quite
>>>>>>> easy to overlook that and incidentally break tests, for example by
>>>>>>> changing module dependency declaration or license modification.
>>>>>>> this
>>>>>>> fix unifies the way line number sensitivity is indicated and also
>>>>>>> improves readability/maintainability of some tests by using
>>>>>>> constant
>>>>>>> fields instead of magic numbers.
>>>>>>>
>>>>>>> some of line number sensitive tests have been unexpectedly removed
>>>>>>> from execution because they had @test/nodynamiccopyright/
>>>>>>> instead of
>>>>>>> @test tag. this changeset fixes and returns them to regular
>>>>>>> execution.
>>>>>>>
>>>>>>> webrev: http://cr.openjdk.java.net/~iignatyev/8177507/webrev.00
>>>>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8177507
>>>>>>> testing: test/com/sun/jdi
>>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: RFR(M) : 8177507 : line number sensitive tests for jdi should be unified

Igor Ignatyev
Misha, David, Serguei,

Thank you for review!

-- Igor

Loading...