Re: Code Review Request 6203816 and 6720456
For the TBD tests, we sort of just let them run, i.e. through the getInstance(), init(..), generateKey(), and don't enforce the actual result to match the expected result. Hm...that's unfortunate. I guess I don't see a better way to do it either. http://cr.openjdk.java.net/~valeriep/6720456/webrev.00/test/sun/security/pkcs11/KeyGenerator/TestKeyGenerator.java.frames.html Line 107: Minor nit: 80 chars. Not as big a deal as the src directory. Brad On 11/19/2010 4:47 PM, Valerie (Yu-Ching) Peng wrote: Great, thanks for the confirmation... Valerie On 11/19/10 16:45, Weijun Wang wrote: I'm fine with all code changes. Thanks Max On 11/20/2010 08:00 AM, Valerie (Yu-Ching) Peng wrote: On 11/17/10 19:31, Weijun Wang wrote: On 11/18/2010 11:00 AM, Valerie (Yu-Ching) Peng wrote: Thanks for the lightning fast review! TBD means to be determined at runtime. Different machines w/ different versions of Solaris may support different key sizes. So, I use TBD to indicate the key sizes which may only be supported by the newer versions of Solairs release. I see. So for a test failure (i.e. expected != actual) on those TBD tests, there is no way to find out whether it's because the key size is not supported or some other real keypair generation error? For the TBD tests, we sort of just let them run, i.e. through the getInstance(), init(..), generateKey(), and don't enforce the actual result to match the expected result. Our current java API doesn't have the capability to query the provider supported key ranges, so there isn't an easy way to find out at runtime. Enhancing the test framework for this is possible but I doubt we'll get to it given our resource constraints. So, I just handle it this way using the TBD value. If you have some more comments, let me know. Otherwise, I'll be putting them back shortly...and I'll file another CR to address what other concerns that you have... Thanks! Valerie I thought you are on vacation? If not, there are some new PKCS11 test failures which seems related to your resource string changes. I'll take a shot at them if you are on vacation... Yes I am on vacation, but staying at home. I've written a mail to Lana on the PKCS11 test failures. It seems they haven't used the new sunpkcs11.jar, maybe their jdk/make/closed is still not updated. Thanks Max Valerie On 11/17/10 17:00, Weijun Wang wrote: On 11/18/2010 07:31 AM, Valerie (Yu-Ching) Peng wrote: Hi, Max, Can you please help reviewing the following two regression test fixes? 6203816: Can not run test/java/security/Security/ClassLoaderDeadlock.sh from the command line Webrev: http://cr.openjdk.java.net/~valeriep/6203816/webrev.00/ Fix looks fine. I noticed that Deadlock2.sh would fail when the TESTJAVA env variable is set to a JDK instead of a JRE. So, I fixed it here as well. 6720456: New 4150 may have larger blowfish keysizes Webrev: http://cr.openjdk.java.net/~valeriep/6720456/webrev.00/ Haven't looked into the base class PKCS11Test yet, so TBD means you don't care if it succeed or fails? I guess if a bitsize is not supported, the exception should be different and you can detect it and mark PASS? Thanks Max Thanks, Valerie
Re: Code Review Request 6203816 and 6720456
On 11/17/10 19:31, Weijun Wang wrote: On 11/18/2010 11:00 AM, Valerie (Yu-Ching) Peng wrote: Thanks for the lightning fast review! TBD means to be determined at runtime. Different machines w/ different versions of Solaris may support different key sizes. So, I use TBD to indicate the key sizes which may only be supported by the newer versions of Solairs release. I see. So for a test failure (i.e. expected != actual) on those TBD tests, there is no way to find out whether it's because the key size is not supported or some other real keypair generation error? For the TBD tests, we sort of just let them run, i.e. through the getInstance(), init(..), generateKey(), and don't enforce the actual result to match the expected result. Our current java API doesn't have the capability to query the provider supported key ranges, so there isn't an easy way to find out at runtime. Enhancing the test framework for this is possible but I doubt we'll get to it given our resource constraints. So, I just handle it this way using the TBD value. If you have some more comments, let me know. Otherwise, I'll be putting them back shortly...and I'll file another CR to address what other concerns that you have... Thanks! Valerie I thought you are on vacation? If not, there are some new PKCS11 test failures which seems related to your resource string changes. I'll take a shot at them if you are on vacation... Yes I am on vacation, but staying at home. I've written a mail to Lana on the PKCS11 test failures. It seems they haven't used the new sunpkcs11.jar, maybe their jdk/make/closed is still not updated. Thanks Max Valerie On 11/17/10 17:00, Weijun Wang wrote: On 11/18/2010 07:31 AM, Valerie (Yu-Ching) Peng wrote: Hi, Max, Can you please help reviewing the following two regression test fixes? 6203816: Can not run test/java/security/Security/ClassLoaderDeadlock.sh from the command line Webrev: http://cr.openjdk.java.net/~valeriep/6203816/webrev.00/ Fix looks fine. I noticed that Deadlock2.sh would fail when the TESTJAVA env variable is set to a JDK instead of a JRE. So, I fixed it here as well. 6720456: New 4150 may have larger blowfish keysizes Webrev: http://cr.openjdk.java.net/~valeriep/6720456/webrev.00/ Haven't looked into the base class PKCS11Test yet, so TBD means you don't care if it succeed or fails? I guess if a bitsize is not supported, the exception should be different and you can detect it and mark PASS? Thanks Max Thanks, Valerie
Re: Code Review Request 6203816 and 6720456
Great, thanks for the confirmation... Valerie On 11/19/10 16:45, Weijun Wang wrote: I'm fine with all code changes. Thanks Max On 11/20/2010 08:00 AM, Valerie (Yu-Ching) Peng wrote: On 11/17/10 19:31, Weijun Wang wrote: On 11/18/2010 11:00 AM, Valerie (Yu-Ching) Peng wrote: Thanks for the lightning fast review! TBD means to be determined at runtime. Different machines w/ different versions of Solaris may support different key sizes. So, I use TBD to indicate the key sizes which may only be supported by the newer versions of Solairs release. I see. So for a test failure (i.e. expected != actual) on those TBD tests, there is no way to find out whether it's because the key size is not supported or some other real keypair generation error? For the TBD tests, we sort of just let them run, i.e. through the getInstance(), init(..), generateKey(), and don't enforce the actual result to match the expected result. Our current java API doesn't have the capability to query the provider supported key ranges, so there isn't an easy way to find out at runtime. Enhancing the test framework for this is possible but I doubt we'll get to it given our resource constraints. So, I just handle it this way using the TBD value. If you have some more comments, let me know. Otherwise, I'll be putting them back shortly...and I'll file another CR to address what other concerns that you have... Thanks! Valerie I thought you are on vacation? If not, there are some new PKCS11 test failures which seems related to your resource string changes. I'll take a shot at them if you are on vacation... Yes I am on vacation, but staying at home. I've written a mail to Lana on the PKCS11 test failures. It seems they haven't used the new sunpkcs11.jar, maybe their jdk/make/closed is still not updated. Thanks Max Valerie On 11/17/10 17:00, Weijun Wang wrote: On 11/18/2010 07:31 AM, Valerie (Yu-Ching) Peng wrote: Hi, Max, Can you please help reviewing the following two regression test fixes? 6203816: Can not run test/java/security/Security/ClassLoaderDeadlock.sh from the command line Webrev: http://cr.openjdk.java.net/~valeriep/6203816/webrev.00/ Fix looks fine. I noticed that Deadlock2.sh would fail when the TESTJAVA env variable is set to a JDK instead of a JRE. So, I fixed it here as well. 6720456: New 4150 may have larger blowfish keysizes Webrev: http://cr.openjdk.java.net/~valeriep/6720456/webrev.00/ Haven't looked into the base class PKCS11Test yet, so TBD means you don't care if it succeed or fails? I guess if a bitsize is not supported, the exception should be different and you can detect it and mark PASS? Thanks Max Thanks, Valerie
Re: Code Review Request 6203816 and 6720456
I'm fine with all code changes. Thanks Max On 11/20/2010 08:00 AM, Valerie (Yu-Ching) Peng wrote: On 11/17/10 19:31, Weijun Wang wrote: On 11/18/2010 11:00 AM, Valerie (Yu-Ching) Peng wrote: Thanks for the lightning fast review! TBD means to be determined at runtime. Different machines w/ different versions of Solaris may support different key sizes. So, I use TBD to indicate the key sizes which may only be supported by the newer versions of Solairs release. I see. So for a test failure (i.e. expected != actual) on those TBD tests, there is no way to find out whether it's because the key size is not supported or some other real keypair generation error? For the TBD tests, we sort of just let them run, i.e. through the getInstance(), init(..), generateKey(), and don't enforce the actual result to match the expected result. Our current java API doesn't have the capability to query the provider supported key ranges, so there isn't an easy way to find out at runtime. Enhancing the test framework for this is possible but I doubt we'll get to it given our resource constraints. So, I just handle it this way using the TBD value. If you have some more comments, let me know. Otherwise, I'll be putting them back shortly...and I'll file another CR to address what other concerns that you have... Thanks! Valerie I thought you are on vacation? If not, there are some new PKCS11 test failures which seems related to your resource string changes. I'll take a shot at them if you are on vacation... Yes I am on vacation, but staying at home. I've written a mail to Lana on the PKCS11 test failures. It seems they haven't used the new sunpkcs11.jar, maybe their jdk/make/closed is still not updated. Thanks Max Valerie On 11/17/10 17:00, Weijun Wang wrote: On 11/18/2010 07:31 AM, Valerie (Yu-Ching) Peng wrote: Hi, Max, Can you please help reviewing the following two regression test fixes? 6203816: Can not run test/java/security/Security/ClassLoaderDeadlock.sh from the command line Webrev: http://cr.openjdk.java.net/~valeriep/6203816/webrev.00/ Fix looks fine. I noticed that Deadlock2.sh would fail when the TESTJAVA env variable is set to a JDK instead of a JRE. So, I fixed it here as well. 6720456: New 4150 may have larger blowfish keysizes Webrev: http://cr.openjdk.java.net/~valeriep/6720456/webrev.00/ Haven't looked into the base class PKCS11Test yet, so TBD means you don't care if it succeed or fails? I guess if a bitsize is not supported, the exception should be different and you can detect it and mark PASS? Thanks Max Thanks, Valerie
Re: Code Review Request 6203816 and 6720456
On 11/18/2010 11:00 AM, Valerie (Yu-Ching) Peng wrote: Thanks for the lightning fast review! TBD means to be determined at runtime. Different machines w/ different versions of Solaris may support different key sizes. So, I use TBD to indicate the key sizes which may only be supported by the newer versions of Solairs release. I see. So for a test failure (i.e. expected != actual) on those TBD tests, there is no way to find out whether it's because the key size is not supported or some other real keypair generation error? I thought you are on vacation? If not, there are some new PKCS11 test failures which seems related to your resource string changes. I'll take a shot at them if you are on vacation... Yes I am on vacation, but staying at home. I've written a mail to Lana on the PKCS11 test failures. It seems they haven't used the new sunpkcs11.jar, maybe their jdk/make/closed is still not updated. Thanks Max Valerie On 11/17/10 17:00, Weijun Wang wrote: On 11/18/2010 07:31 AM, Valerie (Yu-Ching) Peng wrote: Hi, Max, Can you please help reviewing the following two regression test fixes? 6203816: Can not run test/java/security/Security/ClassLoaderDeadlock.sh from the command line Webrev: http://cr.openjdk.java.net/~valeriep/6203816/webrev.00/ Fix looks fine. I noticed that Deadlock2.sh would fail when the TESTJAVA env variable is set to a JDK instead of a JRE. So, I fixed it here as well. 6720456: New 4150 may have larger blowfish keysizes Webrev: http://cr.openjdk.java.net/~valeriep/6720456/webrev.00/ Haven't looked into the base class PKCS11Test yet, so TBD means you don't care if it succeed or fails? I guess if a bitsize is not supported, the exception should be different and you can detect it and mark PASS? Thanks Max Thanks, Valerie
Code Review Request 6203816 and 6720456
Hi, Max, Can you please help reviewing the following two regression test fixes? 6203816: Can not run test/java/security/Security/ClassLoaderDeadlock.sh from the command line Webrev: http://cr.openjdk.java.net/~valeriep/6203816/webrev.00/ I noticed that Deadlock2.sh would fail when the TESTJAVA env variable is set to a JDK instead of a JRE. So, I fixed it here as well. 6720456: New 4150 may have larger blowfish keysizes Webrev: http://cr.openjdk.java.net/~valeriep/6720456/webrev.00/ Thanks, Valerie
Re: Code Review Request 6203816 and 6720456
On 11/18/2010 07:31 AM, Valerie (Yu-Ching) Peng wrote: Hi, Max, Can you please help reviewing the following two regression test fixes? 6203816: Can not run test/java/security/Security/ClassLoaderDeadlock.sh from the command line Webrev: http://cr.openjdk.java.net/~valeriep/6203816/webrev.00/ Fix looks fine. I noticed that Deadlock2.sh would fail when the TESTJAVA env variable is set to a JDK instead of a JRE. So, I fixed it here as well. 6720456: New 4150 may have larger blowfish keysizes Webrev: http://cr.openjdk.java.net/~valeriep/6720456/webrev.00/ Haven't looked into the base class PKCS11Test yet, so TBD means you don't care if it succeed or fails? I guess if a bitsize is not supported, the exception should be different and you can detect it and mark PASS? Thanks Max Thanks, Valerie
Re: Code Review Request 6203816 and 6720456
Thanks for the lightning fast review! TBD means to be determined at runtime. Different machines w/ different versions of Solaris may support different key sizes. So, I use TBD to indicate the key sizes which may only be supported by the newer versions of Solairs release. I thought you are on vacation? If not, there are some new PKCS11 test failures which seems related to your resource string changes. I'll take a shot at them if you are on vacation... Valerie On 11/17/10 17:00, Weijun Wang wrote: On 11/18/2010 07:31 AM, Valerie (Yu-Ching) Peng wrote: Hi, Max, Can you please help reviewing the following two regression test fixes? 6203816: Can not run test/java/security/Security/ClassLoaderDeadlock.sh from the command line Webrev: http://cr.openjdk.java.net/~valeriep/6203816/webrev.00/ Fix looks fine. I noticed that Deadlock2.sh would fail when the TESTJAVA env variable is set to a JDK instead of a JRE. So, I fixed it here as well. 6720456: New 4150 may have larger blowfish keysizes Webrev: http://cr.openjdk.java.net/~valeriep/6720456/webrev.00/ Haven't looked into the base class PKCS11Test yet, so TBD means you don't care if it succeed or fails? I guess if a bitsize is not supported, the exception should be different and you can detect it and mark PASS? Thanks Max Thanks, Valerie
