Re: RFR8194230, jdk/internal/jrtfs/remote/RemoteRuntimeImageTest.java fails with NPE

[Felix Yang]

Hi Alan,

    please review the update patch, and reduced checking as suggested:

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

Thanks,
Felix
On 2018/8/6 18:23, Alan Bateman wrote:

On 31/07/2018 07:16, Felix Yang wrote:

Hi all,

    please review a patch to improve the checking on the settings. 
Originally it will throw NPE, if specified path is invalid.


Bug:

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

Webrev:

    http://cr.openjdk.java.net/~xiaofeya/8194230/webrev.00/
Have you considered merging getJdk8Path and isJdk8 so that it simple 
checks for the release file and rejects the value of JDK8_HOME when it 
doesn't exist? I think that should reduce the number of checks and 
keep it very simple.


-Alan

RFR8194230, jdk/internal/jrtfs/remote/RemoteRuntimeImageTest.java fails with NPE

[Felix Yang]

Hi all,

    please review a patch to improve the checking on the settings. 
Originally it will throw NPE, if specified path is invalid.


Bug:

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

Webrev:

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

Thanks,

Felix

RFR 8202708, Add a check of opening stream for not-existing UNC url

[Felix Yang]

Hi all,

   please review a change to add additional checking for opening stream 
with not-existing UNC url.


Bug:

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

Webrev:

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

Thanks,

Felix

RFR 8201348, ProblemList update for bugid associated with SSLSocketParametersTest.sh

[Felix Yang]

Hi,

    please review a minor change on the associated bug id in 
ProblemList.txt.


Bug:

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

Patch:

diff -r f088ec60bed5 test/jdk/ProblemList.txt
--- a/test/jdk/ProblemList.txt  Mon Apr 09 10:39:29 2018 -0700
+++ b/test/jdk/ProblemList.txt  Mon Apr 09 19:26:47 2018 -0700
@@ -778,7 +778,7 @@

 com/sun/jndi/ldap/LdapTimeoutTest.java 8151678 linux-all

-javax/rmi/ssl/SSLSocketParametersTest.sh 8194663 generic-all
+javax/rmi/ssl/SSLSocketParametersTest.sh 8162906 generic-all

 


Thanks,

Felix

RFR 8190505, typo in test/jdk/ProblemList.txt

[Felix Yang]

Please review a minor patch to correct typo in test/jdk/ProblemList.txt. 
Change "Hashmap" to "HashMap".


diff -r 4a35a00eb001 test/jdk/ProblemList.txt
--- a/test/jdk/ProblemList.txt    Wed Nov 01 16:45:28 2017 +0100
+++ b/test/jdk/ProblemList.txt    Wed Nov 01 23:34:44 2017 -0400
@@ -320,7 +320,7 @@

 org/omg/CORBA/OrbPropertiesTest.java                8175177 
generic-all


-javax/rmi/PortableRemoteObject/ConcurrentHashmapTest.java 8080643 
generic-all
+javax/rmi/PortableRemoteObject/ConcurrentHashMapTest.java 8080643 
generic-all


 javax/rmi/ssl/SSLSocketParametersTest.sh 8162906 generic-all


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

Thanks,

Felix

Re: RFR 8184904/10, jdk/internal/jrtfs/WithSecurityManager fails with exploded builds

[Felix Yang]

Hi Alan,
pushed as suggested.

Thanks,
Felix
> On 20 Jul 2017, at 7:23 PM, Alan Bateman  wrote:
> 
> This skips the testing on exploded builds, which I think is okay. A minor 
> point before pushing is that you replace "lib/modules" with "lib", "modules" 
> as the elements will be joined by the method.
> 
> -Alan

Re: RFR 8184904/10, jdk/internal/jrtfs/WithSecurityManager fails with exploded builds

[Felix Yang]

Hi Alan,

if I understand correctly, you meant that just to check for 
${java.home}/lib/modules is enough?


Please review the updated patch. I also adjusted checking in Basic.java, 
which also has duplicate checking and is problematic (jrt-fs.jar is no 
longer under JDK root directory)


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

Thanks,
Felix
On 2017/7/19 17:50, Alan Bateman wrote:

On 19/07/2017 10:43, Felix Yang wrote:

Hi all,

please review a patch to skip parts of testing with exploded builds.

Bug:

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

Webrev:

http://cr.openjdk.java.net/~xiaofeya/8184904/webrev.00/
The check for the image type should be independent of the allow/deny 
parameter. Alternatively, and as jrtfs does support exploded builds, 
is to use a different policy file when on an exploded build.


Also checking for an image doesn't only need to check for 
${java.home}/lib/modules.


-Alan

RFR 8184904/10, jdk/internal/jrtfs/WithSecurityManager fails with exploded builds

[Felix Yang]

Hi all,

please review a patch to skip parts of testing with exploded builds.

Bug:

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

Webrev:

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

Thanks,

Felix

Re: RFR 10: 8184808 (process) isAlive should use pid for validity, not /proc/pid

[Felix Yang]

Hi Roger,

 is it necessary to add the bug id to OnExitTest?

-Felix
On 2017/7/19 2:46, Roger Riggs wrote:
Please review a fix for an intermittent failure in the ProcessHandle 
OnExitTest

that fails frequently on Solaris.

ProcessHandle.isAlive is using /proc/pid/psinfo to determine if a 
process is alive and it's start time.
However, it appears that the between the process exiting and the 
reaping of its status, the
psinfo file indicates the process is alive but kill(pid, 0) reports 
that is is not alive.
Depending on a race, the ProcessHandler.onExit may determine the 
process has exited

but later isAlive may report it is alive.

To have a consistent view of the process being alive, 
ProcessHandle.isAlive in its native implementation
should use kill(pid, 0) to determine if the process is definitively 
determine if the process alive.


The original issue[1] will be kept open until it is known that it is 
resolved.


Webrev:
  http://cr.openjdk.java.net/~rriggs/webrev-alive-solaris-8184808/

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

Thanks, Roger

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

Re: JDK 10 RFR of JDK-8183378: Refactor java/lang/System/MacEncoding/MacJNUEncoding.sh to java

[Felix Yang]

Hi Amy,

looks fine. Just one comment on sentence below. "LOCALE" looks to 
be a local variable, though used several times. Switch to usual naming?


  50 final String LOCALE = args[2];

-Felix
On 2017/7/6 9:47, Amy Lu wrote:

java/lang/System/MacEncoding/MacJNUEncoding.sh

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

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

Thanks,
Amy

JDK 10 RFR 8182620, Problem list java/lang/String/nativeEncoding/StringPlatformChars.java for JDK-8182569

[Felix Yang]

Hi there,

   please review a patch to problem list 
java/lang/String/nativeEncoding/StringPlatformChars.java temporarily. It 
has been failing in some test setup with "Error. Use -nativepath to 
specify the location of native code".


Bug: https://bugs.openjdk.java.net/browse/JDK-8182620

Thanks,

Felix

Patch:

diff -r 77ad59329987 test/ProblemList.txt
--- a/test/ProblemList.txt  Tue Jun 20 13:43:43 2017 -0700
+++ b/test/ProblemList.txt  Tue Jun 20 18:54:54 2017 -0700
@@ -126,6 +126,8 @@

 jdk/internal/misc/JavaLangAccess/NewUnsafeString.java 8176188 generic-all

+java/lang/String/nativeEncoding/StringPlatformChars.java 8182569 
windows-all,solaris-all

+
 

 # jdk_instrument

Re: (10) RFR of JDK-8181912,Refactor locale related shell test test/java/io/File/MacPathTest.sh to java test

[Felix Yang]

Hamlin,

this may be not a blocker. if you prefer 'pure' java, it can be 
achieved by adjusting environment for ProcessBuilder.


see Amy's patch: 
http://cr.openjdk.java.net/~amlu/8181395/webrev.00/test/java/nio/charset/Charset/DefaultCharsetTest.java.html


In further, you may extend and wrap such usage in ProcessTools, because 
this looks to be a common usage, at least in encoding use cases.


-Felix
On 2017/6/16 11:40, Hamlin Li wrote:


Hi Felix,

Thank you for your suggestions.

But I think it's just a java wrapper around a shell, not a real java.

Thank you

-Hamlin


On 2017/6/16 9:41, Felix Yang wrote:


Hi Hamlin,

 I think you need something like:

/ProcessTools.executeCommand("sh", "-c", yourTestCmd).../

yourTestCmd is like the original cmd in shell "
export LC_ALL=en_US.UTF-8 ;${TESTJAVA}/bin/java ${TESTVMOPTS} -cp ${TESTCLASSES} 
MacPathTest"
-Felix
On 2017/6/16 9:32, Naoto Sato wrote:

Hi Hamlin,

What I meant was that setting the java locale either through 
Locale.setDefault() or user.language/user.country properties won't 
affect the default encoding determination. Other properties 
(file.encoding/sun.jnu.encoding) would set the default, but I am not 
sure how they are supposed to be used in regression tests.


Naoto

On 6/15/17 4:59 PM, Hamlin Li wrote:

Hi Naoto,

Thank you for comments.

Do you mean there is no way to set encoding through system property 
or java API? And can I understand it as it's better to keep the 
shell test rather than convert it to java test.


Thank you

-Hamlin


On 2017/6/16 0:45, Naoto Sato wrote:

Hi,

Setting the default Java Locale and/or user.* properties has 
nothing to do with the default encoding. The default encoding on 
mac/unix environments is determined from the environment variable 
LC_CTYPE.


Naoto

On 6/14/17 8:35 PM, Hamlin Li wrote:


On 2017/6/15 1:22, Alan Bateman wrote:

On 12/06/2017 09:00, Hamlin Li wrote:

Would you please review the below patch?

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

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

Are you sure that setting the user.* properties on the command 
actually works? I assume we'll have to change this back at some 
point to launch the test with LC_ALL set to UTF-8.

Hi Alan,

Besides of setting user.language and user.country, the patch also 
sets file.encoding, and sun.jnu.encoding as UTF-8.


I understand your concern: we're not sure if setting -Dxxx 
properties will have totally same functionality as setting env 
variable LC_ALL. I have no answer for this question.

What tests have been done:
  1. set user.* properties will affect what Locale.getDefault() 
gets, set file.encoding will affect what 
java.nio.charset.Charset.defaultCharset() gets.

  2. jprt passed.
  3. test on some other mac machine in SQE pool.

Do you suggest we should keep this test as shell?

Thank you
-Hamlin


-Alan

RFR 8182321/10, Mark java/lang/ProcessHandle/OnExitTest.java as intermittent

[Felix Yang]

Hi,

please review a minor patch to add @key intermittent to 
java/lang/ProcessHandle/OnExitTest.java. It has been observed to be 
failing intermittently on Solaris x64.


Bug: https://bugs.openjdk.java.net/browse/JDK-8182321

Patch:

diff -r d6e163b08d8c test/java/lang/ProcessHandle/OnExitTest.java
--- a/test/java/lang/ProcessHandle/OnExitTest.java  Thu Jun 15 
15:00:30 2017 -0700
+++ b/test/java/lang/ProcessHandle/OnExitTest.java  Thu Jun 15 
21:33:07 2017 -0700

@@ -40,6 +40,7 @@

 /*
  * @test
+ * @key intermittent
  * @library /test/lib
  * @modules java.base/jdk.internal.misc
  *  jdk.management

Thanks,

Felix

Re: (10) RFR of JDK-8181912,Refactor locale related shell test test/java/io/File/MacPathTest.sh to java test

[Felix Yang]

Hi Hamlin,

 I think you need something like:

/ProcessTools.executeCommand("sh", "-c", yourTestCmd).../

yourTestCmd is like the original cmd in shell "

export LC_ALL=en_US.UTF-8 ;${TESTJAVA}/bin/java ${TESTVMOPTS} -cp ${TESTCLASSES} 
MacPathTest"

-Felix
On 2017/6/16 9:32, Naoto Sato wrote:

Hi Hamlin,

What I meant was that setting the java locale either through 
Locale.setDefault() or user.language/user.country properties won't 
affect the default encoding determination. Other properties 
(file.encoding/sun.jnu.encoding) would set the default, but I am not 
sure how they are supposed to be used in regression tests.


Naoto

On 6/15/17 4:59 PM, Hamlin Li wrote:

Hi Naoto,

Thank you for comments.

Do you mean there is no way to set encoding through system property 
or java API? And can I understand it as it's better to keep the shell 
test rather than convert it to java test.


Thank you

-Hamlin


On 2017/6/16 0:45, Naoto Sato wrote:

Hi,

Setting the default Java Locale and/or user.* properties has nothing 
to do with the default encoding. The default encoding on mac/unix 
environments is determined from the environment variable LC_CTYPE.


Naoto

On 6/14/17 8:35 PM, Hamlin Li wrote:


On 2017/6/15 1:22, Alan Bateman wrote:

On 12/06/2017 09:00, Hamlin Li wrote:

Would you please review the below patch?

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

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

Are you sure that setting the user.* properties on the command 
actually works? I assume we'll have to change this back at some 
point to launch the test with LC_ALL set to UTF-8.

Hi Alan,

Besides of setting user.language and user.country, the patch also 
sets file.encoding, and sun.jnu.encoding as UTF-8.


I understand your concern: we're not sure if setting -Dxxx 
properties will have totally same functionality as setting env 
variable LC_ALL. I have no answer for this question.

What tests have been done:
  1. set user.* properties will affect what Locale.getDefault() 
gets, set file.encoding will affect what 
java.nio.charset.Charset.defaultCharset() gets.

  2. jprt passed.
  3. test on some other mac machine in SQE pool.

Do you suggest we should keep this test as shell?

Thank you
-Hamlin


-Alan

Re: RFR 8181080/10, Refactor several sun/net shell tests to plain java tests

[Felix Yang]

Excuse me,

   Alan, could you have a look at this patch?

Thanks,
Felix
On 2017/6/7 15:04, Felix Yang wrote:

Hi Alan,

updated patch. Keep the first version of OtherResourcesTest with 
clean-up.


http://cr.openjdk.java.net/~xiaofeya/8181080/webrev.02/

Thanks,
Felix
On 2017/6/7 0:36, Alan Bateman wrote:

On 06/06/2017 16:54, Felix Yang wrote:
Well, probably no. executeTestJava will add test vm opts to the exec 
commands, so it will show two "--limit-modules xxx" either. This is 
not the test intended.
Ah okay, I did write the original test but forget that 
executeTestJava inherits the options.


Okay, if we can clean up OtherResourcesTest then I'll go along with 
that approach. By clean-up then it would be nice to move the test 
description to the top and also reformat the the use of 
executeCommand so that it's easy to read.


-Alan.

Re: RFR 8181080/10, Refactor several sun/net shell tests to plain java tests

[Felix Yang]

Hi Alan,

updated patch. Keep the first version of OtherResourcesTest with 
clean-up.


http://cr.openjdk.java.net/~xiaofeya/8181080/webrev.02/

Thanks,
Felix
On 2017/6/7 0:36, Alan Bateman wrote:

On 06/06/2017 16:54, Felix Yang wrote:
Well, probably no. executeTestJava will add test vm opts to the exec 
commands, so it will show two "--limit-modules xxx" either. This is 
not the test intended.
Ah okay, I did write the original test but forget that executeTestJava 
inherits the options.


Okay, if we can clean up OtherResourcesTest then I'll go along with 
that approach. By clean-up then it would be nice to move the test 
description to the top and also reformat the the use of executeCommand 
so that it's easy to read.


-Alan.

Re: RFR 8181080/10, Refactor several sun/net shell tests to plain java tests

[Felix Yang]

Alan,

   comments inline

thanks

-Felix
On 2017/6/6 23:40, Alan Bateman wrote:

On 06/06/2017 16:28, Felix Yang wrote:

All 3 tests will fail with error missing java.compiler module.
I also attempted with --limit-modules=java.se. Then other resources 
will fail with "java.lang.RuntimeException: Need to run with 
--limit-modules java.base". vmoptions will be added  to @run entry by 
jtreg, then lead the confusion.


SO IMO, the first proposal to fork a java process with constant 
arguments is probably better than using @run entry.
http://cr.openjdk.java.net/~xiaofeya/8181080/webrev.00/test/sun/net/www/protocol/jrt/OtherResourcesTest.java.html 



Do you agree?
It's probably okay but would be nicer if OtherResourcesTest used 
executeTestJava (it doesn't need to use JDKToolFinder or executeCommand).
Well, probably no. executeTestJava will add test vm opts to the exec 
commands, so it will show two "--limit-modules xxx" either. This is not 
the test intended.


-Felix


-Alan

Re: RFR 8181080/10, Refactor several sun/net shell tests to plain java tests

[Felix Yang]

Alan and Chris,


On 2017/6/6 18:12, Alan Bateman wrote:



On 06/06/2017 10:27, Felix Yang wrote:

Addressed all comments. Please review the updated patch:

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

Can you run the tests in sun/net/www/protocol/jrt with 
`-vmoption:--limit-modules=java.desktop` to see how it behaves? I 
think, but not certain, that this is the reason why we introduced a 
shell to run that test.

All 3 tests will fail with error missing java.compiler module.
I also attempted with --limit-modules=java.se. Then other resources will 
fail with "java.lang.RuntimeException: Need to run with --limit-modules 
java.base". vmoptions will be added  to @run entry by jtreg, then lead 
the confusion.


SO IMO, the first proposal to fork a java process with constant 
arguments is probably better than using @run entry.

http://cr.openjdk.java.net/~xiaofeya/8181080/webrev.00/test/sun/net/www/protocol/jrt/OtherResourcesTest.java.html

Do you agree?

Thanks,
Felix


-Alan

Re: RFR 8181080/10, Refactor several sun/net shell tests to plain java tests

[Felix Yang]

Addressed all comments. Please review the updated patch:

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

Thanks,
Felix
On 2017/6/6 16:25, Alan Bateman wrote:



On 06/06/2017 09:15, Chris Hegarty wrote:

:

OtherResourcesTest.java is not strictly needed. You should be able to
provide an appropriate @run tag to OtherResources.java directly, like:

   @run main/othervm --limit-modules=java.base OtherResources

 As I recall, the reason we created this test a shell test, is because 
jtreg couldn't handle --limit-modules in the @run tag. So I agree we 
should go back to it and ideally just specify it to the @run tag as 
per your mail.


-Alan

Re: Ping~ Re: RFR 8181080/10, Refactor several sun/net shell tests to plain java tests

[Felix Yang]

Amy,

thanks for the comments.


On 2017/6/6 10:37, Amy Lu wrote:

sun/net/www/protocol/file/DirPermissionDenied.java
+
+ @BeforeTest
+ public void setup() throws Throwable {
+ // mkdir and chmod "333"
+ Files.createDirectories(TEST_DIR);
+ ProcessTools.executeCommand("chmod", "333", TEST_DIR.toString()) Maybe just 
do this with java api instead run chmod command?
As I know, there is no straight and exact replacement for chmod in java. 
Java provides PosixFilePermissions on Unix/Linux platforms. But on 
windows, File.set* looks a bit different. So, IMO, chmod is probably a 
nature choice.


-Felix


+
+ @AfterTest
+ public void tearDown() throws IOException {
+ if (Files.exists(TEST_DIR)) {
+ Files.delete(TEST_DIR);
+ }
+ }
Missed "Add back read access" before trying to delete it?
Based on my test results (local and JPRT), it deleted w/o any 
exceptions. This was originally added by Max in 
https://bugs.openjdk.java.net/browse/JDK-7078355.

TO Max, could you comment if such chmod is still necessary?
If yes, since this is just for test clean-up rather than a part of test 
logic, is it enough with "File.setReadable(...)"?


-Felix


Thanks,
Amy

On 6/6/17 9:33 AM, Felix Yang wrote:

Thanks:-)
-Felix
On 2017/6/1 16:32, Felix Yang wrote:

Hi there,

please review the patch convert several sun/net shell tests to 
plain java tests.


Bug:

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

Webrev:

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

Thanks,

Felix

Ping~ Re: RFR 8181080/10, Refactor several sun/net shell tests to plain java tests

[Felix Yang]

Thanks:-)
-Felix
On 2017/6/1 16:32, Felix Yang wrote:

Hi there,

please review the patch convert several sun/net shell tests to 
plain java tests.


Bug:

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

Webrev:

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

Thanks,

Felix

Re: RFR 8181414/10, Refactor misc test/sun/net/www/protocol/jar shell tests to plain java tests

[Felix Yang]

Igor and Paul,

 thanks for the review. Pushed with typo fixed.

-Felix
On 2017/6/3 6:04, Igor Ignatyev wrote:
http://cr.openjdk.java.net/~xiaofeya/8181414/webrev.00/test/sun/net/www/protocol/jar/B5105410.java.udiff.html 
<http://cr.openjdk.java.net/%7Exiaofeya/8181414/webrev.00/test/sun/net/www/protocol/jar/B5105410.java.udiff.html> 


+ # @summary ZipFile$ZipFileInputStream doesn't close handle to zipfile

a typo, it should be * @summary

-- Igor


On Jun 1, 2017, at 12:39 AM, Felix Yang <felix.y...@oracle.com> wrote:

Hi there,

   please review the patch to convert several shell tests to plain 
java tests.


Bug:

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

Webrev:

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

Thanks,

Felix

Re: RFR 8181413/10, Refactor test/sun/net/www/protocol/jar/jarbug/run.sh to plain java tests

[Felix Yang]

Hi Paul,

 this is from original test logic. According with the bug history, 
it is added by https://bugs.openjdk.java.net/browse/JDK-7152892 to avoid 
intermittent test failures.


Thanks,
Felix
On 2017/6/3 5:08, Paul Sandoz wrote:

It looks ok, but i wonder why need to chmod?

Paul.


On 1 Jun 2017, at 00:13, Felix Yang <felix.y...@oracle.com> wrote:

Hi there,

please review the patch to covert a shell test to a plain java test, and 
simplify a few of  unnecessary test logic.

Bug:

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

Webrev:

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

Thanks,

Felix

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

[Felix Yang]

Igor


On 2017/6/2 10:11, Igor Ignatyev wrote:

Hi Felix,

none of the jdk tests which fail w/ NCDFE: 
jdk/test/lib/process/StreamPumper depend on StreamPumper directly, 
they get this dependency transitively 
from jdk/test/lib/process/ProcessTools,

That is why I think it is a bug too.
so I don't see how you will find this good definition of "explicit" 
even for the failures at hand.
Just meant "expected behavior", as it makes test code clear for me. Of 
course it fails, otherwise there will be no such discussion at all.


-Felix


I recommend to work around this the same way we did it in hotspot, 
which reliably removed almost all our NCDFE failures, -- remove 
explicit @build, if not all for all classes, then at least for 
jdk/test/lib/** classes.


-- Igor

On Jun 1, 2017, at 6:52 PM, Felix Yang <felix.y...@oracle.com 
<mailto:felix.y...@oracle.com>> wrote:


Hi Igor and Ioi,

 I partially agree with you. As initially stated in the proposal 
and bug(JDK-8181299 
<https://bugs.openjdk.java.net/browse/JDK-8181299>), I don't think 
this patch is a fix but a quick workaround to make them runnable.


"explicit" is reasonable for me. But "explicit" should not be 
restricted as "explicit all, including dependencies", as it is not 
productive or even realistic in the long term.


Thanks,
Felix
On 2017/6/2 7:58, Igor Ignatyev wrote:

For example: doing this may be enough for now:

* @build jdk.test.lib.process.*

But what if in the future, jdk.test.lib.process is restructured to 
have a private package jdk.test.lib.process.hidden? To work around 
CODETOOLS-7901986, all the test cases that must be modified to the 
following, which unnecessarily exposes library implementation 
details to the library users:


* @build jdk.test.lib.process.* jdk.test.lib.process.hidden.*


and in fact, there is already similar problem and 
http://cr.openjdk.java.net/~xiaofeya/8181299/webrev.01/ 
<http://cr.openjdk.java.net/%7Exiaofeya/8181299/webrev.01/> does not 
address it.
jdk/test/lib/process/ProcessTools depends on jdk/test/lib/Utils so 
all tests which have '@build jdk.test.lib.process.ProcessTools' will 
have to have  '@build jdk.test.lib.Utils'. then we 
have OutputAnalyzer which depends on ProcessTools so all tests which 
@build jdk.test.lib.process.OutputAnalyzer will @build ProcessTools 
and Utils explicitly.  many testlibrary classes which on 
jdk.test.lib.process.OutputAnalyzer, so one will have to 
specify OutputAnalyzer ProcessTools and Utils in the tests which 
depends on other testlibrary classes.  to make things even worse, 
Utils depends on OutputAnalyzer and there are lots of tests and test 
library classes which depend on Utils, so all of them will have to 
have at least '@build jdk.test.lib.Utils 
jdk.test.lib.process.OutputAnalyzer jdk.test.lib.process.ProcessTools'. 
and they will work stable till someone refactors them and extract 
some new classes. that is to say, it's nearly impossible to have all 
explicit @build actions.


Cheers,
-- Igor

On Jun 1, 2017, at 3:37 PM, Ioi Lam <ioi@oracle.com 
<mailto:ioi@oracle.com>> wrote:




On 6/1/17 1:17 PM, Igor Ignatyev wrote:
On Jun 1, 2017, at 1:20 AM, Chris Hegarty 
<chris.hega...@oracle.com <mailto:chris.hega...@oracle.com>> wrote:


Igor,

On 1 Jun 2017, at 04:32, Igor Ignatyev <igor.ignat...@oracle.com 
<mailto:igor.ignat...@oracle.com>> wrote:


Hi Felix,

I have suggested the exact opposite change[1-3] to fix the same 
problem.
I’m sorry, but this is all just too confusing. After your change, 
who, or what, is

responsible for building/compiling the test library dependencies?
jtreg is responsible, there is an implicit build for each @run, 
and jtreg will analyze a test class to get transitive closure for 
static dependencies, hence you have to have @build only for 
classes which are not in constant pool, e.g. used only by 
reflection or whose classnames are only used to spawn a new java 
instance.



I suspect the problem is caused by a long standing bug in jtreg 
that results in library classes being partially compiled. Please 
see my evaluation in


https://bugs.openjdk.java.net/browse/CODETOOLS-7901986

In the bug report, there is test case that can reliably reproduce 
the NoClassDefFoundError problem.


I think adding all the @build commands in the tests are just 
band-aids. Things will break unless every test explicitly uses 
@build to build every class in every library that they use, 
including all the private classes that are not directly accessible 
by the test cases.


For example: doing this may be enough for now:

* @build jdk.test.lib.process.*

But what if in the future, jdk.test.lib.process is restructured to 
have a private package jdk.test.lib.process.hidden? To work around 
CODETOOLS-7901986, all the test cases that must be modified to the 
following, which unnecessarily exposes library implement

RFR 8181414/10, Refactor misc test/sun/net/www/protocol/jar shell tests to plain java tests

[Felix Yang]

Hi there,

please review the patch to convert several shell tests to plain 
java tests.


Bug:

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

Webrev:

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

Thanks,

Felix

RFR 8181413/10, Refactor test/sun/net/www/protocol/jar/jarbug/run.sh to plain java tests

[Felix Yang]

Hi there,

please review the patch to covert a shell test to a plain java 
test, and simplify a few of  unnecessary test logic.


Bug:

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

Webrev:

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

Thanks,

Felix

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 
<http://cr.openjdk.java.net/%7Eiignatyev//8181391/webrev.00/index.html>


Thanks,
-- Igor
On May 31, 2017, at 8:27 PM, Felix Yang <felix.y...@oracle.com 
<mailto:felix.y...@oracle.com>> wrote:


Hi Chris and Daniel,

  new webrev with a few of explicit builds than wildcard.

http://cr.openjdk.java.net/~xiaofeya/8181299/webrev.01/ 
<http://cr.openjdk.java.net/%7Exiaofeya/8181299/webrev.01/>


Thanks,
Felix
On 2017/5/31 18:20, Chris Hegarty wrote:
On 31 May 2017, at 10:42, Felix Yang <felix.y...@oracle.com 
<mailto:felix.y...@oracle.com>> 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/ 
<http://cr.openjdk.java.net/%7Exiaofeya/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.

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: 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: RFR 8166139/10, Refactor java/net shell cases to java

[Felix Yang]

Hi Chris,

please review the updated webrev below. Comments inline.

http://cr.openjdk.java.net/~xiaofeya/8166139/webrev.03/

-Felix
On 2017/5/29 18:19, Chris Hegarty wrote:

Felix,

Thanks for taking this one. A few comments:


1) test/java/net/URLConnection/6212146/TestDriver.java
   Please check indentation around L81:

  80 Files.copy(testJar,
 ___targetDir.resolve(ARCHIVE_NAME),
  81 ___StandardCopyOption.REPLACE_EXISTING);

   Also, and more importantly, the ulimit that was executed in the shell
   is supposed to affect the potential file descriptor usage of the
   following java process. I believe that is not captured correctly
   in the TestDriver.java, since there is no parent/child relationship
   between the processes, no?

Fixed, thanks
Felix



2) test/java/net/URLClassLoader/getresourceasstream/policy

   Please add a copyright/header.

Checked existing test policy files, all of them have no 
copyright/header. So I hesitated to add here. Please confirm.


-Felix
3) getresourceasstream/TestDriver.java, CheckSealedTest.java, 
CloseTest.java


   If you statically import StandardCopyOption.REPLACE_EXISTING in a
   few places it may lead to a few shorter lines.

Changed, thanks
Felix


4) Why has the test for 503 simply been removed?

As priorly commented, the test is probably not necessary.
Because it actually quits immediately after prerequisite checking for 
"javax/swing/text/rtf/charsets/mac.txt". According with JDK-503, it 
is to cover endorsed scenarios, while endorsed mechanism has been 
removed in JDK 9.


-Felix


-Chris.


On 27/05/17 02:15, Felix Yang wrote:

Hi Roger,

thanks for the comments. Please see the new webrev

http://cr.openjdk.java.net/~xiaofeya/8166139/webrev.02/

-Felix
On 2017/5/27 3:50, Roger Riggs wrote:

Hi Felix,

fyi, there is a new version of webrev.ksh that provides convenient
next and previous links.
http://hg.openjdk.java.net/code-tools/webrev/raw-file/d26c194772db/webrev.ksh 





CloseTest.java: 66; checking WORK_DIR *after* calling setup() does not
make sense.

Ditto: closetest/GetResourceAsStream.java

Removed such check, since the test depends on several system
properties("test.src", "user.dir"). It is not expected to run outside
jtreg.

-Felix


URLConnection/6212146/Test.java:47 typo: ULR -> URL

Regards, Roger


On 5/25/2017 11:14 PM, Felix Yang wrote:

Hi Roger,

please review the updated webrev:

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

Thanks,
Felix
On 2017/5/26 3:24, Roger Riggs wrote:

Hi Felix,

closetest/CloseTest:

32: Please put the @modules directives together (and not repeat)

Fixed
-Felix


44: do not use wildcard imports  (reconfigure your IDE if necessary
to avoid accidents).

Fixed and also organized imports in other test files
-Felix


63: setup() is invoked twice...  remove 1 or explain why 2 calls

Fixed
-Felix


69, 78,90,91,... :  space in method call is not proper style

Fixed and also formatted other history codes. Just format only
without logic change.
Not restricted to this test. I only formatted history codes "nearby",
to avoid hiding real logic changes in the patch.

-Felix


sealing/CheckSealed.java:
 - Why is @test block removed?
Should be converted to @run main CheckSealed

There was a checksealed.sh. I replaced it with CheckSealedTest.java
and declared @test there.
http://cr.openjdk.java.net/~xiaofeya/8166139/webrev.01/test/java/net/URLClassLoader/sealing/CheckSealedTest.java.html 




BTW, you may noticed test/java/net/URLClassLoader/B503.* were
removed. That is because corresponding prerequisite was removed even
in JDK 8.

Perhaps then add a comment to CheckSealed.java indicating it is
compiled and executed by CheckSealedTest.java.

Roger



-Felix


Regards, Roger



On 5/25/2017 4:08 AM, Felix Yang wrote:

Hi there,

   please review following patch to convert all shell cases under
java/net to plain java codes.

Webrev:

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

Bug:

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


Thanks,

Felix

Re: RFR 8166139/10, Refactor java/net shell cases to java

[Felix Yang]

Hi Roger,

thanks for the comments. Please see the new webrev

http://cr.openjdk.java.net/~xiaofeya/8166139/webrev.02/

-Felix
On 2017/5/27 3:50, Roger Riggs wrote:

Hi Felix,

fyi, there is a new version of webrev.ksh that provides convenient 
next and previous links.

http://hg.openjdk.java.net/code-tools/webrev/raw-file/d26c194772db/webrev.ksh


CloseTest.java: 66; checking WORK_DIR *after* calling setup() does not 
make sense.


Ditto: closetest/GetResourceAsStream.java
Removed such check, since the test depends on several system 
properties("test.src", "user.dir"). It is not expected to run outside jtreg.


-Felix


URLConnection/6212146/Test.java:47 typo: ULR -> URL

Regards, Roger


On 5/25/2017 11:14 PM, Felix Yang wrote:

Hi Roger,

please review the updated webrev:

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

Thanks,
Felix
On 2017/5/26 3:24, Roger Riggs wrote:

Hi Felix,

closetest/CloseTest:

32: Please put the @modules directives together (and not repeat)

Fixed
-Felix


44: do not use wildcard imports  (reconfigure your IDE if necessary 
to avoid accidents).

Fixed and also organized imports in other test files
-Felix


63: setup() is invoked twice...  remove 1 or explain why 2 calls

Fixed
-Felix


69, 78,90,91,... :  space in method call is not proper style
Fixed and also formatted other history codes. Just format only 
without logic change.
Not restricted to this test. I only formatted history codes "nearby", 
to avoid hiding real logic changes in the patch.


-Felix


sealing/CheckSealed.java:
 - Why is @test block removed?
Should be converted to @run main CheckSealed
There was a checksealed.sh. I replaced it with CheckSealedTest.java 
and declared @test there.
http://cr.openjdk.java.net/~xiaofeya/8166139/webrev.01/test/java/net/URLClassLoader/sealing/CheckSealedTest.java.html 



BTW, you may noticed test/java/net/URLClassLoader/B503.* were 
removed. That is because corresponding prerequisite was removed even 
in JDK 8.
Perhaps then add a comment to CheckSealed.java indicating it is 
compiled and executed by CheckSealedTest.java.


Roger



-Felix


Regards, Roger



On 5/25/2017 4:08 AM, Felix Yang wrote:

Hi there,

   please review following patch to convert all shell cases under 
java/net to plain java codes.


Webrev:

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

Bug:

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


Thanks,

Felix

Re: RFR 8166139/10, Refactor java/net shell cases to java

[Felix Yang]

Hi Amy,

it is intended, because in my changes CompilerUtils is usually used 
together with JarUtils, which has not been moved yet. It is a bit 
confusing to add two lib directories, and even there are files with the 
same name. So it may be clearer to refactor lib usage unified in 
JDK-8075327 <https://bugs.openjdk.java.net/browse/JDK-8075327> or 
clean-ups tasks with single purpose.


Thanks,

Felix

On 2017/5/26 11:43, Amy Lu wrote:

Hi, Felix

I noticed that CompilerUtils from jdk/test/lib/testlibrary is still 
used in tests. It’s better to use the version [1] from top level.


(Not a reviewer.)

Thanks,
Amy

[1] 
http://hg.openjdk.java.net/jdk10/jdk10/file/tip/test/lib/jdk/test/lib/compiler/CompilerUtils.java


On 5/26/17 11:14 AM, Felix Yang wrote:

Hi Roger,

please review the updated webrev:

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

Thanks,
Felix
On 2017/5/26 3:24, Roger Riggs wrote:

Hi Felix,

closetest/CloseTest:

32: Please put the @modules directives together (and not repeat)

Fixed
-Felix


44: do not use wildcard imports  (reconfigure your IDE if necessary 
to avoid accidents).

Fixed and also organized imports in other test files
-Felix


63: setup() is invoked twice...  remove 1 or explain why 2 calls

Fixed
-Felix


69, 78,90,91,... :  space in method call is not proper style
Fixed and also formatted other history codes. Just format only 
without logic change.
Not restricted to this test. I only formatted history codes "nearby", 
to avoid hiding real logic changes in the patch.


-Felix


sealing/CheckSealed.java:
 - Why is @test block removed?
Should be converted to @run main CheckSealed
There was a checksealed.sh. I replaced it with CheckSealedTest.java 
and declared @test there.
http://cr.openjdk.java.net/~xiaofeya/8166139/webrev.01/test/java/net/URLClassLoader/sealing/CheckSealedTest.java.html 



BTW, you may noticed test/java/net/URLClassLoader/B503.* were 
removed. That is because corresponding prerequisite was removed even 
in JDK 8.


-Felix


Regards, Roger



On 5/25/2017 4:08 AM, Felix Yang wrote:

Hi there,

   please review following patch to convert all shell cases under 
java/net to plain java codes.


Webrev:

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

Bug:

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


Thanks,

Felix

Re: RFR 8166139/10, Refactor java/net shell cases to java

[Felix Yang]

Hi Roger,

please review the updated webrev:

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

Thanks,
Felix
On 2017/5/26 3:24, Roger Riggs wrote:

Hi Felix,

closetest/CloseTest:

32: Please put the @modules directives together (and not repeat)

Fixed
-Felix


44: do not use wildcard imports  (reconfigure your IDE if necessary to 
avoid accidents).

Fixed and also organized imports in other test files
-Felix


63: setup() is invoked twice...  remove 1 or explain why 2 calls

Fixed
-Felix


69, 78,90,91,... :  space in method call is not proper style
Fixed and also formatted other history codes. Just format only without 
logic change.
Not restricted to this test. I only formatted history codes "nearby", to 
avoid hiding real logic changes in the patch.


-Felix


sealing/CheckSealed.java:
 - Why is @test block removed?
Should be converted to @run main CheckSealed
There was a checksealed.sh. I replaced it with CheckSealedTest.java and 
declared @test there.

http://cr.openjdk.java.net/~xiaofeya/8166139/webrev.01/test/java/net/URLClassLoader/sealing/CheckSealedTest.java.html

BTW, you may noticed test/java/net/URLClassLoader/B503.* were 
removed. That is because corresponding prerequisite was removed even in 
JDK 8.


-Felix


Regards, Roger



On 5/25/2017 4:08 AM, Felix Yang wrote:

Hi there,

   please review following patch to convert all shell cases under 
java/net to plain java codes.


Webrev:

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

Bug:

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


Thanks,

Felix

RFR 8166139/10, Refactor java/net shell cases to java

[Felix Yang]

Hi there,

   please review following patch to convert all shell cases under 
java/net to plain java codes.


Webrev:

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

Bug:

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


Thanks,

Felix

Re: RFR 8087307/9, new tests for ServiceLoader updates for module

[Felix Yang]

Hi all,

updated tests according with comments. Also comments inline.

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

Thanks,

Felix

On 2017/5/19 13:57, Hamlin Li wrote:

Hi Felix,

I have some comments as below:


1. Possible test gaps for Locating Order
1.1 providers in named module have high priority than 
providers in unnamed
1.2 ServiceLoader.load​(ModuleLayer layer, Class service) 
is not tested
This is a set of additional tests.  The above scenarios have been 
covered in existing j.u.ServiceLoader/modules/Basic.java


-Felix

2. Possible test gap for automatic module, it is not tested

Add a basic test for providers in automatic module. It leverages 
existing scripts implementations in other tests.


3. Could you remove unnecessary module dependency on java.scripting, 
replace it with a customized module or java.base? it will make your 
tests run under as many as conditions.

Good catch,thanks
-Felix


4. For [a|b]filesystem
could you use URLStreamHandlerProvider rather than 
FileSystemProvider, it will make your test files(FileSystemForInstr, 
AFileSystem, BFileSystem) more simple.
This is intended to leverage JDK built-in providers (jar, jrt) in 
Platform and Bootstrap classloaders. There is no explicit implementation 
for URLStreamHandlerProvider.

-Felix


5. MisUsesTest.java and missuse
missuses: miss => mis? (missuses => mis.uses)
Possible test gap: add similar use case for unnamed module?
A good question, but I think it is not necessary. The test is to cover 
missing "uses" in module descriptor with a service 
(URLStreamHandlerProvider) having no explicit implementation. It 
implicitly indicates that the "can use" check not-related with how it is 
implemented.


-Felix


6. OrderingWithInstrTest.java
Could you use jtreg tag "driver RedefineModuleAgent" rather than 
"@run main/othervm OrderingWithInstrTest" to trigger the preparation 
of test? and move createAgentJar() and main() into 
RedefineModuleAgent.java.

Fixed, thanks.
-Felix


7. ReloadTest.java
The test success depends on execution order of tests, so need to 
add priority=N to every @Test method, or separate tests into 2 
classes, one for negative, one for normal tests.
The tests looks for me not depend on order. They only depend on setup 
method which is annotated with BeforeClass.


8. Some additional minor comments:
8.1 split module name with ".", for e.g. multiprovides => 
multi.provides
I may keep the naming to be identical with existing ServiceLoader module 
tests.
8.2 wrap long lines, and revert unnecessary wrapping too, in 
several files.

8.3 correct indent, e.g. in PermissionTest.java
8.4 Is default constructor "public ProviderX() { }" necessary in 
multi.ProviderX ?

Resolved above, thanks


8.5 rename Service to OrderedService?

I may keep to be identical with existing ServiceLoader module tests.
-Felix


Thank you

-Hamlin


On 2017/5/19 9:38, Felix Yang wrote:

Ping:)

-Felix

On 16 May 2017, at 4:59 PM, Felix Yang <felix.y...@oracle.com> wrote:

Hi there,

please review the new tests added for ServiceLoader updates for 
module system.


Test bug:

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

Webrev:

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

Related product Changes:

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

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

Thanks,

Felix

Re: RFR 8087307/9, new tests for ServiceLoader updates for module

[Felix Yang]

Ping:)

-Felix
> On 16 May 2017, at 4:59 PM, Felix Yang <felix.y...@oracle.com> wrote:
> 
> Hi there,
> 
>please review the new tests added for ServiceLoader updates for module 
> system.
> 
> Test bug:
> 
>https://bugs.openjdk.java.net/browse/JDK-8087307
> 
> Webrev:
> 
>http://cr.openjdk.java.net/~xiaofeya/8087307/webrev.00/
> 
> Related product Changes:
> 
>https://bugs.openjdk.java.net/browse/JDK-8132026
> 
>https://bugs.openjdk.java.net/browse/JDK-8047814
> 
> Thanks,
> 
> Felix
> 
> 

RFR 8087307/9, new tests for ServiceLoader updates for module

[Felix Yang]

Hi there,

please review the new tests added for ServiceLoader updates for 
module system.


Test bug:

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

Webrev:

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

Related product Changes:

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

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

Thanks,

Felix

RFR 8178912, Throw away sample tests

[Felix Yang]

Hi there,

   please review following change to remove tests for several samples, 
which have been removed in "JEP 298: Remove Demos and Samples".


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

Bug: https://bugs.openjdk.java.net/browse/JDK-8178912

Corresponding JEP: https://bugs.openjdk.java.net/browse/JDK-8164813

Thanks,

Felix

RFR 8176195/9, Fix misc module dependencies in jdk_core tests

[Felix Yang]

Hi there,

   please review the changes to explicitly declare dependencies in 
tests. This will make better test selection with --limit-module option.


Bug:

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

Webrev:

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

I didn't use TEST.properties to declare dependency with jdk.charsets in 
jdk_nio tests, because there are still lots of tests in the same folder 
not depending it.


Thanks,

Felix

RFR 8173159, Problem list java/rmi/activation/ActivationGroup/downloadActivationGroup/DownloadActivationGroup.java on Windows

[Felix Yang]

Hi there,
   please review the  request to problem-list the test on Windows platform. It 
has been observed to be failing frequently.

Bug  https://bugs.openjdk.java.net/browse/JDK-8173159 


Thanks,
Felix


diff -r 1045f9722697 test/ProblemList.txt
--- a/test/ProblemList.txt  Sat Jan 21 03:53:21 2017 +
+++ b/test/ProblemList.txt  Fri Jan 20 21:15:34 2017 -0800
@@ -228,6 +228,8 @@
 
 java/rmi/activation/Activatable/extLoadedImpl/ext.sh8062724 
generic-all
 
+java/rmi/activation/ActivationGroup/downloadActivationGroup/DownloadActivationGroup.java
 8169569 windows-all
+
 sun/rmi/rmic/newrmic/equivalence/run.sh 8145980 
generic-all
 
 

Re: RFR 8172765, closed/javax/sound/sampled/Clip/AppletAudioClip/SoundBug.java failed in headless system

[Felix Yang]

oops! Incidentally sent to wrong alias. Sorry, please ignore it.

Thanks,
Felix
> On 13 Jan 2017, at 9:57 AM, Felix Yang <felix.y...@oracle.com> wrote:
> 
> Hi team,
>please review the patch to mark following test as headful. It has been 
> failing in Mach 5 for several runs
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8172765
> 
> Thanks,
> Felix
> 
> Patch:
> 
> diff -r 2ca43b220611 javax/sound/sampled/Clip/AppletAudioClip/SoundBug.java
> --- a/javax/sound/sampled/Clip/AppletAudioClip/SoundBug.java  Fri Jan 13 
> 01:36:12 2017 +
> +++ b/javax/sound/sampled/Clip/AppletAudioClip/SoundBug.java  Fri Jan 13 
> 09:34:46 2017 -0800
> @@ -11,7 +11,7 @@
>  * @test
>  * @bug 4181910
>  * @summary AudioClip incompatibility
> - * @key closed-binary
> + * @key closed-binary headful
>  */
> public class SoundBug extends Frame {
> 
> 

RFR 8172765, closed/javax/sound/sampled/Clip/AppletAudioClip/SoundBug.java failed in headless system

[Felix Yang]

Hi team,
please review the patch to mark following test as headful. It has been 
failing in Mach 5 for several runs

Bug: https://bugs.openjdk.java.net/browse/JDK-8172765

Thanks,
Felix

Patch:

diff -r 2ca43b220611 javax/sound/sampled/Clip/AppletAudioClip/SoundBug.java
--- a/javax/sound/sampled/Clip/AppletAudioClip/SoundBug.javaFri Jan 13 
01:36:12 2017 +
+++ b/javax/sound/sampled/Clip/AppletAudioClip/SoundBug.javaFri Jan 13 
09:34:46 2017 -0800
@@ -11,7 +11,7 @@
  * @test
  * @bug 4181910
  * @summary AudioClip incompatibility
- * @key closed-binary
+ * @key closed-binary headful
  */
 public class SoundBug extends Frame {

Re: RFR 8075884, new tests to check runtime usage with Multi-Release jars

[Felix Yang]

Hi Paul,
 thanks for the comments. Please review the updated patch. About JJS 
testing, I’m not really aware of Nashorn codes but just help to implement the 
designed one. Andrey may comment more. In my opinion, it looks to be a 
reasonable use case from functional aspect.

Webrev: http://cr.openjdk.java.net/~xiaofeya/8075884/webrev.01/ 
<http://cr.openjdk.java.net/~xiaofeya/8075884/webrev.01/>

Thanks,
Felix

> On 6 Jan 2017, at 11:26 AM, Paul Sandoz <paul.san...@oracle.com> wrote:
> 
> Hi Felix,
> 
> Generally looks good.
> 
> RuntimeTest
> —
> 
>  78 @BeforeTest
>  79 protected void setUpTest() throws Throwable {
> 
> Can you use @BeforeClass? since i believe the jar files only need to be 
> created once for all tests.
> 
> (And i presume it just overwrites any existing files that were previously 
> generated.)
> 
> 
> 172 if (mainVersionActual != mainVersionExpected) {
> 173 throw new AssertionError(
> 174 "Test failed: Expected Main class version: "
> 175 + mainVersionExpected + " Actual version: "
> 176 + mainVersionActual);
> 177 }
> 
> You can use Assert.equals.
> 
> 
> 191 @Test(dataProvider = "jarFiles")
> 192 void testJjs(String jar, int mainVer, int helperVer, int resVer)
> 193 throws Throwable {
> 
> What is the rational for testing with jjs? i.e. what does it test beyond the 
> other tests?
> 
> Paul.
> 
> 
>> On 5 Jan 2017, at 22:20, Felix Yang <felix.y...@oracle.com> wrote:
>> 
>> Hi there,
>> 
>>   please review the following new tests to check runtime usage with 
>> Multi-Release jars.
>> 
>> Bug:
>> 
>>   https://bugs.openjdk.java.net/browse/JDK-8075884
>> 
>> Webrev:
>> 
>>   http://cr.openjdk.java.net/~xiaofeya/8075884/webrev.00/
>> 
>> Thanks,
>> 
>> Felix
>> 
> 

RFR 8075884, new tests to check runtime usage with Multi-Release jars

[Felix Yang]

Hi there,

please review the following new tests to check runtime usage with 
Multi-Release jars.


Bug:

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

Webrev:

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

Thanks,

Felix

Ping - Re: RFR 8170890, Problem list tools/javadoc/CheckResourceKeys.java and tools/javadoc/ReleaseOption.java until fix for JDK-8170772

[Felix Yang]

Hi all,

any comment on this problem-list request?

Thanks,
Felix
On 2016/12/8 11:06, Felix Yang wrote:

Hi,

   please review the patch to problem-list two tier1 tests from Linux 
platform.


Bug:

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

Thanks,

Felix


diff -r 586c93260d3b test/ProblemList.txt
--- a/test/ProblemList.txt  Mon Dec 05 15:08:24 2016 -0800
+++ b/test/ProblemList.txt  Wed Dec 07 18:47:22 2016 -0800
@@ -31,6 +31,8 @@
 jdk/javadoc/tool/enum/enumType/Main.java 8152313 generic-all
convert to doclet test framework
 jdk/javadoc/tool/varArgs/Main.java 8152313generic-all convert to 
doclet test framework
 jdk/javadoc/doclet/testIOException/TestIOException.java 8164597
windows-all

+tools/javadoc/CheckResourceKeys.java 8170772linux-all
+tools/javadoc/ReleaseOption.java 8170772linux-all

 ### 


 #

Re: RFR 8169115, java/net/InetAddress/ptr/lookup.sh failed intermittently

[Felix Yang]

Langer and Dmitry,

   thanks for the review. Pushed with typo fixed

-Felix
On 2016/12/8 20:21, Langer, Christoph wrote:

Hi Felix,

looks good also for me.

Small typo:
85 throw new RuntimeException("Mistmatch between default"

Mistmatch -> Mismatch :)

Best regards
Christoph


-Original Message-
From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Felix
Yang
Sent: Donnerstag, 8. Dezember 2016 11:35
To: Dmitry Samersoff <dmitry.samers...@oracle.com>; core-libs-
d...@openjdk.java.net; net-...@openjdk.java.net; CHRIS.HEGARTY
<chris.hega...@oracle.com>
Subject: Re: RFR 8169115, java/net/InetAddress/ptr/lookup.sh failed
intermittently

Hi Dmitry,

 I tested your suggested "icann.org" and it indeed works well. Please
review the updated webrev:

http://cr.openjdk.java.net/~xiaofeya/8169115/webrev.02/

Thanks,
Felix
On 2016/12/6 19:16, Dmitry Samersoff wrote:

Felix,

1. I'm not sure that javaweb.sfbay.sun.com is the best domain name for
this test. Could we use different one (e.g. icann.org)

2. This test run JTREG -> Test VM -> Another VM. Could we optimize
process usage?

It might be better to create a jtreg "same vm" process that

 1. launch another process with -Djava.net.preferIPv4Stack=true
 that do A and PRT lookup in one run.

 2. Read results of process above, do PTR lookup with default
 settings and compare results.

-Dmitry


On 2016-12-06 12:06, Felix Yang wrote:

Hi,

 please review the following patch. It generally coverts codes from
shell to plain java.

Bug:

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

Webrev:

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

Thanks,

Felix

Re: RFR 8169115, java/net/InetAddress/ptr/lookup.sh failed intermittently

[Felix Yang]

Hi Dmitry,

   I tested your suggested "icann.org" and it indeed works well. Please 
review the updated webrev:


http://cr.openjdk.java.net/~xiaofeya/8169115/webrev.02/

Thanks,
Felix
On 2016/12/6 19:16, Dmitry Samersoff wrote:

Felix,

1. I'm not sure that javaweb.sfbay.sun.com is the best domain name for
this test. Could we use different one (e.g. icann.org)

2. This test run JTREG -> Test VM -> Another VM. Could we optimize
process usage?

It might be better to create a jtreg "same vm" process that

1. launch another process with -Djava.net.preferIPv4Stack=true
that do A and PRT lookup in one run.

2. Read results of process above, do PTR lookup with default
settings and compare results.

-Dmitry


On 2016-12-06 12:06, Felix Yang wrote:

Hi,

please review the following patch. It generally coverts codes from
shell to plain java.

Bug:

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

Webrev:

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

Thanks,

Felix

RFR 8170890, Problem list tools/javadoc/CheckResourceKeys.java and tools/javadoc/ReleaseOption.java until fix for JDK-8170772

[Felix Yang]

Hi,

   please review the patch to problem-list two tier1 tests from Linux 
platform.


Bug:

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

Thanks,

Felix


diff -r 586c93260d3b test/ProblemList.txt
--- a/test/ProblemList.txt  Mon Dec 05 15:08:24 2016 -0800
+++ b/test/ProblemList.txt  Wed Dec 07 18:47:22 2016 -0800
@@ -31,6 +31,8 @@
 jdk/javadoc/tool/enum/enumType/Main.java 8152313generic-all
convert to doclet test framework
 jdk/javadoc/tool/varArgs/Main.java 8152313generic-allconvert 
to doclet test framework
 jdk/javadoc/doclet/testIOException/TestIOException.java 8164597
windows-all

+tools/javadoc/CheckResourceKeys.java 8170772linux-all
+tools/javadoc/ReleaseOption.java 8170772linux-all

 ###
 #

Ping - Re: RFR 8043838, Test java/net/ServerSocket/AcceptCauseFileDescriptorLeak.java failed intermittently in nightly

[Felix Yang]

:-)

-Felix
> On 6 Dec 2016, at 9:28 AM, Felix Yang <felix.y...@oracle.com> wrote:
> 
> Add core-libs.
> 
> Thanks,
> Felix
> On 2016/12/5 22:14, Felix Yang wrote:
>> Hi,
>> 
>>   updated webrev. May I have a reviewer to review this
>> 
>> http://cr.openjdk.java.net/~xiaofeya/8043838/webrev.01/
>> 
>> -Felix
>> On 2016/12/5 15:50, Felix Yang wrote:
>>> 
>>> 
>>> On 2016/12/5 15:47, Langer, Christoph wrote:
>>>> Hi Felix,
>>>> 
>>>> looks ok to me.
>>>> 
>>>> You probably should remove the reference to the old shell script in 
>>>> comment line 25, though:
>>>> 
>>>> 25  * Test run from script, AcceptCauseFileDescriptorLeak.sh
>>> Christoph, Good catch!
>>> 
>>> Thanks,
>>> Felix
>>>> 
>>>> Best regards
>>>> Christoph
>>> 
>> 
> 

Re: RFR 8169115, java/net/InetAddress/ptr/lookup.sh failed intermittently

[Felix Yang]

Hi Dmitry,

thanks for the comments. My opinions inlined...

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

-Felix
On 2016/12/6 19:16, Dmitry Samersoff wrote:

Felix,

1. I'm not sure that javaweb.sfbay.sun.com is the best domain name for
this test. Could we use different one (e.g. icann.org)
Probably not. I'm not sure about the test design history, but 
"icann.org" may require the test environment(inside Oracle) to set up 
proxy to access internet, otherwise the test may be just skipped 
silently. "javaweb.sfbay.sun.com" is accessible both inside and outside 
Oracle network.


-Felix


2. This test run JTREG -> Test VM -> Another VM. Could we optimize
process usage?

It might be better to create a jtreg "same vm" process that

1. launch another process with -Djava.net.preferIPv4Stack=true
that do A and PRT lookup in one run.




2. Read results of process above, do PTR lookup with default
settings and compare results.

-Dmitry


On 2016-12-06 12:06, Felix Yang wrote:

Hi,

please review the following patch. It generally coverts codes from
shell to plain java.

Bug:

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

Webrev:

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

Thanks,

Felix

Re: RFR 8081390, javax/management/remote/mandatory/connection/RMIConnector_NPETest.java may leave orphaned processes

[Felix Yang]

Hi Roger,

thanks for the comments. I added more information to the comments.

-Felix
On 2016/12/6 23:45, Roger Riggs wrote:

Hi,

Looks fine, rmid will be destroyed in the finally clause.

!! There is no description in the bug and the open comments are 
unenlightening.

Attaching one of the failing logs would be have been useful.

Roger

On 12/6/2016 2:32 AM, Hamlin Li wrote:

I see, looks fine. But I'm not a reviewer.

Thank you
-Hamlin

On 2016/12/6 11:38, Felix Yang wrote:

Hi Hamlin,

   as stated in the bug, the timeout is more-likely a test setup 
issue that small timeout factor together with "-Xcomp".


But  in theory,  if not put rmid.start() in finally, it indeed 
possibly leaves orphaned processes. So that is still a minor problem 
to fix.


Thanks,
Felix
On 2016/12/6 11:26, Hamlin Li wrote:

Hi Felix,

As the issue is timeout at rmid.start(), so it does not resolve the 
issue to just move rmid.start() to try block.


I saw the issue happened last in 2015/6, maybe we could just close 
it as "Not Reproduced"?



Thank you
-Hamlin

On 2016/12/6 10:08, Felix Yang wrote:

Hi,

   please review the small fix to avoid  orphaned processes left.

Bug:

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

Webrev:

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

Thanks,

Felix

RFR 8169115, java/net/InetAddress/ptr/lookup.sh failed intermittently

[Felix Yang]

Hi,

   please review the following patch. It generally coverts codes from 
shell to plain java.


Bug:

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

Webrev:

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

Thanks,

Felix

Re: RFR 8081390, javax/management/remote/mandatory/connection/RMIConnector_NPETest.java may leave orphaned processes

[Felix Yang]

Hi Hamlin,

   as stated in the bug, the timeout is more-likely a test setup issue 
that small timeout factor together with "-Xcomp".


But  in theory,  if not put rmid.start() in finally, it indeed possibly 
leaves orphaned processes. So that is still a minor problem to fix.


Thanks,
Felix
On 2016/12/6 11:26, Hamlin Li wrote:

Hi Felix,

As the issue is timeout at rmid.start(), so it does not resolve the 
issue to just move rmid.start() to try block.


I saw the issue happened last in 2015/6, maybe we could just close it 
as "Not Reproduced"?



Thank you
-Hamlin

On 2016/12/6 10:08, Felix Yang wrote:

Hi,

   please review the small fix to avoid  orphaned processes left.

Bug:

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

Webrev:

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

Thanks,

Felix

RFR 8081390, javax/management/remote/mandatory/connection/RMIConnector_NPETest.java may leave orphaned processes

[Felix Yang]

Hi,

   please review the small fix to avoid  orphaned processes left.

Bug:

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

Webrev:

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

Thanks,

Felix

Re: RFR 8162521, java/net/Authenticator/B4933582.sh fails intermittently with BindException

[Felix Yang]

Hi Daniel,

   please review the new webrev:

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

Thanks,
Felix
On 2016/12/1 18:58, Daniel Fuchs wrote:

Hi Felix,

Good to see one more script converted to java.

I wonder if the fact that the test use to run two separate
JVMs was significant. Hopefully it's not.
According original bug 
(https://bugs.openjdk.java.net/browse/JDK-4933582), I didn't find out a 
strong reason to run with two different JVMs.
But original TestHttpServe.terminate() cannot free ports on Windows,  
that is why the shutdown section was adjusted. This could be a possible 
reason.


-Felix


The changes in TestHttpServer seem OK - this class is shared
by many other tests - so any modifications here is to be taken
with care. I trust you ran all the jdk_net tests?

Yes, tested on all platforms.

-Felix


In B4933582.java:

maybe MyAuthenticator.count should be made volatile (or AtomicInteger).
I don't think it's strictly necessary as I believe our implementation
will only call it in the client thread (which is always the main thread
here) - but it may be a better idea to use AtomicInteger anyway: just
make no assumption :-)

I'd also suggest to use try-with-resource to close in
CacheImpl (int port) and CacheImpl::writeMap to make sure
the file is always properly closed.

Updated all the history codes, as you suggested
Thanks,
Felix

best regards,

-- daniel


On 01/12/16 09:47, Felix Yang wrote:

Hi,

   please review the following patch. The code was converted from shell
script to plain Java. Since it is by-design to bind on a prior-used
port, add a few of re-tries for possible BindException.

Bug:

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

Webrev:

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

Thanks,
Felix

RFR 8162521, java/net/Authenticator/B4933582.sh fails intermittently with BindException

[Felix Yang]

Hi,

   please review the following patch. The code was converted from shell 
script to plain Java. Since it is by-design to bind on a prior-used 
port, add a few of re-tries for possible BindException.


Bug:

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

Webrev:

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

Thanks,
Felix

RFR 8170559, Incorrect bug id in problem list

[Felix Yang]

Hi there,

please review the change to correct a mistake. An incorrect bug id 
was incidentally used in problem list.


Bug: https://bugs.openjdk.java.net/browse/JDK-8170559

Thanks,

Felix


diff -r a563aaa85446 test/ProblemList.txt
--- a/test/ProblemList.txt  Wed Nov 30 17:15:58 2016 -0800
+++ b/test/ProblemList.txt  Wed Nov 30 17:23:24 2016 -0800
@@ -305,6 +305,6 @@
 # jdk_other

 com/sun/jndi/ldap/DeadSSLLdapTimeoutTest.java 8169942 
linux-i586,macosx-all,windows-x64
-javax/rmi/PortableRemoteObject/8146975/RmiIiopReturnValueTest.java 
8170248 linux-all
+javax/rmi/PortableRemoteObject/8146975/RmiIiopReturnValueTest.java 
8169737 linux-all


 

Re: RFR 8170248, Problem list javax/rmi/PortableRemoteObject/8146975/RmiIiopReturnValueTest.java

[Felix Yang]

Excuse me,

On 2016/11/23 13:15, Felix Yang wrote:

Hi,

   this test fails frequently on Linux platforms. I suggest to exclude 
it until JDK-8169737 is fixed.


Thanks,

Felix




Corrected diff:

diff -r 67d3235a317f test/ProblemList.txt
--- a/test/ProblemList.txt  Wed Nov 23 10:12:01 2016 +0800
+++ b/test/ProblemList.txt  Tue Nov 22 21:04:46 2016 -0800
@@ -305,5 +305,6 @@
 # jdk_other

 com/sun/jndi/ldap/DeadSSLLdapTimeoutTest.java 8141370 
linux-i586,macosx-all
+javax/rmi/PortableRemoteObject/8146975/RmiIiopReturnValueTest.java 
8169737 linux-all


 

RFR 8170248, Problem list javax/rmi/PortableRemoteObject/8146975/RmiIiopReturnValueTest.java

[Felix Yang]

Hi,

   this test fails frequently on Linux platforms. I suggest to exclude 
it until JDK-8169737 is fixed.


Thanks,

Felix


diff -r 67d3235a317f test/ProblemList.txt
--- a/test/ProblemList.txt  Wed Nov 23 10:12:01 2016 +0800
+++ b/test/ProblemList.txt  Tue Nov 22 20:44:56 2016 -0800
@@ -305,5 +305,6 @@
 # jdk_other

 com/sun/jndi/ldap/DeadSSLLdapTimeoutTest.java 8141370 
linux-i586,macosx-all
+javax/rmi/PortableRemoteObject/8146975/RmiIiopReturnValueTest.java 
8170248 linux-all


 

RFR 8170249/9, Problem list jdk/jshell/ToolFormatTest.java and jdk/jshell/ReplaceTest.java until 8170216 is fixed

[Felix Yang]

Hi there,

   please review the change to problem following tests on Soalris 
Sparcv9. They have been observed to be failing quite frequently on 
Solaris Sparcv9


Thanks,

Felix


diff -r 318dd5fce0ee test/ProblemList.txt
--- a/test/ProblemList.txt  Tue Nov 22 16:31:03 2016 -0800
+++ b/test/ProblemList.txt  Tue Nov 22 19:20:36 2016 -0800
@@ -39,6 +39,8 @@
 jdk/jshell/EditorPadTest.java 8161276windows-allTest set-up 
cannot press buttons
 jdk/jshell/ToolBasicTest.java 8139873generic-allJShell tests 
failing

 jdk/jshell/ExternalEditorTest.java 8170108generic-all
+jdk/jshell/ToolFormatTest.java 8170216solaris-sparcv9
+jdk/jshell/ReplaceTest.java 8170216solaris-sparcv9

 ###
 #

Re: RFR (JAXP) JDK-8167478 javax/xml/jaxp/unittest/parsers/Bug6341770.java failed with "java.security.AccessControlException: access denied ("java.io.FilePermission" "sko?ice")"

[Felix Yang]

A comment on naming, alpha to ALPHA ?
-Felix

> 在 2016年10月13日，17:05，Frank Yuan  写道：
> 
> Hi all
> 
> 
> 
> Would you like to review http://cr.openjdk.java.net/~fyuan/8167478/webrev.00/ 
> ?
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8167478 
> 
> 
> 
> This is a test bug, because Bug6341770.java is invalid when the system 
> environment doesn't support non-ascii characters, the test
> will exit immediately in this condition.
> 
> 
> 
> 
> 
> Thanks,
> 
> 
> 
> Frank
> 
> 
> 

RFR 8157135, Fix module dependencies javax/* EE tests

[Felix Yang]

Hi all,

   please review the patch for some tests, which explicitly declare 
module dependencies to EE modules.


Bug: https://bugs.openjdk.java.net/browse/JDK-8157135

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

This is a partial fix left by 
https://bugs.openjdk.java.net/browse/JDK-8155088. The fix had been hold 
off  for a moment, to wait for new jtreg with resolved 
https://bugs.openjdk.java.net/browse/CODETOOLS-7901671. Otherwise those 
tests will be ignored silently.


Thanks,

Felix

RFR 8157816, Mark 4 httpclient tests as intermittently failing

[Felix Yang]

Hi all,
   please review the change to mark following tests with keyword 
'intermittent'. These tests have been observed to fail intermittently 
for a while.

test/java/net/httpclient/SplitResponse.java
test/java/net/httpclient/http2/BasicTest.java
test/java/net/httpclient/http2/ErrorTest.java
test/java/net/httpclient/http2/TLSConnection.java

Bug:  https://bugs.openjdk.java.net/browse/JDK-8157816
Webrev: http://cr.openjdk.java.net/~xiaofeya/8157816/webrev.00/

Thanks,
Felix

Also attached the diff:

diff -r ee29aaab5889 test/java/net/httpclient/SplitResponse.java
--- a/test/java/net/httpclient/SplitResponse.java   Mon May 30 
14:58:59 2016 +0900
+++ b/test/java/net/httpclient/SplitResponse.java   Sun May 29 
23:49:16 2016 -0700

@@ -33,6 +33,7 @@
 /**
  * @test
  * @bug 8087112
+ * @key intermittent
  * @build Server
  * @run main/othervm -Djava.net.HttpClient.log=all SplitResponse
  */
diff -r ee29aaab5889 test/java/net/httpclient/http2/BasicTest.java
--- a/test/java/net/httpclient/http2/BasicTest.java Mon May 30 
14:58:59 2016 +0900
+++ b/test/java/net/httpclient/http2/BasicTest.java Sun May 29 
23:49:16 2016 -0700

@@ -24,6 +24,7 @@
 /*
  * @test
  * @bug 8087112
+ * @key intermittent
  * @library /lib/testlibrary
  * @build jdk.testlibrary.SimpleSSLContext
  * @modules java.httpclient
diff -r ee29aaab5889 test/java/net/httpclient/http2/ErrorTest.java
--- a/test/java/net/httpclient/http2/ErrorTest.java Mon May 30 
14:58:59 2016 +0900
+++ b/test/java/net/httpclient/http2/ErrorTest.java Sun May 29 
23:49:16 2016 -0700

@@ -24,6 +24,7 @@
 /*
  * @test
  * @bug 8157105
+ * @key intermittent
  * @library /lib/testlibrary
  * @build jdk.testlibrary.SimpleSSLContext
  * @modules java.httpclient
diff -r ee29aaab5889 test/java/net/httpclient/http2/TLSConnection.java
--- a/test/java/net/httpclient/http2/TLSConnection.java Mon May 30 
14:58:59 2016 +0900
+++ b/test/java/net/httpclient/http2/TLSConnection.java Sun May 29 
23:49:16 2016 -0700

@@ -39,6 +39,7 @@
 /*
  * @test
  * @bug 8150769 8157107
+ * @key intermittent
  * @summary Checks that SSL parameters can be set for HTTP/2 connection
  * @modules java.httpclient
  * @compile/module=java.httpclient java/net/http/Http2Handler.java

Re: RFR 8155088, Fix module dependencies in java/sql/* and javax/* tests

[Felix Yang]

Hi Alan,

   updated the fix. Divide into two webrevs for 
https://bugs.openjdk.java.net/browse/CODETOOLS-7901671


Webrevs:

http://cr.openjdk.java.net/~xiaofeya/8155088/webrev.02_se/

http://cr.openjdk.java.net/~xiaofeya/8155088/webrev.02_ee/

I suggest to push the first one currently. And file another issue to 
track latter one. Because without fix for CODETOOLS-7901671( in next 
jtreg promotion ), those tests with EE modules in @modules will be 
skipped by default.


Thanks,

Felix

On 2016/5/4 23:44, Felix Yang wrote:

Hi Alan,


On 2016/5/4 22:45, Alan Bateman wrote:



On 04/05/2016 15:39, Felix Yang wrote:

Hi Alan,

please review the updated webrev. Reverted changes for those 
tests with "-addmods".


http://cr.openjdk.java.net/~xiaofeya/8155088/webrev.01/
For the javax.transaction test then don't you also add 
"java.transaction" in the @modules value? The rest looks fine.
I'm a bit confused here. There is "-addmods java.transaction" for 
@compile. Is it still necessary to declare in @modules?
Actually, if add java.transaction in the @modules, the test will be 
skipped by jtreg. Filed 
https://bugs.openjdk.java.net/browse/CODETOOLS-7901671 for jtreg, 
because it looks to be not a good practice for either situation.


-Felix


-Alan

Re: RFR 8155088, Fix module dependencies in java/sql/* and javax/* tests

[Felix Yang]

Hi Alan,


On 2016/5/4 22:45, Alan Bateman wrote:



On 04/05/2016 15:39, Felix Yang wrote:

Hi Alan,

please review the updated webrev. Reverted changes for those 
tests with "-addmods".


http://cr.openjdk.java.net/~xiaofeya/8155088/webrev.01/
For the javax.transaction test then don't you also add 
"java.transaction" in the @modules value? The rest looks fine.
I'm a bit confused here. There is "-addmods java.transaction" for 
@compile. Is it still necessary to declare in @modules?
Actually, if add java.transaction in the @modules, the test will be 
skipped by jtreg. Filed 
https://bugs.openjdk.java.net/browse/CODETOOLS-7901671 for jtreg, 
because it looks to be not a good practice for either situation.


-Felix


-Alan

Re: RFR 8155088, Fix module dependencies in java/sql/* and javax/* tests

[Felix Yang]

Hi Alan,

please review the updated webrev. Reverted changes for those tests 
with "-addmods".


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

Thanks,
Felix
On 2016/4/29 15:25, Alan Bateman wrote:

On 29/04/2016 03:16, Felix Yang wrote:

Hi there,

please review the changes to explicitly declare module 
dependencies for "java/sql/*" and "javax/*" tests;


Bug: https://bugs.openjdk.java.net/browse/JDK-8155088

Webrev: http://cr.openjdk.java.net/~xiaofeya/8155088/webrev.00/
Felix - would it be disruptive to you if I asked to hold off on this 
until we get the changes in jake pushed to jdk9/dev (next week). The 
reason is that this we've changed most of these tests in jake to use 
-addmods. We've also replaced the TEST.properties for the 
javax.transaction tests so that they are run with a driver class 
instead. This is also related to the policy for root modules which 
impacts the tests that we have for the EE modules in the jdk repo.


-Alan

Re: RFR 8155088, Fix module dependencies in java/sql/* and javax/* tests

[Felix Yang]

On 2016/4/29 15:25, Alan Bateman wrote:

On 29/04/2016 03:16, Felix Yang wrote:

Hi there,

please review the changes to explicitly declare module 
dependencies for "java/sql/*" and "javax/*" tests;


Bug:  https://bugs.openjdk.java.net/browse/JDK-8155088

Webrev: http://cr.openjdk.java.net/~xiaofeya/8155088/webrev.00/
Felix - would it be disruptive to you if I asked to hold off on this 
until we get the changes in jake pushed to jdk9/dev (next week). The 
reason is that this we've changed most of these tests in jake to use 
-addmods. We've also replaced the TEST.properties for the 
javax.transaction tests so that they are run with a driver class 
instead. This is also related to the policy for root modules which 
impacts the tests that we have for the EE modules in the jdk repo.

Alan,
 it is OK. Libs SQE will hold off to wait the changes in.

Thanks for the info,
Felix


-Alan

RFR 8155088, Fix module dependencies in java/sql/* and javax/* tests

[Felix Yang]

Hi there,

please review the changes to explicitly declare module dependencies 
for "java/sql/*" and "javax/*" tests;


Bug:  https://bugs.openjdk.java.net/browse/JDK-8155088

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

Thanks,

Felix

Re: RFR 8154733, Fix module dependencies missed in java.rmi tests

[Felix Yang]

Hi Amy,

thanks for pointing this out. Updated webrev:

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

Felix
On 2016/4/26 17:21, Amy Lu wrote:

Hi, Felix

With modules declares in TEST.propertiesshould avoid have to modify 
test file one by one... Maybe I missed things please correct me.


Example:
$ cat jdk/test/java/rmi/TEST.properties
modules = java.rmi

Thanks,
Amy

On 4/26/16 4:53 PM, Felix Yang wrote:

Hi all,

please review the fix to explicitly declare module dependencies 
for rmi tests.


Bug: https://bugs.openjdk.java.net/browse/JDK-8154733
Webrev: http://cr.openjdk.java.net/~xiaofeya/8154733/webrev.00/

Thanks,
Felix

RFR 8154733, Fix module dependencies missed in java.rmi tests

[Felix Yang]

Hi all,

please review the fix to explicitly declare module dependencies for 
rmi tests.


Bug: https://bugs.openjdk.java.net/browse/JDK-8154733
Webrev: http://cr.openjdk.java.net/~xiaofeya/8154733/webrev.00/

Thanks,
Felix

Re: RFR 8146758, NetworkInterfaceStreamTest.java fails intermittently at comparing network interfaces

[Felix Yang]

Hi Chris,
thanks for the review. Could you sponsor the change?

Thanks,
Felix
On 2016/4/15 18:45, Chris Hegarty wrote:

On 15/04/16 10:29, Felix Yang wrote:

Hi all,
please review the following fix. It is an intermittent failure
because of Teredo Tunneling Pseudo-Interface.

Bug: https://bugs.openjdk.java.net/browse/JDK-8146758
Webrev: http://cr.openjdk.java.net/~xiaofeya/8146758/webrev.00/


Looks ok to me.

-Chris.

RFR 8146758, NetworkInterfaceStreamTest.java fails intermittently at comparing network interfaces

[Felix Yang]

Hi all,
   please review the following fix. It is an intermittent failure 
because of Teredo Tunneling Pseudo-Interface.


Bug: https://bugs.openjdk.java.net/browse/JDK-8146758
Webrev: http://cr.openjdk.java.net/~xiaofeya/8146758/webrev.00/

Thanks,
Felix

Re: RFR 8153928, test/lib/share/classes/jdk/test/lib/Utils.java introduced dependency to java.base/jdk.internal.misc

[Felix Yang]

Hi Alan and Amy,
thanks for figuring this out. Updated to suggested practice.

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

Felix

On 2016/4/11 14:46, Alan Bateman wrote:

On 11/04/2016 06:04, Felix Yang wrote:

Amy,
thanks. I'm not sure which practices are suggested. By searching 
the existing tests, I found lots of test with 2+ @modules, so I chose 
to add another @modules. Personally, both ways look clear and 
transparent for me.
We've tried to use one @modules per test but there are 
inconsistencies. I think some of those inconsistencies arose when 
patches were brought into jdk9/dev early and then subsequently merged. 
At some point we should do a pass over the existing usages to get them 
consistent.


-Alan

Re: RFR 8153928, test/lib/share/classes/jdk/test/lib/Utils.java introduced dependency to java.base/jdk.internal.misc

[Felix Yang]

Amy,
thanks. I'm not sure which practices are suggested. By searching 
the existing tests, I found lots of test with 2+ @modules, so I chose to 
add another @modules. Personally, both ways look clear and transparent 
for me.


Thanks,
Felix
On 2016/4/11 12:49, Amy Lu wrote:

On 4/11/16 11:47 AM, Felix Yang wrote:

Hi there,
please review the following bug fix.
Bug: https://bugs.openjdk.java.net/browse/JDK-8153928
Webrev: http://cr.openjdk.java.net/~xiaofeya/8153928/webrev.00/


You might want to do some cleanup to avoid using two @modules tags?

BTW. I'm not openjdk reviewer, please wait for reviewer's feedback.

Thanks,
Amy



After changes in JDK-8153737 
<https://bugs.openjdk.java.net/browse/JDK-8153737>, it introduced new 
dependency of java.base/jdk.internal.misc in 
test/lib/share/classes/jdk/test/lib/Utils.java. This leads to two 
test compilation failures.

See changeset: http://hg.openjdk.java.net/jdk9/dev/rev/1d992540870f

Thanks,
Felix

Re: [8u-dev] Request for Approval for Backport: 8151352 (8151574), jdk/test/sample fails with "effective library path is outside the test suite"

[Felix Yang]

Hi Sean,
 thank you very much. BTW, I will look into 
https://bugs.openjdk.java.net/browse/JDK-8151535 you mentioned.


-Felix
On 2016/3/16 23:01, Seán Coffey wrote:


On 15/03/16 14:38, Felix Yang wrote:

Hi Sean,
thank you for the review. Could you sponsor this change?


Pushed : http://hg.openjdk.java.net/jdk8u/jdk8u-dev/jdk/rev/2e0399f66ddc

regards,
Sean.


-Felix
On 2016/3/15 16:03, Seán Coffey wrote:
Please don't use backport IDs in email communication or approval 
requests. Use the master bug ID in your changeset commit comment. 
I've edited the subject line to include master bug ID (8151352). 
Approved.


The jtreg upgrade disrupted test results for quite a few teams 
(again). https://bugs.openjdk.java.net/browse/JDK-8151535 is another 
one that needs attention.


Regards,
Sean.

On 15/03/2016 05:44, Felix Yang wrote:

Hi there,
please approve the backport of 8151574 from 9 to 8.

Webrev: http://cr.openjdk.java.net/~xiaofeya/8151574/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8151574
Changeset in jdk9: 
http://hg.openjdk.java.net/jdk9/dev/jdk/rev/dbb0fd7f2a2b
Review: 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-March/039332.html


Thanks,
Felix

[8u-dev] RFR 8151535: java/lang/invoke/AccessControlTest.java should be modified to run with JTREG 4.1 b13

[Felix Yang]

Hi there,
please, help to review the fix for test 
'java/lang/invoke/AccessControlTest.java'.


Bug:
https://bugs.openjdk.java.net/browse/JDK-8151535
Webrev:
http://cr.openjdk.java.net/~xiaofeya/8151535/webrev.00/

This fix removed "@library ../../../...", which is invalid with the 
latest change in jtreg. See 
https://bugs.openjdk.java.net/browse/CODETOOLS-7901585.

I need a sponsor for this change.

Thanks,
Felix

Re: [8u-dev] Request for Approval for Backport: 8151352 (8151574), jdk/test/sample fails with "effective library path is outside the test suite"

[Felix Yang]

Hi Sean,
thank you for the review. Could you sponsor this change?

-Felix
On 2016/3/15 16:03, Seán Coffey wrote:
Please don't use backport IDs in email communication or approval 
requests. Use the master bug ID in your changeset commit comment. I've 
edited the subject line to include master bug ID (8151352). Approved.


The jtreg upgrade disrupted test results for quite a few teams 
(again). https://bugs.openjdk.java.net/browse/JDK-8151535 is another 
one that needs attention.


Regards,
Sean.

On 15/03/2016 05:44, Felix Yang wrote:

Hi there,
please approve the backport of 8151574 from 9 to 8.

Webrev: http://cr.openjdk.java.net/~xiaofeya/8151574/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8151574
Changeset in jdk9: 
http://hg.openjdk.java.net/jdk9/dev/jdk/rev/dbb0fd7f2a2b
Review: 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-March/039332.html


Thanks,
Felix

[8u-dev] Request for Approval for Backport: 8151574, jdk/test/sample fails with "effective library path is outside the test suite"

[Felix Yang]

Hi there,
please approve the backport of 8151574 from 9 to 8.

Webrev: http://cr.openjdk.java.net/~xiaofeya/8151574/webrev.00/
Bug: https://bugs.openjdk.java.net/browse/JDK-8151574
Changeset in jdk9: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/dbb0fd7f2a2b
Review: 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-March/039332.html


Thanks,
Felix

Ping - Re: RFR 8151676/9, Add jdk/internal/jline/console/StripAnsiTest.java into problem list for MacOSX

[Felix Yang]

Hi,
can we get either exclusion request or Jan's fix reviewed? This 
hanging test keeps on affecting other tests on some Mac OSX environments.


Thanks,
Felix
On 2016/3/11 16:59, Felix Yang wrote:

Hi Jan,
we have seen test runs hanging 15+ hours on several environments, 
which blocks other testing. So is it OK to exclude it before the fix 
ready?


Thanks,
Felix
On 2016/3/11 16:12, Jan Lahoda wrote:

Hi,

I think I'd prefer to try to fix that. I think there's a good chance 
this fix:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-March/039165.html 



will fix also the Mac OSX problem. Sadly, I didn't hear any feedback 
on that yet.


Thanks,
   Jan

On 11.3.2016 08:15, Felix Yang wrote:

Hi,
 please review at your convenience.
This test
 jdk/internal/jline/console/StripAnsiTest.java
has been observed hanging on several Mac OSX environments. Suggest to
temporarily put into problemlist for Mac OSX.

Bug:
 https://bugs.openjdk.java.net/browse/JDK-8151676

Thanks,
Felix

Patch:

diff -r 13be3e542a07 test/ProblemList.txt
--- a/test/ProblemList.txt  Thu Mar 10 11:52:54 2016 -0800
+++ b/test/ProblemList.txt  Thu Mar 10 22:23:52 2016 -0800
@@ -414,6 +414,10 @@

  # jdk_other

+# 8150519
+
+jdk/internal/jline/console/StripAnsiTest.java macosx-all
+
 



  # 8141370

Re: RFR 8151676/9, Add jdk/internal/jline/console/StripAnsiTest.java into problem list for MacOSX

[Felix Yang]

Hi Jan,
we have seen test runs hanging 15+ hours on several environments, 
which blocks other testing. So is it OK to exclude it before the fix ready?


Thanks,
Felix
On 2016/3/11 16:12, Jan Lahoda wrote:

Hi,

I think I'd prefer to try to fix that. I think there's a good chance 
this fix:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-March/039165.html 



will fix also the Mac OSX problem. Sadly, I didn't hear any feedback 
on that yet.


Thanks,
   Jan

On 11.3.2016 08:15, Felix Yang wrote:

Hi,
 please review at your convenience.
This test
 jdk/internal/jline/console/StripAnsiTest.java
has been observed hanging on several Mac OSX environments. Suggest to
temporarily put into problemlist for Mac OSX.

Bug:
 https://bugs.openjdk.java.net/browse/JDK-8151676

Thanks,
Felix

Patch:

diff -r 13be3e542a07 test/ProblemList.txt
--- a/test/ProblemList.txt  Thu Mar 10 11:52:54 2016 -0800
+++ b/test/ProblemList.txt  Thu Mar 10 22:23:52 2016 -0800
@@ -414,6 +414,10 @@

  # jdk_other

+# 8150519
+
+jdk/internal/jline/console/StripAnsiTest.java macosx-all
+


  # 8141370

RFR 8151676/9, Add jdk/internal/jline/console/StripAnsiTest.java into problem list for MacOSX

[Felix Yang]

Hi,
please review at your convenience.
This test
jdk/internal/jline/console/StripAnsiTest.java
has been observed hanging on several Mac OSX environments. Suggest to 
temporarily put into problemlist for Mac OSX.


Bug:
https://bugs.openjdk.java.net/browse/JDK-8151676

Thanks,
Felix

Patch:

diff -r 13be3e542a07 test/ProblemList.txt
--- a/test/ProblemList.txt  Thu Mar 10 11:52:54 2016 -0800
+++ b/test/ProblemList.txt  Thu Mar 10 22:23:52 2016 -0800
@@ -414,6 +414,10 @@

 # jdk_other

+# 8150519
+
+jdk/internal/jline/console/StripAnsiTest.java macosx-all
+
 

 # 8141370

Re: RFR 8151352, jdk/test/sample fails with "effective library path is outside the test suite"

[Felix Yang]

Joe,
 thank you for the quick review.

Amy,
 could you sponsor this change?

-Felix
On 2016/3/8 2:51, joe darcy wrote:

Hello,

Looks fine; thanks,

-Joe

On 3/7/2016 8:04 AM, Felix Yang wrote:

Hi all,
   please review the fix for two tests under "test/sample/".

Bug:
https://bugs.openjdk.java.net/browse/JDK-8151352
Webrev:
http://cr.openjdk.java.net/~xiaofeya/8151352/webrev.00/

Original declaration, "@library ../../../src/sample...", is invalid 
with the latest change in jtreg. See 
https://bugs.openjdk.java.net/browse/CODETOOLS-7901585. This fix 
doesn't resolve dependency to "src/sample", but only converts them 
into testng tests and declares "external.lib.roots" to avoid dot-dot.


Thanks,
Felix

RFR 8151352, jdk/test/sample fails with "effective library path is outside the test suite"

[Felix Yang]

Hi all,
   please review the fix for two tests under "test/sample/".

Bug:
https://bugs.openjdk.java.net/browse/JDK-8151352
Webrev:
http://cr.openjdk.java.net/~xiaofeya/8151352/webrev.00/

Original declaration, "@library ../../../src/sample...", is invalid with 
the latest change in jtreg. See 
https://bugs.openjdk.java.net/browse/CODETOOLS-7901585. This fix doesn't 
resolve dependency to "src/sample", but only converts them into testng 
tests and declares "external.lib.roots" to avoid dot-dot.


Thanks,
Felix

Re: RFR 8065076/9, test/java/net/SocketPermission/SocketPermissionTest.java failed intermittently

[Felix Yang]

Hi Chris,
 your fix is cool. I will assign the bug to you:)
 a comment on this fix. The test changed system SecurityManager and it is 
not executed with othervm mode. I think you need to rollback the change after 
test. Otherwise it may affect other tests.

Thanks,
Felix
> On Jan 20, 2016, at 12:45 PM, Chris Hegarty <chris.hega...@oracle.com> wrote:
> 
> On 20 Jan 2016, at 06:36, Chris Hegarty <chris.hega...@oracle.com 
> <mailto:chris.hega...@oracle.com>> wrote:
> 
>> Felix,
>> 
>> On 14 Jan 2016, at 06:07, Felix Yang <felix.y...@oracle.com> wrote:
>> 
>>> Hi all,
>>>  please review the fix for 
>>> test/java/net/SocketPermission/SocketPermissionTest.java, which fails 
>>> frequently with "java.net.BindException: Address already in use".
>>> 
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8065076
>>> Webrev: http://cr.openjdk.java.net/~xiaofeya/8065076/webrev.00
>> 
>> My preference is to avoid getFreePort. It is problematic and I believe just 
>> obfuscates
>> the intermittent failures further.
>> 
>> In many of the test scenarios the “listening” socket can be created before 
>> the specific
>> access control context and associated permission are created.
>> 
>> I’ll see if I can get some time to try this out.
> 
> I spent a little time on this today. I basically rewrote the test, but kept 
> the 
> same test scenarios. The use of data providers was cute, but not workable
> since there is no common supertype for the socket classes. I decided to 
> just expand out the test cases manually. This will give the same test
> coverage, but should be stable since it creates the sockets first, on an 
> ephemeral port, and then constructs the permissions appropriately given
> that port.
> 
> http://cr.openjdk.java.net/~chegar/8065076/ 
> <http://cr.openjdk.java.net/~chegar/8065076/>
> 
> The webrev diffs are almost useless, just review the new file, and compare
> test scenarios against the what is in the old file.
> 
> -Chris.

RFR 8065076/9, test/java/net/SocketPermission/SocketPermissionTest.java failed intermittently

[Felix Yang]

Hi all,
please review the fix for 
test/java/net/SocketPermission/SocketPermissionTest.java, which fails 
frequently with "java.net.BindException: Address already in use".


Bug: https://bugs.openjdk.java.net/browse/JDK-8065076
Webrev: http://cr.openjdk.java.net/~xiaofeya/8065076/webrev.00

The test uses getFreePort() test util library to verify permission on 
specific "host:port".  This fix added retries to avoid possible binding 
failures:

1. A port may be still occupied for a short moment after socket close.
2. Another application is possible to occupy the same port during the 
short time window of open-close-reuse. This has been observed in 
experimental test runs.


Thanks,
Felix

Re: RFR: 8142996: move jdk java/util/streams tests into java.base directories

[Felix Yang]

Hi Alan and Jonathon,
the fix was pushed without addressing this comment, so 5 tests has 
been broken. Just filed https://bugs.openjdk.java.net/browse/JDK-8143015.


-Felix
On 2015/11/14 15:30, Alan Bateman wrote:



On 13/11/2015 23:46, Jonathan Gibbons wrote:
Please review the following fix to relocate some of the files in the 
jdk/test/java/util/streams directory.


JIRA: https://bugs.openjdk.java.net/browse/JDK-8142996
Webrev: http://cr.openjdk.java.net/~jjg/8142996/webrev.00/

Although the patch touches a lot of files, only the TEST.properties 
files in the webrev were modified (edited). The remaining changes are 
the result of executing the following commands:


hg rename test/java/util/stream/bootlib/java 
test/java/util/stream/bootlib/java.base/java
hg rename test/java/util/stream/boottest/java 
test/java/util/stream/boottest/java.base/java
There are a few tests that use the streams test framework so they will 
need to be changed too:


 java/net/NetworkInterface/NetworkInterfaceStreamTest.java
 java/nio/file/Files/StreamLinesTest.java
 java/security/PermissionCollection/PermissionCollectionStreamTest.java
 java/util/Scanner/ScannerStreamTest.java
 java/util/regex/PatternStreamTest.java

Can these be updated as part of this push?

-Alan

RFR 8143015/9: 5 tests fail with error "Can't find source for class: java.util.stream.OpTestCase"

[Felix Yang]

Hi,
please review the following fix for 5 broken test cases, which was 
introduced by JDK-8142996. These tests refer with 
java.util.stream.OpTestCase, which was relocated from 
test/java/util/stream/bootlib/ to test/java/util/stream/bootlib/java.base/


Bug: https://bugs.openjdk.java.net/browse/JDK-8143015
Webrev: http://cr.openjdk.java.net/~xiaofeya/8143015/webrev.00/

Thanks,
Felix

Re: [9] RFR 8129833: Need basic tests for rmic

[FELIX YANG]

On 7/15/2015 7:01 PM, Daniel Fuchs wrote:

On 13/07/15 14:53, FELIX YANG wrote:

Hi Daniel,
  please help to review the change for 8129833.

Issue:https://bugs.openjdk.java.net/browse/JDK-8129833
Patch:http://cr.openjdk.java.net/~fyuan/felix/8129833/

The patch add a new class to try rmic with. It covers a problem of
locating the class java.awt.Panel in jigsaw build, see
https://bugs.openjdk.java.net/browse/JDK-8129779.


Hi Felix, it looks good to me.

Would you like me to sponsor it?

Daniel,
 yes, please!

Thank you,
-Felix


best regards,

-- daniel



thanks,
-Felix

[9] RFR 8129833: Need basic tests for rmic

[FELIX YANG]

Hi Daniel,
 please help to review the change for 8129833.

Issue:https://bugs.openjdk.java.net/browse/JDK-8129833
Patch:http://cr.openjdk.java.net/~fyuan/felix/8129833/

The patch add a new class to try rmic with. It covers a problem of locating the 
class java.awt.Panel in jigsaw build, see 
https://bugs.openjdk.java.net/browse/JDK-8129779.

thanks,
-Felix

[9]RFR 8130394: DatagramChannel tests need to be hardened to ignore stray datagrams

[FELIX YANG]

Hi all,
 please help to review the change for 8130394.

Issue: https://bugs.openjdk.java.net/browse/JDK-8130394
Patch:http://cr.openjdk.java.net/~fyuan/felix/8130394/

The patch updates tests to ignore stray datagrams, or at least print 
more information to ease troubleshooting.


thanks,
-Felix

RFR JDK-8079778: Mark intermittently failing: java/rmi/activation/rmidViaInheritedChannel/RmidViaInheritedChannel.java

[FELIX YANG]

Hi all,
java/rmi/activation/rmidViaInheritedChannel/RmidViaInheritedChannel.java 
has been observed to fail intermittently. This fix is to mark it with 
keyword 'intermittent'.


Bug: https://bugs.openjdk.java.net/browse/JDK-8079778

thanks,
-Felix

diff -r d18205a1ef80 
test/java/rmi/activation/rmidViaInheritedChannel/RmidViaInheritedChannel.java
--- 
a/test/java/rmi/activation/rmidViaInheritedChannel/RmidViaInheritedChannel.java 
Fri May 08 09:05:15 2015 -0400
+++ 
b/test/java/rmi/activation/rmidViaInheritedChannel/RmidViaInheritedChannel.java 
Fri May 08 15:47:53 2015 +0100

@@ -29,6 +29,7 @@
  * @library ../../testlibrary
  * @build TestLibrary RMID ActivationLibrary
  * @run main/othervm/timeout=240 RmidViaInheritedChannel
+ * @key intermittent
  */

 import java.io.IOException;

RFR 8061448, Update sun/misc/JarIndex tests to cleanup the check for the jre directory

[FELIX YANG]

Hi all,
   please review the fix for 8061448. It cleans up dependency with 
jre directory and tools.jar, which have been removed by Jigsaw feature.


Bug:
https://bugs.openjdk.java.net/browse/JDK-8061448
Webrev:
http://cr.openjdk.java.net/~xiaofeya/8061448/webrev.00/

I needs a sponsor for this change.

Thanks,
Felix

Re: RFR 8061297 : sun/reflect/CallerSensitive/CallerSensitiveFinder.java should use the JRT FileSystem

[FELIX YANG]

Chris,
 excuse me,  I'm not a reviewer but have some questions.
 at line 209, classes is actually trying to resolve a path like 
c:\jdk1.9.0\modules, which never exists in JDK directory. It should be 
c:\jdk1.9.0\lib\modules.


 207 if (home.resolve(lib).toFile().exists()) {
 208 // either an exploded build or an image
 *209 File classes = home.resolve(modules).toFile();*
 210 if (classes.isDirectory()) {
 211 return Stream.of(classes.toPath());
 212 } else {
 213 return JrtPaths();
 214 }

The test works because it always go to JrtPaths method. The jimages have been 
mounted at initializing of Jrt file system.
There may be two issues here:
1. the naming*J*rtPaths doesn't follow common rules. Is it more suitable 
with*j*rtPaths()?
2. Even when changing classes to lib/modules, it is still probably unable to 
process class files. I tried last code and failed.
*File classes = home.resolve(lib/modules).toFile();*
Because, com.sun.tools.jdeps.ClassFileReader hasn't been upgraded to correctly 
handle Jrt image files.

So this logic is either unnecessary (as stated above, the jimages have been 
mounted at initializing of Jrt file system) or unable to work now.

Thanks,

-Felix
On 1/15/2015 12:31 AM, Chris Hegarty wrote:

On 14/01/15 16:18, Mandy Chung wrote:

On 1/14/2015 8:04 AM, Chris Hegarty wrote:


http://cr.openjdk.java.net/~chegar/8061297/webrev.01/webrev/


Looks okay to me.

In CallerSensitiveFinder.java line 57, 80, 137 - not sure if you were
thinking to pass pool to the CallerSensitiveFinder constructor. Looks
like some editing you meant to cleanup.

   nit: line 76 a space between classes and ==
   line 207 can be removed.


D'oh! Fixed both of these. Webrev updated in place.

-Chris.


Mandy

Re: RFR 8066085: Need a sanity test for rmic -iiop

[FELIX YANG]

Hi Stuart,
 this is the updated webrev: 
http://cr.openjdk.java.net/~xiaofeya/8066085/webrev.02/. Please sponsor 
this changeset for me.


Thank you very much,
-Felix
On 12/24/2014 1:01 PM, Stuart Marks wrote:

Hi Felix,

Good improvements. I think this is pretty close; I have just a few 
minor comments.


* The test is named IIOPSanityTest but it's in a directory named 
iiopCompilation. I don't think we need different names. There are many 
test classes named Foo that reside in a directory named foo; I 
think we should do the same here. I think keeping the directory name 
iiopCompilation and renaming the class to IIOPCompilation makes 
things a bit more descriptive.


* Declaring string constants like RMIC_PROGRAM, IIOP_OPTION, and 
CP_OPTION doesn't really help things, since they're each used exactly 
once, the string values are unlikely to change, and the symbolic names 
are just derived from the string vaues. You might as well use string 
literals directly in each place they're needed.


* Printing the command that's about to be executed is a good idea, but 
the printf statement that prints the command has to be kept in synch 
with the array containing the command and args. If the command were to 
change, it'd be easy to forget to update the printf statement or to 
get it wrong.


* I'll also observe that there is an overload of the ProcessBuilder 
constructor that takes a ListString instead of a String[]. Lists are 
often easier to work with than arrays, and they have a toString() 
method that can be used implicitly by print statements. This suggests 
an alternative for constructing the rmic command line, something like 
the following:


ListString command = Arrays.asList(
rmicProgramStr, -iiop, -classpath, testClasses, classname);
System.out.println(Running command:  + command);

After you fix these up I think this will be ready to go in.

I can sponsor this changeset for you if you need a sponsor.

Thanks,

s'marks




On 12/19/14 1:56 AM, FELIX YANG wrote:

Hi Stuart,
 please review the updated webrev:
http://cr.openjdk.java.net/~xiaofeya/8066085/webrev.01/

Thanks,
-Felix
On 12/12/2014 4:08 PM, Stuart Marks wrote:

On 12/11/14 8:41 PM, FELIX YANG wrote:

Hi all,
 please review the fix to add a sanity checking for rmic -iiop 
compiling.


Bug:
https://bugs.openjdk.java.net/browse/JDK-8066085

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


Hi Felix,

Thanks for picking up the writing of this test. I have several 
comments.


1. Usually we try to avoid naming tests after bug IDs. There are 
such tests in
the suite but most tests have more descriptive names. I'd suggest 
something

like iiopSanityTest.

2. There are other rmic tests in jdk/test/sun/rmi/rmic; the test 
should be

moved there (and the path to the TestLibrary updated correspondingly).

3. The test summary should be a bit more explicit. Instead of a 
sanity test

for rmic -iiop compiling I'd suggest something like Compiles a
PortableRemoteObject with rmic -iiop and ensures that stub and tie 
classes are

generated.

4. When invoking rmic, you need to make sure to pass the -iiop option.

5. Add check for the _Tie class as well as the _Stub class. Not that 
it really
matters, since we know these classes are in the unnamed package, but 
the stub
and tie class files would end up in directories corresponding to 
their package

names -- if they were in a named package.

6. In doTest(), the try/catch of InterruptedException and 
IOException and call
to TestLibrary.bomb() is mostly unnecessary. (Yes, other RMI tests 
do this,
and I've been removing it where appropriate. In the RMI test 
library, the
bomb() call merely wraps and rethrows the exception, and doesn't 
really add
any value. It's like calling fail() in a test-ng test.) In general 
an uncaught
exception will cause a jtreg test to fail, so you might as well just 
declare
the method with throws Exception and let them propagate. This 
helps reduce

clutter.

7. Similarly I'm not sure it's necessary to check the classname 
argument for
null; the failure should be apparent enough if classname is null. 
But if you

think it's worth it, a concise way to null-check an argument is to use
Objects.requireNonNull().

8. The rmiComplie method is misnamed (or typo'd); I'd suggest 
something like

runRmic.

9. Changing the directory of the rmic subprocess is confusing, 
because it
makes different parts of the program deal with different paths to 
the same
files. In addition, switching directories to the test.classes 
directory will
leave the stub/tie classes there, which could possibly confuse 
future test

runs, since that directory isn't necessarily cleared between test runs.

The default working directory of jtreg tests is the scratch 
directory, which
is cleared before each test run. Leaving the current directory as-is 
when
running the subprocesses will write the generated stub/tie classes 
to the

scratch directory, which is the right

Re: RFR 8066085: Need a sanity test for rmic -iiop

[FELIX YANG]

Hi Stuart,
please review the updated webrev:
http://cr.openjdk.java.net/~xiaofeya/8066085/webrev.01/

Thanks,
-Felix
On 12/12/2014 4:08 PM, Stuart Marks wrote:

On 12/11/14 8:41 PM, FELIX YANG wrote:

Hi all,
 please review the fix to add a sanity checking for rmic -iiop 
compiling.


Bug:
https://bugs.openjdk.java.net/browse/JDK-8066085

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


Hi Felix,

Thanks for picking up the writing of this test. I have several comments.

1. Usually we try to avoid naming tests after bug IDs. There are such 
tests in the suite but most tests have more descriptive names. I'd 
suggest something like iiopSanityTest.


2. There are other rmic tests in jdk/test/sun/rmi/rmic; the test 
should be moved there (and the path to the TestLibrary updated 
correspondingly).


3. The test summary should be a bit more explicit. Instead of a 
sanity test for rmic -iiop compiling I'd suggest something like 
Compiles a PortableRemoteObject with rmic -iiop and ensures that stub 
and tie classes are generated.


4. When invoking rmic, you need to make sure to pass the -iiop option.

5. Add check for the _Tie class as well as the _Stub class. Not that 
it really matters, since we know these classes are in the unnamed 
package, but the stub and tie class files would end up in directories 
corresponding to their package names -- if they were in a named package.


6. In doTest(), the try/catch of InterruptedException and IOException 
and call to TestLibrary.bomb() is mostly unnecessary. (Yes, other RMI 
tests do this, and I've been removing it where appropriate. In the RMI 
test library, the bomb() call merely wraps and rethrows the exception, 
and doesn't really add any value. It's like calling fail() in a 
test-ng test.) In general an uncaught exception will cause a jtreg 
test to fail, so you might as well just declare the method with 
throws Exception and let them propagate. This helps reduce clutter.


7. Similarly I'm not sure it's necessary to check the classname 
argument for null; the failure should be apparent enough if classname 
is null. But if you think it's worth it, a concise way to null-check 
an argument is to use Objects.requireNonNull().


8. The rmiComplie method is misnamed (or typo'd); I'd suggest 
something like runRmic.


9. Changing the directory of the rmic subprocess is confusing, because 
it makes different parts of the program deal with different paths to 
the same files. In addition, switching directories to the test.classes 
directory will leave the stub/tie classes there, which could possibly 
confuse future test runs, since that directory isn't necessarily 
cleared between test runs.


The default working directory of jtreg tests is the scratch 
directory, which is cleared before each test run. Leaving the current 
directory as-is when running the subprocesses will write the generated 
stub/tie classes to the scratch directory, which is the right thing.


This implies that you'll need to add the -classpath argument and a 
path to the test.classes directory to the rmic command line.


Thanks,

s'marks