Re: RFR 8181299/10, Several jdk tests fail with java.lang.NoClassDefFoundError: jdk/test/lib/process/StreamPumper

[Felix Yang]

Hi Igor,

just noticed your change. It is indeed necessary to clarify what is 
the best practice here.


Thanks,
Felix
On 2017/6/1 11:32, Igor Ignatyev wrote:

Hi Felix,

I have suggested the exact opposite change[1-3] to fix the same problem.

[1] https://bugs.openjdk.java.net/browse/JDK-8181391
[2] 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-June/048012.html
[3] 
http://cr.openjdk.java.net/~iignatyev//8181391/webrev.00/index.html 



Thanks,
-- Igor
On May 31, 2017, at 8:27 PM, Felix Yang > wrote:


Hi Chris and Daniel,

  new webrev with a few of explicit builds than wildcard.

http://cr.openjdk.java.net/~xiaofeya/8181299/webrev.01/ 



Thanks,
Felix
On 2017/5/31 18:20, Chris Hegarty wrote:
On 31 May 2017, at 10:42, Felix Yang > wrote:


Hi there,

   please review the patch to various jdk tests to explicitly 
compiling test libraries and the lib's dependencies. Though it 
could be a jtreg issue (I think so), it is necessary to get the 
tests running firstly.


Bug:

https://bugs.openjdk.java.net/browse/JDK-8181299

Webrev:

http://cr.openjdk.java.net/~xiaofeya/8181299/webrev.00/ 

This may be ok to get the tests running again, but explicit build 
targets

would be better. The contents, and module dependencies, from classes
in the test library are subject to change, so building all classes may
require more modules than in the @modules tags in the test.
With latest webrev, no new @modules introduced by this change, though 
I fixed a missing from original tests.


I prefer to keep "@build jdk.test.lib.process.*" here. Because, with 
current test lib package layout, "@build jdk.test.lib.process.*" 
equals with

/@build jdk.test.lib.process.OutputAnalyzer
//jdk.test.lib.process.OutputBuffer
//jdk.test.lib.process.ProcessTools
jdk.test.lib.process.//StreamPumper//
///jdk.test.lib.process.ExitCode/ /"

It is a bit ugly and not productive, when I only use ProcessTools 
directly but have to declare a bunch of @builds. That is why I think 
this is not a fix but a workaround.


Thanks,
Felix


I agree with Daniel, each test should be run separately in a clean
environment, to verify that it can build the necessary dependencies.
This is actually not the case. I executed repeatedly each test works 
well separately. The problem occurs when there are more and more 
tests using the same test libs.


As stated in the bugs [1] and [2], if there are multi tests using a 
lib, such as ProcessTools, there could be possible collision 
occurring on its dependencies.
For ProcessTools, StreamPumper (ONLY) will be disappear sometimes. It 
looks some dependency classes were treated by jtreg as some-how 
shared, and removed unexpectedly.


[1]https://bugs.openjdk.java.net/browse/JDK-8181299
[2]https://bugs.openjdk.java.net/browse/CODETOOLS-7901986.

Thanks,
Felix
This may be a straight forward way to identify explicit build 
dependencies

and avoid the wildcards.

-Chris.

Re: JDK10: RFR(xxs): 8181207: 8177809 breaks AIX 5.3, 6.1 builds

[Vyom Tewari]

Hi Thomas,

one minor comment apart from what Christoph gave, you can combine the 
the  two explicit assignment statements to one in all three places.


rv = (jlong)sb.st_mtimespec.tv_sec * 1000;

rv += (jlong)sb.st_mtimespec.tv_nsec / 100; to rv= 
(jlong)sb.st_mtimespec.tv_sec * 1000  +(jlong)sb.st_mtimespec.tv_nsec / 100;


Thanks,
Vyom


On Wednesday 31 May 2017 08:59 PM, Thomas Stüfe wrote:

Hi Volker,

Good suggestions! I completely overlooked the ..._n members in stat64
struct. It seems it is even documented:
https://www.ibm.com/support/knowledgecenter/ssw_aix_72/com.ibm.aix.files/stat.h.htm

new webrev:
http://cr.openjdk.java.net/~stuefe/webrevs/8181207-8177809-breaks-AIX-builds/webrev.01/webrev/

..Thomas

On Wed, May 31, 2017 at 10:49 AM, Volker Simonis 
wrote:


Hi Thomas,

as far as I can see, AIX supports both, the st_[a,c,m]time members in
the stat64 structure for seconds and the corresponding
st_[a,c,m]time_n members for nanosecond resolution since at least 5.3.
Can you please use both - there's no reason to discriminate AIX here
:)

Also, can you please change the code such that we have:

#ifdef MACOSX
...
#else
#ifdef AIX
...
#else
...
#endif
#endif

I don't really like using "ifndef XXX" for everything else except XXX.

Thnank you and best regards,
Volker


On Tue, May 30, 2017 at 11:46 AM, Thomas Stüfe 
wrote:

Hi all,

may I have please a review for this tiny change:

Bug: https://bugs.openjdk.java.net/browse/JDK-8181207
webrev:
http://cr.openjdk.java.net/~stuefe/webrevs/8181207-

8177809-breaks-AIX-builds/webrev.00/webrev/

This reverts 8177809 for AIX because it leads to build errors on older

AIX

systems. We want to retain the ability to build on older AIX releases.

Thanks, Thomas

Re: RFR 8181299/10, Several jdk tests fail with java.lang.NoClassDefFoundError: jdk/test/lib/process/StreamPumper

[Igor Ignatyev]

Hi Felix,

I have suggested the exact opposite change[1-3] to fix the same problem.

[1] https://bugs.openjdk.java.net/browse/JDK-8181391
[2] http://mail.openjdk.java.net/pipermail/core-libs-dev/2017-June/048012.html
[3] http://cr.openjdk.java.net/~iignatyev//8181391/webrev.00/index.html

Thanks,
-- Igor
> On May 31, 2017, at 8:27 PM, Felix Yang  wrote:
> 
> Hi Chris and Daniel,
> 
>   new webrev with a few of explicit builds than wildcard.
> 
> http://cr.openjdk.java.net/~xiaofeya/8181299/webrev.01/ 
> 
> 
> Thanks,
> Felix
> On 2017/5/31 18:20, Chris Hegarty wrote:
>>> On 31 May 2017, at 10:42, Felix Yang  wrote:
>>> 
>>> Hi there,
>>> 
>>>please review the patch to various jdk tests to explicitly compiling 
>>> test libraries and the lib's dependencies. Though it could be a jtreg issue 
>>> (I think so), it is necessary to get the tests running firstly.
>>> 
>>> Bug:
>>> 
>>>https://bugs.openjdk.java.net/browse/JDK-8181299
>>> 
>>> Webrev:
>>> 
>>>http://cr.openjdk.java.net/~xiaofeya/8181299/webrev.00/
>> This may be ok to get the tests running again, but explicit build targets
>> would be better. The contents, and module dependencies, from classes
>> in the test library are subject to change, so building all classes may
>> require more modules than in the @modules tags in the test.
> With latest webrev, no new @modules introduced by this change, though I fixed 
> a missing from original tests.
> 
> I prefer to keep "@build jdk.test.lib.process.*" here. Because, with current 
> test lib package layout, "@build jdk.test.lib.process.*" equals with
> /@build jdk.test.lib.process.OutputAnalyzer
> //jdk.test.lib.process.OutputBuffer
> //jdk.test.lib.process.ProcessTools
> jdk.test.lib.process.//StreamPumper//
> ///jdk.test.lib.process.ExitCode/ /"
> 
> It is a bit ugly and not productive, when I only use ProcessTools directly 
> but have to declare a bunch of @builds. That is why I think this is not a fix 
> but a workaround.
> 
> Thanks,
> Felix
>> 
>> I agree with Daniel, each test should be run separately in a clean
>> environment, to verify that it can build the necessary dependencies.
> This is actually not the case. I executed repeatedly each test works well 
> separately. The problem occurs when there are more and more tests using the 
> same test libs.
> 
> As stated in the bugs [1] and [2], if there are multi tests using a lib, such 
> as ProcessTools, there could be possible collision occurring on its 
> dependencies.
> For ProcessTools, StreamPumper (ONLY) will be disappear sometimes. It looks 
> some dependency classes were treated by jtreg as some-how shared, and removed 
> unexpectedly.
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8181299 
> 
> [2] https://bugs.openjdk.java.net/browse/CODETOOLS-7901986 
> .
> 
> Thanks,
> Felix
>> This may be a straight forward way to identify explicit build dependencies
>> and avoid the wildcards.
>> 
>> -Chris.

Re: JDK 10 RFR of JDK-8181126: Refactor shell test java/nio/Buffer/LimitDirectMemory.sh to java

[Amy Lu]

Thank you all for reviewing.

Pushed with minor update as Roger suggested.

Thanks,
Amy

On 5/31/17 5:11 AM, Roger Riggs wrote:

Hi Amy,

LimitDirectMemory.java: 61  - Can you add a usage message to the 
exception identifying the arguments.
 It is good practice to provide a message so when reading a failing 
log, you don't have to go back to the source and stack trace.


LimitDirectMemoryNegativeTest.java:

45:  Add a usage message to the exception like:  "missing size argument"
 it will help someone who later tries to run it from the command line

51 - the test should check that the error message includes the
  offending argument;  append the arg[0] to ERR.

.shouldContain(ERR*+ arg[0]*)

Looks fine, no need for another review.

Thanks, Roger


On 5/26/2017 8:29 PM, Brent Christian wrote:

That all looks fine to me, Amy.

(You'll also need a JDK 10 Reviewer).

Thanks,
-Brent

On 5/25/17 7:42 PM, Amy Lu wrote:

java/nio/Buffer/LimitDirectMemory.sh

Please review this patch to refactor the shell test to java.

bug: https://bugs.openjdk.java.net/browse/JDK-8181126
webrev: http://cr.openjdk.java.net/~amlu/8181126/webrev.00/

Thanks,
Amy

Re: (doc-only) 9 RFR of JDK-8181082: class-level since tag issues in java.base & java.datatransfer module

[Sergey Bylokhov]

Looks fine to me. Thank you for clarification.

- huaming...@oracle.com wrote:

> Ping.
> 
> Thank you
> 
> -Hamlin
> 
> 
> On 2017/5/31 8:13, Hamlin Li wrote:
> > Hi Alan, Sergey,
> >
> > Sorry for delayed reply, I have been on vacation for last few days.
> >
> > I do have a tool (still under development, it's based on javadoc 
> > doclet) to scan versions (currently from 1.7 to 9) of jdk source,
> find 
> > since tag issues, and give suggested since tag versions if feasible.
> 
> > But for older versions of jdk source code, for example 1.6 or older
> 
> > versions, the tool has trouble with them, so for classes added in 
> > older versions, the tool can only detect the issues, but can not
> give 
> > the suggested since tag versions, the changes in this webrev is 
> > generated manually. As for older versions, we only need to fix
> issues 
> > once, so I think it's worth to do the necessary manual check and
> fix.
> >
> > Thank you
> >
> > -Hamlin
> >
> >
> > On 2017/5/27 14:17, Alan Bateman wrote:
> >>
> >>
> >> On 27/05/2017 02:14, Hamlin Li wrote:
> >>> Is someone available to review the change?
> >>>
> >> I skimmed through it,  crossing checking several against the docs
> for 
> >> JDK 1.0, JDK 1.1, and JDK 1.2. I didn't spot any mistakes but I
> agree 
> >> with Sergey, it would be good to know if this was done by creating
> 
> >> the lists from the binaries rather than manual.
> >>
> >> -Alan
> >>
> >>>
> >>>
> >>> On 2017/5/25 17:19, Hamlin Li wrote:
>  Would you please review below patch?
> 
>  bug: https://bugs.openjdk.java.net/browse/JDK-8181082
> 
>  webrev: http://cr.openjdk.java.net/~mli/8181082/webrev.00/
> 
> 
>  Thank you
> 
>  -Hamlin
> 
> >>>
> >>
> >

Re: (doc-only) 9 RFR of JDK-8181082: class-level since tag issues in java.base & java.datatransfer module

[Hamlin Li]

Ping.

Thank you

-Hamlin


On 2017/5/31 8:13, Hamlin Li wrote:

Hi Alan, Sergey,

Sorry for delayed reply, I have been on vacation for last few days.

I do have a tool (still under development, it's based on javadoc 
doclet) to scan versions (currently from 1.7 to 9) of jdk source, find 
since tag issues, and give suggested since tag versions if feasible. 
But for older versions of jdk source code, for example 1.6 or older 
versions, the tool has trouble with them, so for classes added in 
older versions, the tool can only detect the issues, but can not give 
the suggested since tag versions, the changes in this webrev is 
generated manually. As for older versions, we only need to fix issues 
once, so I think it's worth to do the necessary manual check and fix.


Thank you

-Hamlin


On 2017/5/27 14:17, Alan Bateman wrote:



On 27/05/2017 02:14, Hamlin Li wrote:

Is someone available to review the change?

I skimmed through it,  crossing checking several against the docs for 
JDK 1.0, JDK 1.1, and JDK 1.2. I didn't spot any mistakes but I agree 
with Sergey, it would be good to know if this was done by creating 
the lists from the binaries rather than manual.


-Alan




On 2017/5/25 17:19, Hamlin Li wrote:

Would you please review below patch?

bug: https://bugs.openjdk.java.net/browse/JDK-8181082

webrev: http://cr.openjdk.java.net/~mli/8181082/webrev.00/


Thank you

-Hamlin

Re: (10) RFR of JDK-8166142: Refactor java.io.serialization shell tests to java

[Hamlin Li]

Hi Roger,

Thank you for detailed reviewing, fixed as you suggested except below 
comment:


81-83/93/95:  use  SHARE.resolve(xxx).toString to compute the paths.

As the code is constructing a class path with 2 paths rather than 
constructing a path.


new webrev: http://cr.openjdk.java.net/~mli/8166142/webrev.03/

Thank you

-Hamlin


On 2017/6/1 0:47, Roger Riggs wrote:

Hi Hamlin,

RenamePackageTest.java:
  - 48,61:  rename "sutup" -> "setup"

81-83/93/95:  use  SHARE.resolve(xxx).toString to compute the paths.

ClassPathTest.java:
42: add a space before "{"

56:  Generally, exception messages should not end in "."; they are phrases
so they could be printed with context.
In this specific case it is not important, just a pattern to follow 
consistently.


Thanks, Roger

On 5/26/2017 9:14 PM, Hamlin Li wrote:

Hi Roger,

Thank you for catching it, new webrev: 
http://cr.openjdk.java.net/~mli/8166142/webrev.02/


Thank you

-Hamlin


On 2017/5/27 3:30, Roger Riggs wrote:

Hi Hamlin,

SubclassTest.java:37: typo" excepiton" -> exception


SubclassDataLossTest.java:
103/104:  Adding the class loader close() calls isn't very effective 
since if there is an exception they are not
closed and the creation in a static initializer is mismatched with 
main() code.


It would be cleaner to open in main() using try-with-resources and 
close them in a finally clause.

And pass the loaders to the test function.
But for simplicity, perhaps just leave out the new calls to 
ldr1/2.close.


(There is something odd about the webrev links, the nagivation links 
don't work as expected. but that's transient)


Thanks, Roger



On 5/26/2017 12:26 AM, Hamlin Li wrote:

Hi Roger,

Thank you for detailed review. Fixed as you suggested, new webrev: 
http://cr.openjdk.java.net/~mli/8166142/webrev.01/


Thank you

-Hamlin


On 2017/5/26 2:54, Roger Riggs wrote:

Hi Hamlin,

For imports they should import specific classes, wildcards are not 
used.


maskSyntheticModifier/Test.java
consTest/Test.java
deserializeButton/Test.java

 test/java/io/Serializable/packageAccess/Driver.java: the name 
"Driver" is not descriptive and should be.


Each Driver.java should have a different and 
functional/descriptive name.


Better yet, there should be a single program that creates jar 
files using command line arguments

that can be invoked depending on the test.

There is a mix of Driver's copying files vs the test program 
copying; can that be made more uniform.


Test.java files should have descriptive/functional names. (Even if 
the duplicating the directory)


Regards, Roger


On 5/24/2017 5:09 AM, Hamlin Li wrote:

Would you please review the below patch?

bug: https://bugs.openjdk.java.net/browse/JDK-8166142

webrev: http://cr.openjdk.java.net/~mli/8166142/webrev.00/


Thank you

-Hamlin

Re: RFR 9+-: 8180582: The bind to rmiregistry is rejected by registryFilter even though registryFilter is set

[Stuart Marks]

Hi Roger,

RegistryImpl change looks fine. I have a quibble on the regex used in the test.

 162 Matcher m = Pattern
 163 .compile(".*maxdepth=(\\d*)")
 164 .matcher(Objects.requireNonNullElse(registryFilter, ""));
 165 int depthOverride = m.find() ? Integer.parseInt(m.group(1)) : 
REGISTRY_MAX_DEPTH;


Since Matcher.find() is called, the pattern can be found anywhere in the input 
string, so the leading ".*" is unnecessary. I'd suggest replacing it with "\\b" 
to match a word boundary immediately preceding "maxdepth". Also, "\\d*" will 
match zero characters, which could cause the subsequent parseInt() to fail.


These issues aren't of immediate concern, as the test works as it stands. They 
may make the test a bit harder to maintain, though.


But I think the main concern is that if the regex were modified by future 
maintenance, it might not match, which would cause the test to test 
REGISTRY_MAX_DEPTH redundantly instead of the depth the filter is attempting to 
specify. This problem wouldn't be completely silent, but it'd be easy to miss. 
You'd have to inspect the log pretty carefully to see what's going on. So maybe 
it would be good to have something like a command-line argument that specifies 
the expected value of maxdepth. Yes, this is redundant, but it's a useful 
cross-check, I think.


There are several identical lines of code starting at line 184. Maybe these 
could be extracted into a common method.


These are all cleanup issues, so it's up to you how much additional work you 
want to put into this before it gets into 9.


s'marks



On 5/31/17 7:02 AM, Roger Riggs wrote:

Thanks Daniel for the review and corrected link.

On 5/30/2017 7:04 PM, Daniel Fuchs wrote:

Hi Roger,

On 30/05/17 19:26, Roger Riggs wrote:

Hi Daniel,

Fixed, there is no need for the unbind since the registry is not reused and
it might
cause a spurious success.

(Also corrected the exception error message @ 151 and 153).

Webrev updated in place.
...


I assume you meant
http://cr.openjdk.java.net/~rriggs/webrev-depth-max-8180582/index.html
which is what I reviewed ;-)

Looks good!

-- danel



Thanks, Roger


On 5/30/2017 2:00 PM, Daniel Fuchs wrote:

Hi Roger,

Looks good! Just one nit with the test:

 194 registry.unbind(name);
 195 Assert.fail("Registry filter should have rejected depth: "
+ depthOverride + 1);

Technically, the test will also pass if bind() succeed but
unbind() fails - for some unforeseen reason.

best regards,

-- daniel

On 25/05/17 16:31, Roger Riggs wrote:

Please review an update to the RMI Registry built-in serial filter
implementation limit on the depth
of a instance that can be bound in the registry.  The initial depth limit
was 5 and it was too limiting
for some existing applications.  It limit is updated to depth = 20 and
should be more than adequate.

Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-depth-max-8180582/index.html

Issue:
https://bugs.openjdk.java.net/browse/JDK-8180582

Thanks, Roger

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

[Igor Ignatyev]

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

Re: RFR(S) : 8180898: remove JavaToolUtils testlibrary class

[Alan Bateman]

On 31/05/2017 19:04, Igor Ignatyev wrote:

http://cr.openjdk.java.net/~iignatyev//8180898/webrev.00/index.html


Looks okay to me.

RFR(S) : 8180898: remove JavaToolUtils testlibrary class

[Igor Ignatyev]

http://cr.openjdk.java.net/~iignatyev//8180898/webrev.00/index.html
> 209 lines changed: 0 ins; 209 del; 0 mod; 0

Hi all,

8180895[1] has updated the sole user of JavaToolUtils class to use other 
testlibrary classes, JavaToolUtils itself does not provide any unique methods, 
so it should be removed.

webrev: http://cr.openjdk.java.net/~iignatyev//8180898/webrev.00/index.html
jbs: https://bugs.openjdk.java.net/browse/JDK-8180898
testing: grep -rl JavaToolUtils ./jdk/test

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

Thanks,
-- Igor

Re: RFR: JDK-8176784 - Amend HREF to technote/guides in CORBA API docs to unilinks for guides

[Mark Sheppard]

thanks Roger ...
 i'll make it consistent and add the  tag in this instance and 
check for others

occurrences, also.

I used the same anchor text for the translation to @extLink

regards
Mark

On 31/05/2017 18:11, Roger Riggs wrote:

Hi Mark,

org/omg/CORBA/package.html:  for consistency should

{@extLink jidlexception Java IDL exceptions }

be the same as others:

{@extLink jidlexception documentation on JavaIDL exceptions}

I'm not sure if the  is really needed.  I'd remove it if not 
necessary.


Otherwise looks fine.

Roger



On 5/31/2017 12:43 PM, Mark Sheppard wrote:

Hi,
   please oblige and review the following changes
http://cr.openjdk.java.net/~msheppar/8176784-IV/webrev.02/

which address the issue raised in
https://bugs.openjdk.java.net/browse/JDK-8176784

this results in changes to  href in the corba javadoc, such that the
relative links to technote/guides html pages have been
replaces with the @extLink annotation. This will result in a
URL query link being created to replace the technote links, when
the javadoc is generated.

regards
Mark

Re: RFR: JDK-8176784 - Amend HREF to technote/guides in CORBA API docs to unilinks for guides

[Roger Riggs]

Hi Mark,

org/omg/CORBA/package.html:  for consistency should

{@extLink jidlexception Java IDL exceptions }

be the same as others:

{@extLink jidlexception documentation on JavaIDL exceptions}

I'm not sure if the  is really needed.  I'd remove it if not 
necessary.


Otherwise looks fine.

Roger



On 5/31/2017 12:43 PM, Mark Sheppard wrote:

Hi,
   please oblige and review the following changes
http://cr.openjdk.java.net/~msheppar/8176784-IV/webrev.02/

which address the issue raised in
https://bugs.openjdk.java.net/browse/JDK-8176784

this results in changes to  href in the corba javadoc, such that the
relative links to technote/guides html pages have been
replaces with the @extLink annotation. This will result in a
URL query link being created to replace the technote links, when
the javadoc is generated.

regards
Mark

Re: (10) RFR of JDK-8166142: Refactor java.io.serialization shell tests to java

[Roger Riggs]

Hi Hamlin,

RenamePackageTest.java:
  - 48,61:  rename "sutup" -> "setup"

81-83/93/95:  use  SHARE.resolve(xxx).toString to compute the paths.

ClassPathTest.java:
42: add a space before "{"

56:  Generally, exception messages should not end in "."; they are phrases
so they could be printed with context.
In this specific case it is not important, just a pattern to follow 
consistently.


Thanks, Roger

On 5/26/2017 9:14 PM, Hamlin Li wrote:

Hi Roger,

Thank you for catching it, new webrev: 
http://cr.openjdk.java.net/~mli/8166142/webrev.02/


Thank you

-Hamlin


On 2017/5/27 3:30, Roger Riggs wrote:

Hi Hamlin,

SubclassTest.java:37: typo" excepiton" -> exception


SubclassDataLossTest.java:
103/104:  Adding the class loader close() calls isn't very effective 
since if there is an exception they are not
closed and the creation in a static initializer is mismatched with 
main() code.


It would be cleaner to open in main() using try-with-resources and 
close them in a finally clause.

And pass the loaders to the test function.
But for simplicity, perhaps just leave out the new calls to 
ldr1/2.close.


(There is something odd about the webrev links, the nagivation links 
don't work as expected. but that's transient)


Thanks, Roger



On 5/26/2017 12:26 AM, Hamlin Li wrote:

Hi Roger,

Thank you for detailed review. Fixed as you suggested, new webrev: 
http://cr.openjdk.java.net/~mli/8166142/webrev.01/


Thank you

-Hamlin


On 2017/5/26 2:54, Roger Riggs wrote:

Hi Hamlin,

For imports they should import specific classes, wildcards are not 
used.


maskSyntheticModifier/Test.java
consTest/Test.java
deserializeButton/Test.java

 test/java/io/Serializable/packageAccess/Driver.java: the name 
"Driver" is not descriptive and should be.


Each Driver.java should have a different and functional/descriptive 
name.


Better yet, there should be a single program that creates jar files 
using command line arguments

that can be invoked depending on the test.

There is a mix of Driver's copying files vs the test program 
copying; can that be made more uniform.


Test.java files should have descriptive/functional names. (Even if 
the duplicating the directory)


Regards, Roger


On 5/24/2017 5:09 AM, Hamlin Li wrote:

Would you please review the below patch?

bug: https://bugs.openjdk.java.net/browse/JDK-8166142

webrev: http://cr.openjdk.java.net/~mli/8166142/webrev.00/


Thank you

-Hamlin

RFR: JDK-8176784 - Amend HREF to technote/guides in CORBA API docs to unilinks for guides

[Mark Sheppard]

Hi,
   please oblige and review the following changes
http://cr.openjdk.java.net/~msheppar/8176784-IV/webrev.02/

which address the issue raised in
https://bugs.openjdk.java.net/browse/JDK-8176784

this results in changes to  href in the corba javadoc, such that the
relative links to technote/guides html pages have been
replaces with the @extLink annotation. This will result in a
URL query link being created to replace the technote links, when
the javadoc is generated.

regards
Mark

Re: JDK10: RFR(xxs): 8181207: 8177809 breaks AIX 5.3, 6.1 builds

[Thomas Stüfe]

Hi Volker,

Good suggestions! I completely overlooked the ..._n members in stat64
struct. It seems it is even documented:
https://www.ibm.com/support/knowledgecenter/ssw_aix_72/com.ibm.aix.files/stat.h.htm

new webrev:
http://cr.openjdk.java.net/~stuefe/webrevs/8181207-8177809-breaks-AIX-builds/webrev.01/webrev/

..Thomas

On Wed, May 31, 2017 at 10:49 AM, Volker Simonis 
wrote:

> Hi Thomas,
>
> as far as I can see, AIX supports both, the st_[a,c,m]time members in
> the stat64 structure for seconds and the corresponding
> st_[a,c,m]time_n members for nanosecond resolution since at least 5.3.
> Can you please use both - there's no reason to discriminate AIX here
> :)
>
> Also, can you please change the code such that we have:
>
> #ifdef MACOSX
> ...
> #else
> #ifdef AIX
> ...
> #else
> ...
> #endif
> #endif
>
> I don't really like using "ifndef XXX" for everything else except XXX.
>
> Thnank you and best regards,
> Volker
>
>
> On Tue, May 30, 2017 at 11:46 AM, Thomas Stüfe 
> wrote:
> > Hi all,
> >
> > may I have please a review for this tiny change:
> >
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8181207
> > webrev:
> > http://cr.openjdk.java.net/~stuefe/webrevs/8181207-
> 8177809-breaks-AIX-builds/webrev.00/webrev/
> >
> > This reverts 8177809 for AIX because it leads to build errors on older
> AIX
> > systems. We want to retain the ability to build on older AIX releases.
> >
> > Thanks, Thomas
>

Re: RFR(XXS) : 8181307: tests added/changed by 8166139 should be updated to use the latest testlibrary

[Roger Riggs]

+1, looks good.

Roger

On 5/31/2017 11:19 AM, Igor Ignatyev wrote:

http://cr.openjdk.java.net/~iignatyev//8181307/webrev.00/index.html

2 lines changed: 1 ins; 0 del; 1 mod;

Hi all,

misfortunately, 8180887[1] was created and tested before 8166139[2] has been 
integrated. then the fixes met in jdk10/jdk10, 
java/net/URLClassLoader/closetest/CloseTest.java started to fail due to 'Error. 
can't find jdk.testlibrary.FileUtils in test directory or libraries'. the test 
should be updated to use /test/lib library and not to build removed 
jdk.testlibrary.FileUtils.

jbs: https://bugs.openjdk.java.net/browse/JDK-8181307
webrev: http://cr.openjdk.java.net/~iignatyev//8181307/webrev.00/index.html
testing: java/net tests


[1] https://bugs.openjdk.java.net/browse/JDK-8180887
[2] https://bugs.openjdk.java.net/browse/JDK-8166139

Thanks.
-- Igor

Re: jdk9 vs. jdk8 : TimeZone getDisplayName(Locale.GERMAN) for TZ=MET

[dalibor topic]

On 31.05.2017 17:11, Baesken, Matthias wrote:


I wonder why the display names for timezone MET changed ( I observed this on 
Linux and Solaris) when I compare
jdk8 to jdk9 .

The test is very small, it just outputs for Locale.GERMAN  the display name.
With jdk8 we get a translated german timezone name, but not with jdk9  (this is 
a bit strange because  the jdk9 rsource file
src/jdk.localedata/share/classes/sun/util/resources/ext/TimeZoneNames_de.java
still contains the translation).

Is this intentional  or a bug ?


Hi Matthias,

it might be a side effect of http://openjdk.java.net/jeps/252 .

cheers,
dalibor topic


Best regards, Matthias



TimeZoneTest.java

import java.util.TimeZone;
import java.util.Locale;

public class TimeZoneTest {

public static void main(String[] args)
{
TimeZone tz = TimeZone.getDefault();
// now test for german
String dng = tz.getDisplayName(Locale.GERMAN);
System.out.println("timezone display name for Locale.GERMAN " + dng);
}
}


for environment variable TZ=MET  (export TZ=MET) we get :
../output-jdk8/images/j2sdk-image/bin/java  TimeZoneTest
timezone display name for Locale.GERMAN Zentraleuropäische Zeit

../output-jdk9/images/jdk/bin/java  TimeZoneTest
timezone display name for Locale.GERMAN Middle Europe Time



--
 Dalibor Topic | Principal Product Manager
Phone: +494089091214  | Mobile: +491737185961


ORACLE Deutschland B.V. & Co. KG | Kühnehöfe 5 | 22761 Hamburg

ORACLE Deutschland B.V. & Co. KG
Hauptverwaltung: Riesstr. 25, D-80992 München
Registergericht: Amtsgericht München, HRA 95603

Komplementärin: ORACLE Deutschland Verwaltung B.V.
Hertogswetering 163/167, 3543 AS Utrecht, Niederlande
Handelsregister der Handelskammer Midden-Niederlande, Nr. 30143697
Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher

 Oracle is committed to developing
practices and products that help protect the environment

RFR(XXS) : 8181307: tests added/changed by 8166139 should be updated to use the latest testlibrary

[Igor Ignatyev]

http://cr.openjdk.java.net/~iignatyev//8181307/webrev.00/index.html
> 2 lines changed: 1 ins; 0 del; 1 mod;

Hi all,

misfortunately, 8180887[1] was created and tested before 8166139[2] has been 
integrated. then the fixes met in jdk10/jdk10, 
java/net/URLClassLoader/closetest/CloseTest.java started to fail due to 'Error. 
can't find jdk.testlibrary.FileUtils in test directory or libraries'. the test 
should be updated to use /test/lib library and not to build removed 
jdk.testlibrary.FileUtils. 

jbs: https://bugs.openjdk.java.net/browse/JDK-8181307
webrev: http://cr.openjdk.java.net/~iignatyev//8181307/webrev.00/index.html
testing: java/net tests


[1] https://bugs.openjdk.java.net/browse/JDK-8180887
[2] https://bugs.openjdk.java.net/browse/JDK-8166139

Thanks.
-- Igor

jdk9 vs. jdk8 : TimeZone getDisplayName(Locale.GERMAN) for TZ=MET

[Baesken, Matthias]

I wonder why the display names for timezone MET changed ( I observed this on 
Linux and Solaris) when I compare
jdk8 to jdk9 .

The test is very small, it just outputs for Locale.GERMAN  the display name.
With jdk8 we get a translated german timezone name, but not with jdk9  (this is 
a bit strange because  the jdk9 rsource file
src/jdk.localedata/share/classes/sun/util/resources/ext/TimeZoneNames_de.java
still contains the translation).

Is this intentional  or a bug ?

Best regards, Matthias



TimeZoneTest.java

import java.util.TimeZone;
import java.util.Locale;

public class TimeZoneTest {

public static void main(String[] args)
{
TimeZone tz = TimeZone.getDefault();
// now test for german
String dng = tz.getDisplayName(Locale.GERMAN);
System.out.println("timezone display name for Locale.GERMAN " + dng);
}
}


for environment variable TZ=MET  (export TZ=MET) we get :
../output-jdk8/images/j2sdk-image/bin/java  TimeZoneTest
timezone display name for Locale.GERMAN Zentraleuropäische Zeit

../output-jdk9/images/jdk/bin/java  TimeZoneTest
timezone display name for Locale.GERMAN Middle Europe Time

Re: RFR(S) : 8180805 : move RandomFactory to the top level testlibrary

[Brian Burkhalter]

Hi Igor,

On May 31, 2017, at 7:23 AM, Igor Ignatyev  wrote:

> thank you for your review.

You’re welcome.

> regarding '@Deprecated(since=“9”, forRemoval=true)', since the only users of 
> testlibrary classes are our own tests and we do not need to inform anyone or 
> wait for a new release to remove them, I don't think it makes much sense to 
> have such attributes.

I think it’s fine: I was just pointing out a possibility.

> PS sorry, by some reason the email unnoticeably stuck in my outbox, and I 
> have already pushed the patch. if you have any further comments, I will 
> address them by a separate fix.

No, it’s good.

Thanks,

Brian

Re: RFR 9: (doclint) 8181156 html5 doclint issues in java.base javadoc

[Alan Bateman]

On 31/05/2017 15:34, Roger Riggs wrote:

Please review javadoc markup change to update to html5 acceptable to 
doclint.

The table formatting is updated to html5 markup.

The Docs build is updated to require doclint html5.

Webrev:
  jdk: http://cr.openjdk.java.net/~rriggs/webrev-base-html5-8181156/
  build: http://cr.openjdk.java.net/~rriggs/webrev-top-html5-8181156/
Some of the lines are way too long now and will means lots of horizontal 
scrolling when looking at diffs in the future. Can these be cleaned up 
before these changes are pushed?


-Alan

RFR 9: (doclint) 8181156 html5 doclint issues in java.base javadoc

[Roger Riggs]

Please review javadoc markup change to update to html5 acceptable to 
doclint.

The table formatting is updated to html5 markup.

The Docs build is updated to require doclint html5.

Webrev:
  jdk: http://cr.openjdk.java.net/~rriggs/webrev-base-html5-8181156/
  build: http://cr.openjdk.java.net/~rriggs/webrev-top-html5-8181156/

Issue:
https://bugs.openjdk.java.net/browse/JDK-8181156 html5 doclint issues in 
java.base javadoc


Thanks, Roger

Re: RFR(S) : 8180805 : move RandomFactory to the top level testlibrary

[Igor Ignatyev]

Hi Brian,

thank you for your review.

regarding '@Deprecated(since=“9”, forRemoval=true)', since the only users of 
testlibrary classes are our own tests and we do not need to inform anyone or 
wait for a new release to remove them, I don't think it makes much sense to 
have such attributes.

PS sorry, by some reason the email unnoticeably stuck in my outbox, and I have 
already pushed the patch. if you have any further comments, I will address them 
by a separate fix.

Thanks,
-- Igor

> On May 30, 2017, at 9:36 AM, Brian Burkhalter  
> wrote:
> 
> Hi Igor,
> 
> On May 26, 2017, at 11:30 PM, Igor Ignatyev  > wrote:
> 
>> this changeset introduces jdk.test.lib.RandomFactory in the top level 
>> testlibrary and updates all but java/time/ tests to use it instead of 
>> jdk.testlibrary.RandomFactory from the jdk testlibrary.
> 
> Code changes look OK provided all tests still run properly. I have not been 
> following the test package refactoring however so no comment on that.
> 
>> due to CODETOOLS-7901987[1], java/time/ tests can't use the top level 
>> testlibrary, so jdk.testlibrary.RandomFactory hasn't been removed, however 
>> it has been marked as deprecated and will be removed by JDK-8181118[2] as 
>> soon as there is promoted jtreg w/ CODETOOLS-7901987 fixed.
> 
> Should this be using “@Deprecated(since=“9”, forRemoval=true)” or something 
> like that?
> 
> Thanks,
> 
> Brian

Re: RFR(S) : 8180805 : move RandomFactory to the top level testlibrary

[Igor Ignatyev]

Hi Alan,

we believe it's a jtreg bug. we have seen similar intermittent failures in 
hotspot testing[1], Ioi(cc'ed) did a really great job analyzing the root 
cause[2], in to words jtreg puts a part of testlibrary to one path and another 
part to a different path. [3] is his suggested fix for jtreg.

-- Igor 

[1] https://bugs.openjdk.java.net/browse/CODETOOLS-7901986
[2] 
https://bugs.openjdk.java.net/secure/attachment/70197/jtreg_random_class_not_found.txt
[3] http://cr.openjdk.java.net/~iklam/jtreg/7901986_split_library/
  
> On May 31, 2017, at 1:38 AM, Alan Bateman  wrote:
> 
> On 31/05/2017 09:05, Felix Yang wrote:
> 
>> Hi Alan
>> 
>>even with explicit compilation, I also observed failures. I'm curious 
>> what is the best practice here. IMO, there could be a potential jtreg bug.
> One of the tests listed in JDK-8181299 is 
> java/net/URLConnection/6212146/TestDriver.java. In jdk10/jdk10 the test 
> description has:
> 
> @build jdk.test.lib.JDKToolFinder jdk.test.lib.process.ProcessTools Test
> 
> but the test fails intermittently with:
> 
> java.lang.NoClassDefFoundError: jdk/test/lib/process/StreamPumper
> at jdk.test.lib.process.ProcessTools.getOutput(ProcessTools.java:85)
> at jdk.test.lib.process.OutputAnalyzer.(OutputAnalyzer.java:47)
> at jdk.test.lib.process.ProcessTools.executeProcess(ProcessTools.java:397)
> at jdk.test.lib.process.ProcessTools.executeProcess(ProcessTools.java:425)
> at jdk.test.lib.process.ProcessTools.executeCommand(ProcessTools.java:475)
> at TestDriver.main(TestDriver.java:61)
> 
> I assume changing the @build to jdk.test.lib.process.* will fix this, 
> assuming all the classes needed for ProcessTools are in this package.
> 
> As to why it's intermittent then I think it's a side effect of test library 
> classes being compiled by one test and then re-used by a test that runs 
> sometime later in the same VM. Combine that with implicit compilation, varied 
> @build usages, and concurrently should explain why it's intermittent. I see 
> you've cc'ed Jon Gibbons and he is the best person to comment on this. Now 
> seems a good time to get to the bottom of these issues, esp. with Igor 
> changing lots of tests to drop the explicit compilation of test library 
> classes.
> 
> -Alan

Re: RFR 9+-: 8180582: The bind to rmiregistry is rejected by registryFilter even though registryFilter is set

[Roger Riggs]

Thanks Daniel for the review and corrected link.

On 5/30/2017 7:04 PM, Daniel Fuchs wrote:

Hi Roger,

On 30/05/17 19:26, Roger Riggs wrote:

Hi Daniel,

Fixed, there is no need for the unbind since the registry is not 
reused and it might

cause a spurious success.

(Also corrected the exception error message @ 151 and 153).

Webrev updated in place.
...


I assume you meant
http://cr.openjdk.java.net/~rriggs/webrev-depth-max-8180582/index.html
which is what I reviewed ;-)

Looks good!

-- danel



Thanks, Roger


On 5/30/2017 2:00 PM, Daniel Fuchs wrote:

Hi Roger,

Looks good! Just one nit with the test:

 194 registry.unbind(name);
 195 Assert.fail("Registry filter should have rejected 
depth: " + depthOverride + 1);


Technically, the test will also pass if bind() succeed but
unbind() fails - for some unforeseen reason.

best regards,

-- daniel

On 25/05/17 16:31, Roger Riggs wrote:
Please review an update to the RMI Registry built-in serial filter 
implementation limit on the depth
of a instance that can be bound in the registry.  The initial depth 
limit was 5 and it was too limiting
for some existing applications.  It limit is updated to depth = 20 
and should be more than adequate.


Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-depth-max-8180582/index.html

Issue:
https://bugs.openjdk.java.net/browse/JDK-8180582

Thanks, Roger

Re: RFR: 8176508 Update JAX-WS RI integration to latest version

[Roman Grigoriadi]

Hi,

New webrev can be found here:
http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/02/ 


Previous review comments have been addressed.

New changes since last webrev upload:

Few “opens xxx to java.xml.bind” in the module-info of java.xml.ws with 
descriptions of files in java.xml.ws, which calls JAXBContext#newInstance. 

JAXB-API:
 - JAXBContext.java - deprecated implementation discovery with jaxb.properties 
resource. 
 - ContextFinder when called with String contextPath now tries to resolve 
jaxb.properties with Class#getResource if ClassLoader#getResource fails due to 
insufficient openness of jaxb.properties resource package.
 - better JAXBException message when package of jaxb classes is not open to 
java.xml.bind

JAXB-RI:
 - fixed escaping newlines when using bundled jaxws transport.

SAAJ-RI
 - fixed TCK test failures

JAXWS-RI
 - fixed parsing wsdl in secure mode

We have one JCK runtime test failure, which should be probably fixed in tests, 
I have created issue for it: https://bugs.openjdk.java.net/browse/JCK-7308397 


Please review.

Thanks,
Roman

> On 8 May 2017, at 22:38, Lance Andersen  wrote:
> 
> Hi Roman,
> 
> I made a pass through the webrev and have the following feedback:
> 
> 
> src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/soap/StaxLazySourceBridge.java
>  and several files  - which follow in the webrev, have formatting issues with 
> the newly added @override and existing @overrides and should probably be 
> cleaned up
> 
> src/java.xml.ws/share/classes/com/sun/xml/internal/messaging/saaj/soap/impl/BodyElementImpl.java
>  - can 960 -962 be deleted
> 
> src/java.xml.ws/share/classes/com/sun/xml/internal/ws/util/version.properties 
> - The copyright was reverted.  Also what happens to the svn info in this file 
> with the move to github?
> 
> src/java.xml.ws/share/classes/javax/xml/soap/package-info.java -  I would use 
>  for TM
> 
> src/java.xml.ws/share/classes/javax/xml/ws/Service.java - See comments 
> starting at 230 seem off
> 
> src/java.xml.ws/share/classes/javax/xml/ws/WebServiceRef.java -  I would make 
> the comments starting at 139 be consistent with the other comments 
> 
> src/jdk.xml.bind/share/classes/com/sun/tools/internal/jxc/*.properties - the 
> copyright date was reverted
> 
> src/jdk.xml.bind/share/classes/module-info.java  should already be updated in 
> the workspace
> 
> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/processor/ProcessorException.java
>  - The copyright should be updated to 2017
> 
> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/util/WSDLParseException.java
>  -  the copyright was reverted
> 
> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/wsdl/framework/ParseException.java
>  - the copyright was reverted 
> 
> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/wsdl/framework/ValidationException.java
>  - the copyright was reverted
> 
> src/jdk.xml.ws/share/classes/module-info.java - this was already updated in 
> the workspace
> 
> src/java.xml.bind/share/classes/javax/xml/bind/ModuleUtil.java - the 
> copyright should only be 2017
> 
> src/java.xml.ws/share/classes/com/sun/xml/internal/ws/policy/sourcemodel/attach/ContextClassloaderLocalMessages.java.
>  - the copyright should only be 2017
> 
> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/resources/NewmessagesMessages.java
>  - the copyright should only be 2017
> 
> src/jdk.xml.ws/share/classes/com/sun/tools/internal/ws/resources/newmessages.properties
>  - this is in the workspace already
> 
> 
> 
>> On May 3, 2017, at 12:49 PM, Roman Grigoriadi > > wrote:
>> 
>> Hi,
>> 
>> you can find new web rev here:
>> http://cr.openjdk.java.net/~aefimov/jaxws-integrations/8176508/01/ 
>>  
>> > >
>> 
>> Previous review comments are addressed. 
>> 
>>> The change to version.properties reminds me to ask if there is anything in 
>>> the jaxws repo to indicate the version of the JAX-* components? It's often 
>>> difficult to determine what bits are in the JDK vs. the upstream project.
>> 
>> 
>> Version as in our Maven project is 2.3.0-SNAPSHOT for JAX-WS at the time we 
>> are syncing. Subcomponents (SAAJ, JAXB mainly) are promoted, for example 
>> in 
>> jdk/jaxws/src/jdk.xml.bind/share/classes/com/sun/tools/internal/xjc/MessageBundle.properties
>> There is:
>> # for JDK integration - include version in source zip
>> jaxb.jdk.version=2.3.0-b170412.1723
>> 
>> We can add another version.properties file with versions of all JAX-* 
>> components. We may also change version from 2.3.0-SNAPSHOT to something 
>> unique 

Re: Proposal: javax.naming.spi.NamingManager.clearInitialContextFactoryBuilder()

[dalibor topic]

On 18.05.2017 00:20, Andrew Guibert wrote:


Hi Alan, I've checked within IBM and it appears we do not have any OpenJDK
committers.  Still trying to chase down if we have anyone in IBM who can
drive this commit.  Do you know of anyone?



Hi Andrew,

you can find a list of JDK 10 Committers at 
http://openjdk.java.net/census#jdk10 . You can cross reference it with 
the list at http://db.openjdk.java.net/people to lookup their 
organizations, if any.


cheers,
dalibor topic

--
 Dalibor Topic | Principal Product Manager
Phone: +494089091214  | Mobile: +491737185961


ORACLE Deutschland B.V. & Co. KG | Kühnehöfe 5 | 22761 Hamburg

ORACLE Deutschland B.V. & Co. KG
Hauptverwaltung: Riesstr. 25, D-80992 München
Registergericht: Amtsgericht München, HRA 95603

Komplementärin: ORACLE Deutschland Verwaltung B.V.
Hertogswetering 163/167, 3543 AS Utrecht, Niederlande
Handelsregister der Handelskammer Midden-Niederlande, Nr. 30143697
Geschäftsführer: Alexander van der Ven, Jan Schultheiss, Val Maher

 Oracle is committed to developing
practices and products that help protect the environment

Re: RFR 8181299/10, Several jdk tests fail with java.lang.NoClassDefFoundError: jdk/test/lib/process/StreamPumper

[Chris Hegarty]

> On 31 May 2017, at 10:42, Felix Yang  wrote:
> 
> Hi there,
> 
>please review the patch to various jdk tests to explicitly compiling test 
> libraries and the lib's dependencies. Though it could be a jtreg issue (I 
> think so), it is necessary to get the tests running firstly.
> 
> Bug:
> 
>https://bugs.openjdk.java.net/browse/JDK-8181299
> 
> Webrev:
> 
>http://cr.openjdk.java.net/~xiaofeya/8181299/webrev.00/

This may be ok to get the tests running again, but explicit build targets
would be better. The contents, and module dependencies, from classes
in the test library are subject to change, so building all classes may 
require more modules than in the @modules tags in the test.

I agree with Daniel, each test should be run separately in a clean
environment, to verify that it can build the necessary dependencies.
This may be a straight forward way to identify explicit build dependencies
and avoid the wildcards.

-Chris.

Re: RFR 8181299/10, Several jdk tests fail with java.lang.NoClassDefFoundError: jdk/test/lib/process/StreamPumper

[Daniel Fuchs]

Hi Felix,

Looks good to me. Did you try to run each test in isolation
(removing JT* between 2 runs) to check that they compile
all they need?

If you haven't committed yet then I guess a little
  for f in $(hg st . -nma) ; do \
  rm -r JT* ; \
  jtreg -verbose:summary -jdk  $f ; \
  done
might do the trick ;-)

best regards,

-- daniel

On 31/05/2017 10:42, Felix Yang wrote:

Hi there,

 please review the patch to various jdk tests to explicitly 
compiling test libraries and the lib's dependencies. Though it could be 
a jtreg issue (I think so), it is necessary to get the tests running 
firstly.


Bug:

 https://bugs.openjdk.java.net/browse/JDK-8181299

Webrev:

 http://cr.openjdk.java.net/~xiaofeya/8181299/webrev.00/

Thanks,

Felix

RFR 8181299/10, Several jdk tests fail with java.lang.NoClassDefFoundError: jdk/test/lib/process/StreamPumper

[Felix Yang]

Hi there,

please review the patch to various jdk tests to explicitly 
compiling test libraries and the lib's dependencies. Though it could be 
a jtreg issue (I think so), it is necessary to get the tests running 
firstly.


Bug:

https://bugs.openjdk.java.net/browse/JDK-8181299

Webrev:

http://cr.openjdk.java.net/~xiaofeya/8181299/webrev.00/

Thanks,

Felix

Re: JDK10: RFR(xxs): 8181207: 8177809 breaks AIX 5.3, 6.1 builds

[Volker Simonis]

Hi Thomas,

as far as I can see, AIX supports both, the st_[a,c,m]time members in
the stat64 structure for seconds and the corresponding
st_[a,c,m]time_n members for nanosecond resolution since at least 5.3.
Can you please use both - there's no reason to discriminate AIX here
:)

Also, can you please change the code such that we have:

#ifdef MACOSX
...
#else
#ifdef AIX
...
#else
...
#endif
#endif

I don't really like using "ifndef XXX" for everything else except XXX.

Thnank you and best regards,
Volker


On Tue, May 30, 2017 at 11:46 AM, Thomas Stüfe  wrote:
> Hi all,
>
> may I have please a review for this tiny change:
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8181207
> webrev:
> http://cr.openjdk.java.net/~stuefe/webrevs/8181207-8177809-breaks-AIX-builds/webrev.00/webrev/
>
> This reverts 8177809 for AIX because it leads to build errors on older AIX
> systems. We want to retain the ability to build on older AIX releases.
>
> Thanks, Thomas

Re: JDK 10 RFR of JDK-8181301: Refactor shell test AsynchronousChannelGroup/run_any_task.sh to java

[Alan Bateman]

On 31/05/2017 09:10, Amy Lu wrote:


Thank you Alan!

Webrev updated according to the suggestions:
http://cr.openjdk.java.net/~amlu/8181301/webrev.01


Looks good.

Re: RFR(S) : 8180805 : move RandomFactory to the top level testlibrary

[Alan Bateman]

On 31/05/2017 09:05, Felix Yang wrote:


Hi Alan

even with explicit compilation, I also observed failures. I'm 
curious what is the best practice here. IMO, there could be a 
potential jtreg bug.
One of the tests listed in JDK-8181299 is 
java/net/URLConnection/6212146/TestDriver.java. In jdk10/jdk10 the test 
description has:


@build jdk.test.lib.JDKToolFinder jdk.test.lib.process.ProcessTools Test

but the test fails intermittently with:

java.lang.NoClassDefFoundError: jdk/test/lib/process/StreamPumper
at jdk.test.lib.process.ProcessTools.getOutput(ProcessTools.java:85)
at jdk.test.lib.process.OutputAnalyzer.(OutputAnalyzer.java:47)
at jdk.test.lib.process.ProcessTools.executeProcess(ProcessTools.java:397)
at jdk.test.lib.process.ProcessTools.executeProcess(ProcessTools.java:425)
at jdk.test.lib.process.ProcessTools.executeCommand(ProcessTools.java:475)
at TestDriver.main(TestDriver.java:61)

I assume changing the @build to jdk.test.lib.process.* will fix this, 
assuming all the classes needed for ProcessTools are in this package.


As to why it's intermittent then I think it's a side effect of test 
library classes being compiled by one test and then re-used by a test 
that runs sometime later in the same VM. Combine that with implicit 
compilation, varied @build usages, and concurrently should explain why 
it's intermittent. I see you've cc'ed Jon Gibbons and he is the best 
person to comment on this. Now seems a good time to get to the bottom of 
these issues, esp. with Igor changing lots of tests to drop the explicit 
compilation of test library classes.


-Alan

Re: JDK 10 RFR of JDK-8181301: Refactor shell test AsynchronousChannelGroup/run_any_task.sh to java

[Amy Lu]

On 5/31/17 4:01 PM, Alan Bateman wrote:

On 31/05/2017 04:57, Amy Lu wrote:


java/nio/channels/AsynchronousChannelGroup/run_any_task.sh

Please review this patch to refactor the shell test to java.

bug: https://bugs.openjdk.java.net/browse/JDK-8181301
webrev: http://cr.openjdk.java.net/~amlu/8181301/webrev.00/
This looks okay to me although I think I would put the test 
description before the imports in AsExecutor so you don't have to skip 
over those to see how the test is run. A minor nit is that the JAR 
file can be "privileged.jar" rather than "Privileged.jar".


-Alan

Thank you Alan!

Webrev updated according to the suggestions:
http://cr.openjdk.java.net/~amlu/8181301/webrev.01

Thanks,
Amy

Re: JDK 10 RFR of JDK-8181126: Refactor shell test java/nio/Buffer/LimitDirectMemory.sh to java

[Alan Bateman]

On 26/05/2017 03:42, Amy Lu wrote:


java/nio/Buffer/LimitDirectMemory.sh

Please review this patch to refactor the shell test to java.

bug: https://bugs.openjdk.java.net/browse/JDK-8181126
webrev: http://cr.openjdk.java.net/~amlu/8181126/webrev.00/

This looks okay to me.

-Alan

Re: RFR(S) : 8180805 : move RandomFactory to the top level testlibrary

[Felix Yang]

Hi Alan

even with explicit compilation, I also observed failures. I'm 
curious what is the best practice here. IMO, there could be a potential 
jtreg bug.


See:

https://bugs.openjdk.java.net/browse/JDK-8181299

https://bugs.openjdk.java.net/browse/JDK-8181300

Thanks,
Felix
On 2017/5/31 15:27, Alan Bateman wrote:

On 27/05/2017 07:30, Igor Ignatyev wrote:


http://cr.openjdk.java.net/~iignatyev//8180805/webrev.00/index.html

308 lines changed: 110 ins; 40 del; 158 mod


One general comment about all these moves is that I see that many 
tests are being changed to no longer explicitly compile the test 
infrastructure, instead I think it means the tests will rely on 
implicit complication of the infrastructure classes - is that 
intended? We've had a lot a issues in the past with agent VMs + 
concurrency with implicit complication. Have you seen any issues with 
these changes?


-Alan

Re: JDK 10 RFR of JDK-8181301: Refactor shell test AsynchronousChannelGroup/run_any_task.sh to java

[Alan Bateman]

On 31/05/2017 04:57, Amy Lu wrote:


java/nio/channels/AsynchronousChannelGroup/run_any_task.sh

Please review this patch to refactor the shell test to java.

bug: https://bugs.openjdk.java.net/browse/JDK-8181301
webrev: http://cr.openjdk.java.net/~amlu/8181301/webrev.00/
This looks okay to me although I think I would put the test description 
before the imports in AsExecutor so you don't have to skip over those to 
see how the test is run. A minor nit is that the JAR file can be 
"privileged.jar" rather than "Privileged.jar".


-Alan

Re: RFR(S) : 8180888: move jdk.testlibrary.JarUtils to the top level testlibrary

[Alan Bateman]

On 31/05/2017 01:40, Igor Ignatyev wrote:


http://cr.openjdk.java.net/~iignatyev//8180888/webrev.00/index.html

354 lines changed: 148 ins; 149 del; 57 mod;

Hi all,

could you please review this changeset which moves JarUtils to the top level 
testlibrary?

webrev: http://cr.openjdk.java.net/~iignatyev//8180888/webrev.00/index.html
jbs: https://bugs.openjdk.java.net/browse/JDK-8180888
testing: :tier[1-3]


This is "old" JarUtils that is mostly used by the security libs tests. 
We have a "new" JarUtils that we have been using in newer tests - are 
you planning to move that too?


-Alan