[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR(M) : 8177507 : line number sensitive tests for jdi should be unified
From:       "serguei.spitsyn () oracle ! com" <serguei ! spitsyn () oracle ! com>
Date:       2017-03-29 19:57:35
Message-ID: d1e8fcdc-0644-7d9b-38c4-268548776f09 () oracle ! com
[Download RAW message or body]

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<igor.ignatyev@oracle.com>  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


[prev in list] [next in list] [prev in thread] [next in thread]