Greetings,
I think that the regression test "gnu/testlet/java/lang/Integer/parseInt.java" should be fixed to work correctly for pre JDK1.7 case (JDK1.0 .. JDK1.6) and also for JDK1.7 case. The difference between JDK1.6 and JDK1.7 is in the behaviour of method Integer.parseInt() when string beginning with '+' sign is parsed: Integer.parseInt("+42") JDK1.6 throws exception in this case, but parsing the same string is perfectly valid in JDK1.7. So I added the condition to the test which checks what JDK is being tested (I can't figure out how Mauve harness checks the "Tags: JDKxx" tag). I also changed the message printed in case JDK1.7 does not work correctly. Is it possible to push the fixed test to Mauve repository please? Thank you in advance, Pavel --- gnu/testlet/java/lang/Integer/parseInt.java 2008-05-12 23:35:49.000000000 +0200 +++ gnu/testlet/java/lang/Integer/parseInt.java 2011-08-17 17:17:16.000000000 +0200 @@ -106,15 +106,31 @@ } // In JDK1.7, '+' is considered a valid character. - try - { - i = Integer.parseInt("+10"); - harness.check(true); - harness.check(i, 10); - } - catch (NumberFormatException nfe) - { - harness.fail("Leading '+' does not throw NumberFormatException"); + // it means that the following step should be divided + // for pre JDK1.7 case and >= JDK1.7 + String[] javaVersion = System.getProperty("java.version").split("\\."); + if (Integer.parseInt(javaVersion[1]) >= 7) { + try + { + i = Integer.parseInt("+10"); + harness.check(true); + harness.check(i, 10); + } + catch (NumberFormatException nfe) + { + harness.fail("'+10' string is not parsed correctly as expected in JDK1.7"); + } + } + else { // pre JDK1.7 branch + try + { + i = Integer.parseInt("+10"); + harness.fail("'+10' must throw NumberFormatException"); + } + catch (NumberFormatException nfe) + { + harness.check(true); + } } try |
On 18:20 Wed 17 Aug , Pavel Tisnovsky wrote:
> Greetings, > > I think that the regression test "gnu/testlet/java/lang/Integer/parseInt.java" > should be fixed to work correctly for pre JDK1.7 case (JDK1.0 .. JDK1.6) and > also for JDK1.7 case. The difference between JDK1.6 and JDK1.7 is in the > behaviour of method Integer.parseInt() when string beginning with '+' sign is > parsed: > > Integer.parseInt("+42") > > JDK1.6 throws exception in this case, but parsing the same string is perfectly > valid in JDK1.7. So I added the condition to the test which checks what JDK is > being tested (I can't figure out how Mauve harness checks the "Tags: JDKxx" > tag). I also changed the message printed in case JDK1.7 does not work correctly. > > Is it possible to push the fixed test to Mauve repository please? > > Thank you in advance, > Pavel Such tests are dubious because they assume compliance to one particular version, which has never been true of GNU Classpath and can't be verified due to the lack of a Free Software TCK. There will also be versions of OpenJDK7 that report as "1.7" but don't yet have this change. Have you tested this with GNU Classpath? I'm pretty sure it will break the test there as it has the modern behaviour, but most GNU Classpath VMs tend to report earlier versions due to the level of bytecode they support. As the old behaviour is specific to versions of Sun/Oracle's JDK implementation, it should check that this implementation is being used as well, rather than just the version. > --- gnu/testlet/java/lang/Integer/parseInt.java 2008-05-12 23:35:49.000000000 +0200 > +++ gnu/testlet/java/lang/Integer/parseInt.java 2011-08-17 17:17:16.000000000 +0200 > @@ -106,15 +106,31 @@ > } > > // In JDK1.7, '+' is considered a valid character. > - try > - { > - i = Integer.parseInt("+10"); > - harness.check(true); > - harness.check(i, 10); > - } > - catch (NumberFormatException nfe) > - { > - harness.fail("Leading '+' does not throw NumberFormatException"); > + // it means that the following step should be divided > + // for pre JDK1.7 case and >= JDK1.7 > + String[] javaVersion = System.getProperty("java.version").split("\\."); > + if (Integer.parseInt(javaVersion[1]) >= 7) { > + try > + { > + i = Integer.parseInt("+10"); > + harness.check(true); > + harness.check(i, 10); > + } > + catch (NumberFormatException nfe) > + { > + harness.fail("'+10' string is not parsed correctly as expected in JDK1.7"); > + } > + } > + else { // pre JDK1.7 branch > + try > + { > + i = Integer.parseInt("+10"); > + harness.fail("'+10' must throw NumberFormatException"); > + } > + catch (NumberFormatException nfe) > + { > + harness.check(true); > + } > } > > try -- Andrew :) Free Java Software Engineer Red Hat, Inc. (http://www.redhat.com) Support Free Java! Contribute to GNU Classpath and IcedTea http://www.gnu.org/software/classpath http://icedtea.classpath.org PGP Key: F5862A37 (https://keys.indymedia.org/) Fingerprint = EA30 D855 D50F 90CD F54D 0698 0713 C3ED F586 2A37 |
Dr Andrew John Hughes wrote:
> On 18:20 Wed 17 Aug , Pavel Tisnovsky wrote: >> Greetings, >> >> I think that the regression test "gnu/testlet/java/lang/Integer/parseInt.java" >> should be fixed to work correctly for pre JDK1.7 case (JDK1.0 .. JDK1.6) and >> also for JDK1.7 case. The difference between JDK1.6 and JDK1.7 is in the >> behaviour of method Integer.parseInt() when string beginning with '+' sign is >> parsed: >> >> Integer.parseInt("+42") >> >> JDK1.6 throws exception in this case, but parsing the same string is perfectly >> valid in JDK1.7. So I added the condition to the test which checks what JDK is >> being tested (I can't figure out how Mauve harness checks the "Tags: JDKxx" >> tag). I also changed the message printed in case JDK1.7 does not work correctly. >> >> Is it possible to push the fixed test to Mauve repository please? >> >> Thank you in advance, >> Pavel > > Such tests are dubious because they assume compliance to one particular version, > which has never been true of GNU Classpath and can't be verified due to the lack > of a Free Software TCK. There will also be versions of OpenJDK7 that report > as "1.7" but don't yet have this change. Hi Andrew, you are right that we have not FS TCK so the precise JRE behaviour is questionable, but I think that any JDK/JRE which identifies itself as "1.6" should conforms to "Java Platform, Standard Edition 6 API Specification", at least from common developers point of view. > Have you tested this with GNU Classpath? I'm pretty sure it will break the > test there as it has the modern behaviour, but most GNU Classpath VMs tend > to report earlier versions due to the level of bytecode they support. Yeah, that's true because I've tried it using gij: $ gij -version java version "1.5.0" gij (GNU libgcj) version 4.6.0 20110603 (Red Hat 4.6.0-10) Its interesting because it is identified itself as "1.5.0" and parse the string "+10" without problems. > As the old behaviour is specific to versions of Sun/Oracle's JDK implementation, > it should check that this implementation is being used as well, rather than just > the version. Ok, I'll look into it, thank you for review! > >> --- gnu/testlet/java/lang/Integer/parseInt.java 2008-05-12 23:35:49.000000000 +0200 >> +++ gnu/testlet/java/lang/Integer/parseInt.java 2011-08-17 17:17:16.000000000 +0200 >> @@ -106,15 +106,31 @@ >> } >> >> // In JDK1.7, '+' is considered a valid character. >> - try >> - { >> - i = Integer.parseInt("+10"); >> - harness.check(true); >> - harness.check(i, 10); >> - } >> - catch (NumberFormatException nfe) >> - { >> - harness.fail("Leading '+' does not throw NumberFormatException"); >> + // it means that the following step should be divided >> + // for pre JDK1.7 case and >= JDK1.7 >> + String[] javaVersion = System.getProperty("java.version").split("\\."); >> + if (Integer.parseInt(javaVersion[1]) >= 7) { >> + try >> + { >> + i = Integer.parseInt("+10"); >> + harness.check(true); >> + harness.check(i, 10); >> + } >> + catch (NumberFormatException nfe) >> + { >> + harness.fail("'+10' string is not parsed correctly as expected in JDK1.7"); >> + } >> + } >> + else { // pre JDK1.7 branch >> + try >> + { >> + i = Integer.parseInt("+10"); >> + harness.fail("'+10' must throw NumberFormatException"); >> + } >> + catch (NumberFormatException nfe) >> + { >> + harness.check(true); >> + } >> } >> >> try > > |
In reply to this post by ahughes-2
Dr Andrew John Hughes wrote:
> As the old behaviour is specific to versions of Sun/Oracle's JDK implementation, > it should check that this implementation is being used as well, rather than just > the version. Hi Andrew, I've tried to add check for JDK/JRE implementation as you suggested. Patch for parseInt Mauve test is stored in an attachment. Cheers, Pavel --- gnu/testlet/java/lang/Integer/parseInt.java 2008-05-12 23:35:49.000000000 +0200 +++ gnu/testlet/java/lang/Integer/parseInt.java 2011-08-17 17:17:16.000000000 +0200 @@ -27,6 +27,26 @@ public class parseInt implements Testlet { + /** + * Returns true if tested JRE conformns to JDK 1.7 specification. + */ + private static boolean conformToJDK17() + { + String[] javaVersion = System.getProperty("java.version").split("\\."); + String vendorID = System.getProperty("java.vendor"); + // GNU CLASSPATH conform to JDK1.7, at least in case of Integer.parseInt() method + if ("Free Software Foundation, Inc.".equals(vendorID)) + { + return true; + } + // test of OpenJDK + if ("Sun Microsystems Inc.".equals(vendorID)) + { + return Integer.parseInt(javaVersion[1]) >= 7; + } + return false; // questionable + } + public void test(TestHarness harness) { int i; @@ -106,15 +126,30 @@ } // In JDK1.7, '+' is considered a valid character. - try - { - i = Integer.parseInt("+10"); - harness.check(true); - harness.check(i, 10); - } - catch (NumberFormatException nfe) - { - harness.fail("Leading '+' does not throw NumberFormatException"); + // it means that the following step should be divided + // for pre JDK1.7 case and >= JDK1.7 + if (conformToJDK17()) { + try + { + i = Integer.parseInt("+10"); + harness.check(true); + harness.check(i, 10); + } + catch (NumberFormatException nfe) + { + harness.fail("'+10' string is not parsed correctly as expected in JDK1.7"); + } + } + else { // pre JDK1.7 branch + try + { + i = Integer.parseInt("+10"); + harness.fail("'+10' must throw NumberFormatException"); + } + catch (NumberFormatException nfe) + { + harness.check(true); + } } try |
On 14:42 Thu 18 Aug , Pavel Tisnovsky wrote:
> Dr Andrew John Hughes wrote: > > > As the old behaviour is specific to versions of Sun/Oracle's JDK implementation, > > it should check that this implementation is being used as well, rather than just > > the version. > > Hi Andrew, > > I've tried to add check for JDK/JRE implementation as you suggested. Patch for > parseInt Mauve test is stored in an attachment. > > Cheers, > Pavel This looks better, but you're still changing the old behaviour. Not all Classpath VMs are produced by the FSF. As it's a workaround for Sun: String[] javaVersion = System.getProperty("java.version").split("\\."); String vendorID = System.getProperty("java.vendor"); // test of OpenJDK if ("Sun Microsystems Inc.".equals(vendorID)) { return Integer.parseInt(javaVersion[1]) >= 7; } return true; > --- gnu/testlet/java/lang/Integer/parseInt.java 2008-05-12 23:35:49.000000000 +0200 > +++ gnu/testlet/java/lang/Integer/parseInt.java 2011-08-17 17:17:16.000000000 +0200 > @@ -27,6 +27,26 @@ > > public class parseInt implements Testlet > { > + /** > + * Returns true if tested JRE conformns to JDK 1.7 specification. > + */ > + private static boolean conformToJDK17() > + { > + String[] javaVersion = System.getProperty("java.version").split("\\."); > + String vendorID = System.getProperty("java.vendor"); > + // GNU CLASSPATH conform to JDK1.7, at least in case of Integer.parseInt() method > + if ("Free Software Foundation, Inc.".equals(vendorID)) > + { > + return true; > + } > + // test of OpenJDK > + if ("Sun Microsystems Inc.".equals(vendorID)) > + { > + return Integer.parseInt(javaVersion[1]) >= 7; > + } > + return false; // questionable > + } > + > public void test(TestHarness harness) > { > int i; > @@ -106,15 +126,30 @@ > } > > // In JDK1.7, '+' is considered a valid character. > - try > - { > - i = Integer.parseInt("+10"); > - harness.check(true); > - harness.check(i, 10); > - } > - catch (NumberFormatException nfe) > - { > - harness.fail("Leading '+' does not throw NumberFormatException"); > + // it means that the following step should be divided > + // for pre JDK1.7 case and >= JDK1.7 > + if (conformToJDK17()) { > + try > + { > + i = Integer.parseInt("+10"); > + harness.check(true); > + harness.check(i, 10); > + } > + catch (NumberFormatException nfe) > + { > + harness.fail("'+10' string is not parsed correctly as expected in JDK1.7"); > + } > + } > + else { // pre JDK1.7 branch > + try > + { > + i = Integer.parseInt("+10"); > + harness.fail("'+10' must throw NumberFormatException"); > + } > + catch (NumberFormatException nfe) > + { > + harness.check(true); > + } > } > > try -- Andrew :) Free Java Software Engineer Red Hat, Inc. (http://www.redhat.com) Support Free Java! Contribute to GNU Classpath and IcedTea http://www.gnu.org/software/classpath http://icedtea.classpath.org PGP Key: F5862A37 (https://keys.indymedia.org/) Fingerprint = EA30 D855 D50F 90CD F54D 0698 0713 C3ED F586 2A37 |
In reply to this post by Pavel Tisnovsky
On 10:14 Thu 18 Aug , Pavel Tisnovsky wrote:
> Dr Andrew John Hughes wrote: > > On 18:20 Wed 17 Aug , Pavel Tisnovsky wrote: > >> Greetings, > >> > >> I think that the regression test "gnu/testlet/java/lang/Integer/parseInt.java" > >> should be fixed to work correctly for pre JDK1.7 case (JDK1.0 .. JDK1.6) and > >> also for JDK1.7 case. The difference between JDK1.6 and JDK1.7 is in the > >> behaviour of method Integer.parseInt() when string beginning with '+' sign is > >> parsed: > >> > >> Integer.parseInt("+42") > >> > >> JDK1.6 throws exception in this case, but parsing the same string is perfectly > >> valid in JDK1.7. So I added the condition to the test which checks what JDK is > >> being tested (I can't figure out how Mauve harness checks the "Tags: JDKxx" > >> tag). I also changed the message printed in case JDK1.7 does not work correctly. > >> > >> Is it possible to push the fixed test to Mauve repository please? > >> > >> Thank you in advance, > >> Pavel > > > > Such tests are dubious because they assume compliance to one particular version, > > which has never been true of GNU Classpath and can't be verified due to the lack > > of a Free Software TCK. There will also be versions of OpenJDK7 that report > > as "1.7" but don't yet have this change. > > Hi Andrew, > > you are right that we have not FS TCK so the precise JRE behaviour is > questionable, but I think that any JDK/JRE which identifies itself as "1.6" > should conforms to "Java Platform, Standard Edition 6 API Specification", at > least from common developers point of view. > That doesn't reflect reality. That only works if you make the assumption that the only available JDK implementations implement a complete specification, which fails with Free Software. As I say, it's just as true for OpenJDK. Will every build of OpenJDK8 implement the whole 1.8 specification? I'm pretty sure it will be reporting 1.8 before the specification is signed off. 1.7 certainly did. > > Have you tested this with GNU Classpath? I'm pretty sure it will break the > > test there as it has the modern behaviour, but most GNU Classpath VMs tend > > to report earlier versions due to the level of bytecode they support. > > Yeah, that's true because I've tried it using gij: > $ gij -version > java version "1.5.0" > gij (GNU libgcj) version 4.6.0 20110603 (Red Hat 4.6.0-10) > > Its interesting because it is identified itself as "1.5.0" and parse the string > "+10" without problems. > Yes, because it supports 1.5 *bytecode*. We've implemented quite a few 1.6 methods in GNU Classpath and this 1.7 behaviour is in parseInt. 1.7 methods will start been added as needed. -- Andrew :) Free Java Software Engineer Red Hat, Inc. (http://www.redhat.com) Support Free Java! Contribute to GNU Classpath and IcedTea http://www.gnu.org/software/classpath http://icedtea.classpath.org PGP Key: F5862A37 (https://keys.indymedia.org/) Fingerprint = EA30 D855 D50F 90CD F54D 0698 0713 C3ED F586 2A37 |
Free forum by Nabble | Edit this page |