Re: [9] RFR 8134487：updated sun/security/ssl/StatusStapling/* to work with modules
On 18/02/2016 02:44, Tim Du wrote: I grep the files which are using CertificateBuilder and SimpleOCSPServer from jake/jdk/test, the result as below: - javax/net/ssl/Stapling/HttpsUrlConnClient.java - javax/net/ssl/Stapling/SSLEngineWithStapling.java - javax/net/ssl/Stapling/SSLSocketWithStapling.java Above three tests are not using the internal and non-public class , They could pass now.No need to update. - sun/security/ssl/StatusStapling/StatusResponseManagerTests.java This one is in ProblemList.jake now, it could be fixed by this code review. So I think everything about CertificateBuilder and SimpleOCSPServer is caught.Thank you. The issue that I'm concerned about is that the Stapling tests are using test infrastructure that is moved by your patch. There are also other tests that seem to depend on this infrastructure. Would it be possible to start with an empty work directory and run the :jdk_security test group? I think that is the easiest way to catch these issues. It may be that the simplest thing is to not move the infrastructure but instead change the test driver to use @library. -Alan
Re: [9] RFR 8134487：updated sun/security/ssl/StatusStapling/* to work with modules
On 2/17/2016 11:40 PM, Alan Bateman wrote: On 17/02/2016 09:57, Tim Du wrote: Sure. I am aslo updated java.base/sun/security/testlibrary to test/java/security/testlibrary/ , so that keep the testlibrary files could be reused by other tests. Re-generate the latest webrev is http://cr.openjdk.java.net/~tidu/8134487/webrev.03/ , Thanks. Are there other tests that uses this infrastructure that need to be updated too? I'm seeing test failures with other Stapling tests and it looks like they depend on CertificateBuilder and SimplerOSCPServer. Would it be possible to run the jdk_security tests group to make sure that everything is caught? I grep the files which are using CertificateBuilder and SimpleOCSPServer from jake/jdk/test, the result as below: - javax/net/ssl/Stapling/HttpsUrlConnClient.java - javax/net/ssl/Stapling/SSLEngineWithStapling.java - javax/net/ssl/Stapling/SSLSocketWithStapling.java Above three tests are not using the internal and non-public class , They could pass now.No need to update. - sun/security/ssl/StatusStapling/StatusResponseManagerTests.java This one is in ProblemList.jake now, it could be fixed by this code review. So I think everything about CertificateBuilder and SimpleOCSPServer is caught.Thank you. Regards Tim -Alan
Re: [9] RFR 8134487：updated sun/security/ssl/StatusStapling/* to work with modules
On 17/02/2016 09:57, Tim Du wrote: Sure. I am aslo updated java.base/sun/security/testlibrary to test/java/security/testlibrary/ , so that keep the testlibrary files could be reused by other tests. Re-generate the latest webrev is http://cr.openjdk.java.net/~tidu/8134487/webrev.03/ , Thanks. Are there other tests that uses this infrastructure that need to be updated too? I'm seeing test failures with other Stapling tests and it looks like they depend on CertificateBuilder and SimplerOSCPServer. Would it be possible to run the jdk_security tests group to make sure that everything is caught? -Alan
Re: [9] RFR 8134487：updated sun/security/ssl/StatusStapling/* to work with modules
On 17/02/2016 09:57, Tim Du wrote: Sure. I am aslo updated java.base/sun/security/testlibrary to test/java/security/testlibrary/ , so that keep the testlibrary files could be reused by other tests. Re-generate the latest webrev is http://cr.openjdk.java.net/~tidu/8134487/webrev.03/ , Thanks. Thanks, that makes it easier to see what the changed. I think this looks okay now, it just means there is one test driver class. I will remove the tests from the exclude list before pushing this. -Alan
Re: [9] RFR 8134487：updated sun/security/ssl/StatusStapling/* to work with modules
On 2/17/2016 4:23 PM, Alan Bateman wrote: On 17/02/2016 08:19, Tim Du wrote: Follow you suggestion, I updated the code as below: 1.Create java.base/sun/security/ssl under test/sun/security/ssl/StatusStapling/, move all test files and dependency class into java.base/sun/security/ssl ,removed the jtreg label from test java files. 2.Create java.base/sun/security/testlibrary , add CertficateBuilder.java and SimpleOCSPServer.java into this folder, so that StatusResponseManagerTests.java could access them by java.base . 3.Add TestRun.java to load all test. The new webrev is http://cr.openjdk.java.net/~tidu/8134487/webrev.02/ , please help to have a look,Thank you. Good, looks like you have it figured out now. Can you use "hg mv" to move the files and re-generate the weberv? With webrev.02 then it's impossible to see what has changed. Sure. I am aslo updated java.base/sun/security/testlibrary to test/java/security/testlibrary/ , so that keep the testlibrary files could be reused by other tests. Re-generate the latest webrev is http://cr.openjdk.java.net/~tidu/8134487/webrev.03/ , Thanks. Regards Tim -Alan
Re: [9] RFR 8134487：updated sun/security/ssl/StatusStapling/* to work with modules
On 17/02/2016 08:19, Tim Du wrote: Follow you suggestion, I updated the code as below: 1.Create java.base/sun/security/ssl under test/sun/security/ssl/StatusStapling/, move all test files and dependency class into java.base/sun/security/ssl ,removed the jtreg label from test java files. 2.Create java.base/sun/security/testlibrary , add CertficateBuilder.java and SimpleOCSPServer.java into this folder, so that StatusResponseManagerTests.java could access them by java.base . 3.Add TestRun.java to load all test. The new webrev is http://cr.openjdk.java.net/~tidu/8134487/webrev.02/ , please help to have a look,Thank you. Good, looks like you have it figured out now. Can you use "hg mv" to move the files and re-generate the weberv? With webrev.02 then it's impossible to see what has changed. -Alan
Re: [9] RFR 8134487：updated sun/security/ssl/StatusStapling/* to work with modules
Hi Alan: On 2/16/2016 7:25 PM, Alan Bateman wrote: On 16/02/2016 11:05, Tim Du wrote: Hi Alan： I tried @compile/module to fix this issue before, but it does not work ,Use CertStatusReqExtensionTests.java as example, the process as below: 1. Create java.base/sun/security/ssl folder in test/sun/security/ssl/StatusStapling. 2. Move the dependency class like TestCase.java TestUtils.java BogusStatusRequest.java into test/sun/security/ssl/StatusStapling/java.base/sun/security/ssl 3. Move CertStatusReqExtensionTests.java into test/sun/security/ssl/StatusStapling/java.base/sun/security/ssl/ , as it's in package sun.security.ssl and access non-public class. 4. Add @compile/module in CertStatusReqExtensionTests.java as below: ### @test @bug 8046321 @compile/module=java.base sun/security/ssl/TestCase.java @compile/module=java.base sun/security/ssl/TestUtils.java @compile/module=java.base sun/security/ssl/BogusStatusRequest.java @compile/module=java.base sun/security/ssl/CertStatusReqExtensionTests.java @run main/othervm CertStatusReqExtensionTests # Bug got this error message: execStatus=Error. Parse Exceptioin\: Can't find source file\: sun/security/ssl/TestCase.java Build and run CertStatusReqExtensionTest.java by a wrapper could work , and I saw some existing tests were written in this way.So I think this is a valid solution.Thanks. Can you try this: /* * @test * @modules java.base/sun.security.ssl * @build java.base/sun.security.ssl.CertStatusReqExtensionTests * @run main/othervm java.base/sun.security.ssl.CertStatusReqExtensionTests */ The @modules is so that jtreg can invoke un.security.ssl.CertStatusReqExtensionTests. Follow you suggestion, I updated the code as below: 1.Create java.base/sun/security/ssl under test/sun/security/ssl/StatusStapling/, move all test files and dependency class into java.base/sun/security/ssl ,removed the jtreg label from test java files. 2.Create java.base/sun/security/testlibrary , add CertficateBuilder.java and SimpleOCSPServer.java into this folder, so that StatusResponseManagerTests.java could access them by java.base . 3.Add TestRun.java to load all test. The new webrev is http://cr.openjdk.java.net/~tidu/8134487/webrev.02/ , please help to have a look,Thank you. Regards Tim -Alan.
Re: [9] RFR 8134487：updated sun/security/ssl/StatusStapling/* to work with modules
On 16/02/2016 11:05, Tim Du wrote: Hi Alan： I tried @compile/module to fix this issue before, but it does not work ,Use CertStatusReqExtensionTests.java as example, the process as below: 1. Create java.base/sun/security/ssl folder in test/sun/security/ssl/StatusStapling. 2. Move the dependency class like TestCase.java TestUtils.java BogusStatusRequest.java into test/sun/security/ssl/StatusStapling/java.base/sun/security/ssl 3. Move CertStatusReqExtensionTests.java into test/sun/security/ssl/StatusStapling/java.base/sun/security/ssl/ , as it's in package sun.security.ssl and access non-public class. 4. Add @compile/module in CertStatusReqExtensionTests.java as below: ### @test @bug 8046321 @compile/module=java.base sun/security/ssl/TestCase.java @compile/module=java.base sun/security/ssl/TestUtils.java @compile/module=java.base sun/security/ssl/BogusStatusRequest.java @compile/module=java.base sun/security/ssl/CertStatusReqExtensionTests.java @run main/othervm CertStatusReqExtensionTests # Bug got this error message: execStatus=Error. Parse Exceptioin\: Can't find source file\: sun/security/ssl/TestCase.java Build and run CertStatusReqExtensionTest.java by a wrapper could work , and I saw some existing tests were written in this way.So I think this is a valid solution.Thanks. Can you try this: /* * @test * @modules java.base/sun.security.ssl * @build java.base/sun.security.ssl.CertStatusReqExtensionTests * @run main/othervm java.base/sun.security.ssl.CertStatusReqExtensionTests */ The @modules is so that jtreg can invoke un.security.ssl.CertStatusReqExtensionTests. -Alan.
Re: [9] RFR 8134487：updated sun/security/ssl/StatusStapling/* to work with modules
Hi Alan： I tried @compile/module to fix this issue before, but it does not work ,Use CertStatusReqExtensionTests.java as example, the process as below: 1. Create java.base/sun/security/ssl folder in test/sun/security/ssl/StatusStapling. 2. Move the dependency class like TestCase.java TestUtils.java BogusStatusRequest.java into test/sun/security/ssl/StatusStapling/java.base/sun/security/ssl 3. Move CertStatusReqExtensionTests.java into test/sun/security/ssl/StatusStapling/java.base/sun/security/ssl/ , as it's in package sun.security.ssl and access non-public class. 4. Add @compile/module in CertStatusReqExtensionTests.java as below: ### @test @bug 8046321 @compile/module=java.base sun/security/ssl/TestCase.java @compile/module=java.base sun/security/ssl/TestUtils.java @compile/module=java.base sun/security/ssl/BogusStatusRequest.java @compile/module=java.base sun/security/ssl/CertStatusReqExtensionTests.java @run main/othervm CertStatusReqExtensionTests # Bug got this error message: execStatus=Error. Parse Exceptioin\: Can't find source file\: sun/security/ssl/TestCase.java Build and run CertStatusReqExtensionTest.java by a wrapper could work , and I saw some existing tests were written in this way.So I think this is a valid solution.Thanks. Regards Tim On 2/16/2016 3:47 PM, Alan Bateman wrote: On 16/02/2016 03:31, Tim Du wrote: Hi All: Please help to review the fix for Jigsaw test bug.Thanks. Bug: https://bugs.openjdk.java.net/browse/JDK-8134487 Webrev: http://cr.openjdk.java.net/~tidu/8134487/webrev.01/ The test to access non-public class.To workaround the issue , I create the test wrapper to compile test class into the named module with -Xmodule ,-Xpath options and then execute the test with -Xpath ,-m options. It's not clear to me why these tests need wrappers. I see in the bug that you've tried @compile/module=java.base but you run into problems running the tests. Can you say more about this? -Alan
Re: [9] RFR 8134487：updated sun/security/ssl/StatusStapling/* to work with modules
On 16/02/2016 03:31, Tim Du wrote: Hi All: Please help to review the fix for Jigsaw test bug.Thanks. Bug: https://bugs.openjdk.java.net/browse/JDK-8134487 Webrev: http://cr.openjdk.java.net/~tidu/8134487/webrev.01/ The test to access non-public class.To workaround the issue , I create the test wrapper to compile test class into the named module with -Xmodule ,-Xpath options and then execute the test with -Xpath ,-m options. It's not clear to me why these tests need wrappers. I see in the bug that you've tried @compile/module=java.base but you run into problems running the tests. Can you say more about this? -Alan
