public inbox for mauve-discuss@sourceware.org
 help / color / mirror / Atom feed
From: Dr Andrew John Hughes <ahughes@redhat.com>
To: Pavel Tisnovsky <ptisnovs@redhat.com>
Cc: mauve-discuss@sourceware.org
Subject: Re: Fix for test gnu/testlet/java/lang/Integer/parseInt.java
Date: Wed, 17 Aug 2011 16:27:00 -0000	[thread overview]
Message-ID: <20110817162659.GB9583@rivendell.middle-earth.co.uk> (raw)
In-Reply-To: <4E4BEA69.9040702@redhat.com>

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

  reply	other threads:[~2011-08-17 16:27 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-08-17 16:19 Pavel Tisnovsky
2011-08-17 16:27 ` Dr Andrew John Hughes [this message]
2011-08-18  8:12   ` Pavel Tisnovsky
2011-08-18 15:50     ` Dr Andrew John Hughes
2011-08-18 12:41   ` Pavel Tisnovsky
2011-08-18 15:46     ` Dr Andrew John Hughes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110817162659.GB9583@rivendell.middle-earth.co.uk \
    --to=ahughes@redhat.com \
    --cc=mauve-discuss@sourceware.org \
    --cc=ptisnovs@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).