[9] RFR: 8060717: [TESTBUG] Improve test coverage of MethodHandles.explicitCastArguments()
Hello, Please review a test improvement. Covered more cases for MethodHandles.explicitCastArguments(). Bug: https://bugs.openjdk.java.net/browse/JDK-8060717 Webrev: http://cr.openjdk.java.net/~kshefov/8060717/webrev.00/ -Konstantin
Hi Konstantin,
Am 31.07.2015 um 18:37 schrieb Konstantin Shefov <konstantin.shefov@oracle.com>: Please review a test improvement. Covered more cases for MethodHandles.explicitCastArguments().
Bug: https://bugs.openjdk.java.net/browse/JDK-8060717 Webrev: http://cr.openjdk.java.net/~kshefov/8060717/webrev.00/
note that mine is a lower-case review and does not count, but: thumbs up. The level of detail at which the API is tested improves significantly with these changes. Best, Michael -- <http://www.oracle.com/> Dr. Michael Haupt | Principal Member of Technical Staff Phone: +49 331 200 7277 | Fax: +49 331 200 7561 Oracle Java Platform Group | LangTools Team | Nashorn Oracle Deutschland B.V. & Co. KG, Schiffbauergasse 14 | 14467 Potsdam, Germany <http://www.oracle.com/commitment> Oracle is committed to developing practices and products that help protect the environment
Michael, thanks for reviewing! Vladimir, could you take a look, please? -Konstantin On 08/02/2015 05:31 PM, Michael Haupt wrote:
Hi Konstantin,
Am 31.07.2015 um 18:37 schrieb Konstantin Shefov <konstantin.shefov@oracle.com <mailto:konstantin.shefov@oracle.com>>: Please review a test improvement. Covered more cases for MethodHandles.explicitCastArguments().
Bug: https://bugs.openjdk.java.net/browse/JDK-8060717 Webrev: http://cr.openjdk.java.net/~kshefov/8060717/webrev.00/ <http://cr.openjdk.java.net/%7Ekshefov/8060717/webrev.00/>
note that mine is a lower-case review and does not count, but: thumbs up. The level of detail at which the API is tested improves significantly with these changes.
Best,
Michael
--
Oracle <http://www.oracle.com/> Dr. Michael Haupt | Principal Member of Technical Staff Phone: +49 331 200 7277 | Fax: +49 331 200 7561 OracleJava Platform Group | LangTools Team | Nashorn Oracle Deutschland B.V. & Co. KG, Schiffbauergasse 14 | 14467 Potsdam, Germany Green Oracle <http://www.oracle.com/commitment> Oracle is committed to developing practices and products that help protect the environment
Kindly reminder On 08/03/2015 06:06 PM, Konstantin Shefov wrote:
Michael, thanks for reviewing!
Vladimir, could you take a look, please?
-Konstantin
On 08/02/2015 05:31 PM, Michael Haupt wrote:
Hi Konstantin,
Am 31.07.2015 um 18:37 schrieb Konstantin Shefov <konstantin.shefov@oracle.com <mailto:konstantin.shefov@oracle.com>>: Please review a test improvement. Covered more cases for MethodHandles.explicitCastArguments().
Bug: https://bugs.openjdk.java.net/browse/JDK-8060717 Webrev: http://cr.openjdk.java.net/~kshefov/8060717/webrev.00/ <http://cr.openjdk.java.net/%7Ekshefov/8060717/webrev.00/>
note that mine is a lower-case review and does not count, but: thumbs up. The level of detail at which the API is tested improves significantly with these changes.
Best,
Michael
--
Oracle <http://www.oracle.com/> Dr. Michael Haupt | Principal Member of Technical Staff Phone: +49 331 200 7277 | Fax: +49 331 200 7561 OracleJava Platform Group | LangTools Team | Nashorn Oracle Deutschland B.V. & Co. KG, Schiffbauergasse 14 | 14467 Potsdam, Germany Green Oracle <http://www.oracle.com/commitment> Oracle is committed to developing practices and products that help protect the environment
Konstantin, Overall, looks good. Why do you create a new ClassLoader here and not simply reference them directly (they are loaded by application class loader)? + public static void testNonBCPRef2Ref() throws Throwable { + String testClassPath = System.getProperty("test.classes","."); + URL[] classpath = {(new File(testClassPath)).getCanonicalFile() + .toURI().toURL()}; + URLClassLoader ucl = URLClassLoader.newInstance(classpath); + Class testInterface = ucl.loadClass(THIS_CLASS.getSimpleName() + + "$TestInterface"); + Class testSuperClass = ucl.loadClass(THIS_CLASS.getSimpleName() + + "$TestSuperClass"); + Class testSubClass1 = ucl.loadClass(THIS_CLASS.getSimpleName() + + "$TestSubClass1"); I see BCP-to-BCP & non-BCP-to-non-BCP ref-to-ref cases covered. What about BCP-to-non-BCP & non-BCP-to-BCP cases? Best regards, Vladimir Ivanov On 8/3/15 6:06 PM, Konstantin Shefov wrote:
Michael, thanks for reviewing!
Vladimir, could you take a look, please?
-Konstantin
On 08/02/2015 05:31 PM, Michael Haupt wrote:
Hi Konstantin,
Am 31.07.2015 um 18:37 schrieb Konstantin Shefov <konstantin.shefov@oracle.com <mailto:konstantin.shefov@oracle.com>>: Please review a test improvement. Covered more cases for MethodHandles.explicitCastArguments().
Bug: https://bugs.openjdk.java.net/browse/JDK-8060717 Webrev: http://cr.openjdk.java.net/~kshefov/8060717/webrev.00/ <http://cr.openjdk.java.net/%7Ekshefov/8060717/webrev.00/>
note that mine is a lower-case review and does not count, but: thumbs up. The level of detail at which the API is tested improves significantly with these changes.
Best,
Michael
--
Oracle <http://www.oracle.com/> Dr. Michael Haupt | Principal Member of Technical Staff Phone: +49 331 200 7277 | Fax: +49 331 200 7561 OracleJava Platform Group | LangTools Team | Nashorn Oracle Deutschland B.V. & Co. KG, Schiffbauergasse 14 | 14467 Potsdam, Germany Green Oracle <http://www.oracle.com/commitment> Oracle is committed to developing practices and products that help protect the environment
Hi Vladimir Thanks for reviewing On 08/06/2015 01:02 PM, Vladimir Ivanov wrote:
Konstantin,
Overall, looks good.
Why do you create a new ClassLoader here and not simply reference them directly (they are loaded by application class loader)? You a right. Because of class loader hierarchy, all "Test*" classes in this case are loaded by app class loader as an ancestor of url class loader, so it is the same as just reference them directly. To make the "Test*" classes loaded by ucl, they need to be outside of classpath, which will produce extra folders and ".java" files in test workspace.
I think non-bcp to bcp test will play just the same role (work with classes loaded by different class loaders). So I will correct the code. I will add BCP-to-non-BCP & non-BCP-to-BCP cases and remove url classloader. -Konstantin
+ public static void testNonBCPRef2Ref() throws Throwable { + String testClassPath = System.getProperty("test.classes","."); + URL[] classpath = {(new File(testClassPath)).getCanonicalFile() + .toURI().toURL()}; + URLClassLoader ucl = URLClassLoader.newInstance(classpath); + Class testInterface = ucl.loadClass(THIS_CLASS.getSimpleName() + + "$TestInterface"); + Class testSuperClass = ucl.loadClass(THIS_CLASS.getSimpleName() + + "$TestSuperClass"); + Class testSubClass1 = ucl.loadClass(THIS_CLASS.getSimpleName() + + "$TestSubClass1");
I see BCP-to-BCP & non-BCP-to-non-BCP ref-to-ref cases covered. What about BCP-to-non-BCP & non-BCP-to-BCP cases?
Best regards, Vladimir Ivanov
On 8/3/15 6:06 PM, Konstantin Shefov wrote:
Michael, thanks for reviewing!
Vladimir, could you take a look, please?
-Konstantin
On 08/02/2015 05:31 PM, Michael Haupt wrote:
Hi Konstantin,
Am 31.07.2015 um 18:37 schrieb Konstantin Shefov <konstantin.shefov@oracle.com <mailto:konstantin.shefov@oracle.com>>: Please review a test improvement. Covered more cases for MethodHandles.explicitCastArguments().
Bug: https://bugs.openjdk.java.net/browse/JDK-8060717 Webrev: http://cr.openjdk.java.net/~kshefov/8060717/webrev.00/ <http://cr.openjdk.java.net/%7Ekshefov/8060717/webrev.00/>
note that mine is a lower-case review and does not count, but: thumbs up. The level of detail at which the API is tested improves significantly with these changes.
Best,
Michael
--
Oracle <http://www.oracle.com/> Dr. Michael Haupt | Principal Member of Technical Staff Phone: +49 331 200 7277 | Fax: +49 331 200 7561 OracleJava Platform Group | LangTools Team | Nashorn Oracle Deutschland B.V. & Co. KG, Schiffbauergasse 14 | 14467 Potsdam, Germany Green Oracle <http://www.oracle.com/commitment> Oracle is committed to developing practices and products that help protect the environment
Please, look at the modified test http://cr.openjdk.java.net/~kshefov/8060717/webrev.01/ -Konstantin On 08/06/2015 02:06 PM, Konstantin Shefov wrote:
Hi Vladimir
Thanks for reviewing
On 08/06/2015 01:02 PM, Vladimir Ivanov wrote:
Konstantin,
Overall, looks good.
Why do you create a new ClassLoader here and not simply reference them directly (they are loaded by application class loader)? You a right. Because of class loader hierarchy, all "Test*" classes in this case are loaded by app class loader as an ancestor of url class loader, so it is the same as just reference them directly. To make the "Test*" classes loaded by ucl, they need to be outside of classpath, which will produce extra folders and ".java" files in test workspace.
I think non-bcp to bcp test will play just the same role (work with classes loaded by different class loaders).
So I will correct the code. I will add BCP-to-non-BCP & non-BCP-to-BCP cases and remove url classloader.
-Konstantin
+ public static void testNonBCPRef2Ref() throws Throwable { + String testClassPath = System.getProperty("test.classes","."); + URL[] classpath = {(new File(testClassPath)).getCanonicalFile() + .toURI().toURL()}; + URLClassLoader ucl = URLClassLoader.newInstance(classpath); + Class testInterface = ucl.loadClass(THIS_CLASS.getSimpleName() + + "$TestInterface"); + Class testSuperClass = ucl.loadClass(THIS_CLASS.getSimpleName() + + "$TestSuperClass"); + Class testSubClass1 = ucl.loadClass(THIS_CLASS.getSimpleName() + + "$TestSubClass1");
I see BCP-to-BCP & non-BCP-to-non-BCP ref-to-ref cases covered. What about BCP-to-non-BCP & non-BCP-to-BCP cases?
Best regards, Vladimir Ivanov
On 8/3/15 6:06 PM, Konstantin Shefov wrote:
Michael, thanks for reviewing!
Vladimir, could you take a look, please?
-Konstantin
On 08/02/2015 05:31 PM, Michael Haupt wrote:
Hi Konstantin,
Am 31.07.2015 um 18:37 schrieb Konstantin Shefov <konstantin.shefov@oracle.com <mailto:konstantin.shefov@oracle.com>>: Please review a test improvement. Covered more cases for MethodHandles.explicitCastArguments().
Bug: https://bugs.openjdk.java.net/browse/JDK-8060717 Webrev: http://cr.openjdk.java.net/~kshefov/8060717/webrev.00/ <http://cr.openjdk.java.net/%7Ekshefov/8060717/webrev.00/>
note that mine is a lower-case review and does not count, but: thumbs up. The level of detail at which the API is tested improves significantly with these changes.
Best,
Michael
--
Oracle <http://www.oracle.com/> Dr. Michael Haupt | Principal Member of Technical Staff Phone: +49 331 200 7277 | Fax: +49 331 200 7561 OracleJava Platform Group | LangTools Team | Nashorn Oracle Deutschland B.V. & Co. KG, Schiffbauergasse 14 | 14467 Potsdam, Germany Green Oracle <http://www.oracle.com/commitment> Oracle is committed to developing practices and products that help protect the environment
Kindly reminder. 06.08.2015 17:49, Konstantin Shefov пишет:
Please, look at the modified test
http://cr.openjdk.java.net/~kshefov/8060717/webrev.01/
-Konstantin
On 08/06/2015 02:06 PM, Konstantin Shefov wrote:
Hi Vladimir
Thanks for reviewing
On 08/06/2015 01:02 PM, Vladimir Ivanov wrote:
Konstantin,
Overall, looks good.
Why do you create a new ClassLoader here and not simply reference them directly (they are loaded by application class loader)? You a right. Because of class loader hierarchy, all "Test*" classes in this case are loaded by app class loader as an ancestor of url class loader, so it is the same as just reference them directly. To make the "Test*" classes loaded by ucl, they need to be outside of classpath, which will produce extra folders and ".java" files in test workspace.
I think non-bcp to bcp test will play just the same role (work with classes loaded by different class loaders).
So I will correct the code. I will add BCP-to-non-BCP & non-BCP-to-BCP cases and remove url classloader.
-Konstantin
+ public static void testNonBCPRef2Ref() throws Throwable { + String testClassPath = System.getProperty("test.classes","."); + URL[] classpath = {(new File(testClassPath)).getCanonicalFile() + .toURI().toURL()}; + URLClassLoader ucl = URLClassLoader.newInstance(classpath); + Class testInterface = ucl.loadClass(THIS_CLASS.getSimpleName() + + "$TestInterface"); + Class testSuperClass = ucl.loadClass(THIS_CLASS.getSimpleName() + + "$TestSuperClass"); + Class testSubClass1 = ucl.loadClass(THIS_CLASS.getSimpleName() + + "$TestSubClass1");
I see BCP-to-BCP & non-BCP-to-non-BCP ref-to-ref cases covered. What about BCP-to-non-BCP & non-BCP-to-BCP cases?
Best regards, Vladimir Ivanov
On 8/3/15 6:06 PM, Konstantin Shefov wrote:
Michael, thanks for reviewing!
Vladimir, could you take a look, please?
-Konstantin
On 08/02/2015 05:31 PM, Michael Haupt wrote:
Hi Konstantin,
Am 31.07.2015 um 18:37 schrieb Konstantin Shefov <konstantin.shefov@oracle.com <mailto:konstantin.shefov@oracle.com>>: Please review a test improvement. Covered more cases for MethodHandles.explicitCastArguments().
Bug: https://bugs.openjdk.java.net/browse/JDK-8060717 Webrev: http://cr.openjdk.java.net/~kshefov/8060717/webrev.00/ <http://cr.openjdk.java.net/%7Ekshefov/8060717/webrev.00/>
note that mine is a lower-case review and does not count, but: thumbs up. The level of detail at which the API is tested improves significantly with these changes.
Best,
Michael
--
Oracle <http://www.oracle.com/> Dr. Michael Haupt | Principal Member of Technical Staff Phone: +49 331 200 7277 | Fax: +49 331 200 7561 OracleJava Platform Group | LangTools Team | Nashorn Oracle Deutschland B.V. & Co. KG, Schiffbauergasse 14 | 14467 Potsdam, Germany Green Oracle <http://www.oracle.com/commitment> Oracle is committed to developing practices and products that help protect the environment
Looks good. Best regards, Vladimir Ivanov On 8/11/15 3:54 AM, Konstantin Shefov wrote:
Kindly reminder.
06.08.2015 17:49, Konstantin Shefov пишет:
Please, look at the modified test
http://cr.openjdk.java.net/~kshefov/8060717/webrev.01/
-Konstantin
On 08/06/2015 02:06 PM, Konstantin Shefov wrote:
Hi Vladimir
Thanks for reviewing
On 08/06/2015 01:02 PM, Vladimir Ivanov wrote:
Konstantin,
Overall, looks good.
Why do you create a new ClassLoader here and not simply reference them directly (they are loaded by application class loader)? You a right. Because of class loader hierarchy, all "Test*" classes in this case are loaded by app class loader as an ancestor of url class loader, so it is the same as just reference them directly. To make the "Test*" classes loaded by ucl, they need to be outside of classpath, which will produce extra folders and ".java" files in test workspace.
I think non-bcp to bcp test will play just the same role (work with classes loaded by different class loaders).
So I will correct the code. I will add BCP-to-non-BCP & non-BCP-to-BCP cases and remove url classloader.
-Konstantin
+ public static void testNonBCPRef2Ref() throws Throwable { + String testClassPath = System.getProperty("test.classes","."); + URL[] classpath = {(new File(testClassPath)).getCanonicalFile() + .toURI().toURL()}; + URLClassLoader ucl = URLClassLoader.newInstance(classpath); + Class testInterface = ucl.loadClass(THIS_CLASS.getSimpleName() + + "$TestInterface"); + Class testSuperClass = ucl.loadClass(THIS_CLASS.getSimpleName() + + "$TestSuperClass"); + Class testSubClass1 = ucl.loadClass(THIS_CLASS.getSimpleName() + + "$TestSubClass1");
I see BCP-to-BCP & non-BCP-to-non-BCP ref-to-ref cases covered. What about BCP-to-non-BCP & non-BCP-to-BCP cases?
Best regards, Vladimir Ivanov
On 8/3/15 6:06 PM, Konstantin Shefov wrote:
Michael, thanks for reviewing!
Vladimir, could you take a look, please?
-Konstantin
On 08/02/2015 05:31 PM, Michael Haupt wrote:
Hi Konstantin,
> Am 31.07.2015 um 18:37 schrieb Konstantin Shefov > <konstantin.shefov@oracle.com > <mailto:konstantin.shefov@oracle.com>>: > Please review a test improvement. Covered more cases for > MethodHandles.explicitCastArguments(). > > Bug: https://bugs.openjdk.java.net/browse/JDK-8060717 > Webrev: http://cr.openjdk.java.net/~kshefov/8060717/webrev.00/ > <http://cr.openjdk.java.net/%7Ekshefov/8060717/webrev.00/>
note that mine is a lower-case review and does not count, but: thumbs up. The level of detail at which the API is tested improves significantly with these changes.
Best,
Michael
--
Oracle <http://www.oracle.com/> Dr. Michael Haupt | Principal Member of Technical Staff Phone: +49 331 200 7277 | Fax: +49 331 200 7561 OracleJava Platform Group | LangTools Team | Nashorn Oracle Deutschland B.V. & Co. KG, Schiffbauergasse 14 | 14467 Potsdam, Germany Green Oracle <http://www.oracle.com/commitment> Oracle is committed to developing practices and products that help protect the environment
Vladimir, Please review the modified test. Thanks -Konstantin On 08/06/2015 05:49 PM, Konstantin Shefov wrote:
Please, look at the modified test
http://cr.openjdk.java.net/~kshefov/8060717/webrev.01/
-Konstantin
On 08/06/2015 02:06 PM, Konstantin Shefov wrote:
Hi Vladimir
Thanks for reviewing
On 08/06/2015 01:02 PM, Vladimir Ivanov wrote:
Konstantin,
Overall, looks good.
Why do you create a new ClassLoader here and not simply reference them directly (they are loaded by application class loader)? You a right. Because of class loader hierarchy, all "Test*" classes in this case are loaded by app class loader as an ancestor of url class loader, so it is the same as just reference them directly. To make the "Test*" classes loaded by ucl, they need to be outside of classpath, which will produce extra folders and ".java" files in test workspace.
I think non-bcp to bcp test will play just the same role (work with classes loaded by different class loaders).
So I will correct the code. I will add BCP-to-non-BCP & non-BCP-to-BCP cases and remove url classloader.
-Konstantin
+ public static void testNonBCPRef2Ref() throws Throwable { + String testClassPath = System.getProperty("test.classes","."); + URL[] classpath = {(new File(testClassPath)).getCanonicalFile() + .toURI().toURL()}; + URLClassLoader ucl = URLClassLoader.newInstance(classpath); + Class testInterface = ucl.loadClass(THIS_CLASS.getSimpleName() + + "$TestInterface"); + Class testSuperClass = ucl.loadClass(THIS_CLASS.getSimpleName() + + "$TestSuperClass"); + Class testSubClass1 = ucl.loadClass(THIS_CLASS.getSimpleName() + + "$TestSubClass1");
I see BCP-to-BCP & non-BCP-to-non-BCP ref-to-ref cases covered. What about BCP-to-non-BCP & non-BCP-to-BCP cases?
Best regards, Vladimir Ivanov
On 8/3/15 6:06 PM, Konstantin Shefov wrote:
Michael, thanks for reviewing!
Vladimir, could you take a look, please?
-Konstantin
On 08/02/2015 05:31 PM, Michael Haupt wrote:
Hi Konstantin,
Am 31.07.2015 um 18:37 schrieb Konstantin Shefov <konstantin.shefov@oracle.com <mailto:konstantin.shefov@oracle.com>>: Please review a test improvement. Covered more cases for MethodHandles.explicitCastArguments().
Bug: https://bugs.openjdk.java.net/browse/JDK-8060717 Webrev: http://cr.openjdk.java.net/~kshefov/8060717/webrev.00/ <http://cr.openjdk.java.net/%7Ekshefov/8060717/webrev.00/>
note that mine is a lower-case review and does not count, but: thumbs up. The level of detail at which the API is tested improves significantly with these changes.
Best,
Michael
--
Oracle <http://www.oracle.com/> Dr. Michael Haupt | Principal Member of Technical Staff Phone: +49 331 200 7277 | Fax: +49 331 200 7561 OracleJava Platform Group | LangTools Team | Nashorn Oracle Deutschland B.V. & Co. KG, Schiffbauergasse 14 | 14467 Potsdam, Germany Green Oracle <http://www.oracle.com/commitment> Oracle is committed to developing practices and products that help protect the environment
participants (3)
-
Konstantin Shefov
-
Michael Haupt
-
Vladimir Ivanov