public inbox for mauve-discuss@sourceware.org
 help / color / mirror / Atom feed
* RFC: fix for Mauve test javax/swing/Border/TitledBorder/constructors.java
@ 2011-09-02 15:31 Pavel Tisnovsky
  2011-09-07  2:39 ` Dr Andrew John Hughes
  0 siblings, 1 reply; 5+ messages in thread
From: Pavel Tisnovsky @ 2011-09-02 15:31 UTC (permalink / raw)
  To: mauve-discuss

Greetings,

here's fix for another Mauve test. In the test
"javax/swing/Border/TitledBorder/constructors.java" the position of the title in
the border should be compared with the constant TitledBorder.DEFAULT_POSITION,
not with TitledBorder.TOP, at least in cases when following constructors are used:

TitledBorder(Border border)
TitledBorder(Border border, String title)
TitledBorder(String title)

(there exist three other constructors where title position can be explicitly
specified, but the fix changes only test cases which use the previous three
constructors).

Here's proposed fix for the test:

--- javax/swing/border/Border/TitledBorder/constructors.java    2006-02-01
15:14:22.000000000 +0100
+++ javax/swing/border/Border/TitledBorder/constructors.java    2011-09-02
16:37:19.000000000 +0200
@@ -77,7 +77,7 @@
     harness.check(tb.getTitleColor(), c);
     Font f = UIManager.getLookAndFeelDefaults().getFont("TitledBorder.font");
     harness.check(tb.getTitleFont(), f);
-    harness.check(tb.getTitlePosition(), TitledBorder.TOP);
+    harness.check(tb.getTitlePosition(), TitledBorder.DEFAULT_POSITION);
     harness.check(tb.getTitleJustification(), TitledBorder.LEADING);

     tb = new TitledBorder((Border) null);
@@ -98,7 +98,7 @@
     harness.check(tb.getTitleColor(), c);
     Font f = UIManager.getLookAndFeelDefaults().getFont("TitledBorder.font");
     harness.check(tb.getTitleFont(), f);
-    harness.check(tb.getTitlePosition(), TitledBorder.TOP);
+    harness.check(tb.getTitlePosition(), TitledBorder.DEFAULT_POSITION);
     harness.check(tb.getTitleJustification(), TitledBorder.LEADING);

     tb = new TitledBorder((Border) null, "XYZ");
@@ -202,7 +202,7 @@
     harness.check(tb.getTitleColor(), c);
     Font f = UIManager.getLookAndFeelDefaults().getFont("TitledBorder.font");
     harness.check(tb.getTitleFont(), f);
-    harness.check(tb.getTitlePosition(), TitledBorder.TOP);
+    harness.check(tb.getTitlePosition(), TitledBorder.DEFAULT_POSITION);
     harness.check(tb.getTitleJustification(), TitledBorder.LEADING);

     tb = new TitledBorder((String) null);
~


Can anybody please review this change?

Thank you in advance,
Pavel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: RFC: fix for Mauve test javax/swing/Border/TitledBorder/constructors.java
  2011-09-02 15:31 RFC: fix for Mauve test javax/swing/Border/TitledBorder/constructors.java Pavel Tisnovsky
@ 2011-09-07  2:39 ` Dr Andrew John Hughes
  2011-09-07 12:53   ` Pavel Tisnovsky
  0 siblings, 1 reply; 5+ messages in thread
From: Dr Andrew John Hughes @ 2011-09-07  2:39 UTC (permalink / raw)
  To: Pavel Tisnovsky; +Cc: mauve-discuss

On 17:33 Fri 02 Sep     , Pavel Tisnovsky wrote:
> Greetings,
> 
> here's fix for another Mauve test. In the test
> "javax/swing/Border/TitledBorder/constructors.java" the position of the title in
> the border should be compared with the constant TitledBorder.DEFAULT_POSITION,
> not with TitledBorder.TOP, at least in cases when following constructors are used:
> 
> TitledBorder(Border border)
> TitledBorder(Border border, String title)
> TitledBorder(String title)
> 
> (there exist three other constructors where title position can be explicitly
> specified, but the fix changes only test cases which use the previous three
> constructors).
> 

Can you explain why and note whether this affects current test results?

-- 
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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: RFC: fix for Mauve test javax/swing/Border/TitledBorder/constructors.java
  2011-09-07  2:39 ` Dr Andrew John Hughes
@ 2011-09-07 12:53   ` Pavel Tisnovsky
  2011-09-14  8:55     ` Pavel Tisnovsky
  0 siblings, 1 reply; 5+ messages in thread
From: Pavel Tisnovsky @ 2011-09-07 12:53 UTC (permalink / raw)
  To: Dr Andrew John Hughes; +Cc: mauve-discuss

Dr Andrew John Hughes wrote:
> On 17:33 Fri 02 Sep     , Pavel Tisnovsky wrote:
>> Greetings,
>>
>> here's fix for another Mauve test. In the test
>> "javax/swing/Border/TitledBorder/constructors.java" the position of the title in
>> the border should be compared with the constant TitledBorder.DEFAULT_POSITION,
>> not with TitledBorder.TOP, at least in cases when following constructors are used:
>>
>> TitledBorder(Border border)
>> TitledBorder(Border border, String title)
>> TitledBorder(String title)
>>
>> (there exist three other constructors where title position can be explicitly
>> specified, but the fix changes only test cases which use the previous three
>> constructors).
>>
> 
> Can you explain why and note whether this affects current test results?
> 

Hi Andrew,

well, I've gone deeper into this issue and it seems, that GNU Classpath and
OpenJDK have quite different behaviour, but both are AFAIK correct due to
imprecise specification. When one of the following constructors are used to
create TitledBorder:

TitledBorder(Border border)
TitledBorder(Border border, String title)
TitledBorder(String title)

the text position is considered as DEFAULT_POSITION which * is equals * to TOP.
The only difference between GNU Classpath / OpenJDK is that GNU Classpath
explicitly sets text position to TOP in the constructor's body:

public TitledBorder(String title)
{
     this(/* border */ null, title, LEADING, TOP, /* titleFont */ null, /*
titleColor */ null);
}

but OpenJDK still stores DEFAULT_POSITION in following types of code:

switch (position) {
   case TOP:
   case DEFAULT:
       the same code block for TOP & DEFAULT

(the same pattern are used for text justification, ie for constants LEFT and
DEFAULT_JUSTIFICATION).

This means that TitledBorder("title").getTitlePosition()==TitledBorder.TOP on
GNU Classpath but
TitledBorder("title").getTitlePosition()==TitledBorder.DEFAULT_POSITION on
OpenJDK and proprietary JDKs too.

So it's IMHO better to change the test to check for both possibilities.

Pavel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: RFC: fix for Mauve test javax/swing/Border/TitledBorder/constructors.java
  2011-09-07 12:53   ` Pavel Tisnovsky
@ 2011-09-14  8:55     ` Pavel Tisnovsky
  2011-09-14  9:11       ` Mark Wielaard
  0 siblings, 1 reply; 5+ messages in thread
From: Pavel Tisnovsky @ 2011-09-14  8:55 UTC (permalink / raw)
  To: mauve-discuss

Greetings,

here's the new version of a patch for a Mauve test
javax/swing/Border/TitledBorder/constructors.java. This patch make this test
work correctly on OpenJDK & GNU Classpath too:

--- mauve_old/gnu/testlet/javax/swing/border/TitledBorder/constructors.java
2006-02-01 15:14:22.000000000 +0100
+++ mauve_new/gnu/testlet/javax/swing/border/TitledBorder/constructors.java
2011-09-14 10:48:54.000000000 +0200
@@ -65,6 +65,11 @@
     test6(harness);
   }

+  public void checkTitledBorderDefaultPosition(TestHarness harness, int position)
+  {
+    harness.check(position == TitledBorder.TOP || position ==
TitledBorder.DEFAULT_POSITION);
+  }
+
   public void test1(TestHarness harness)
   {
     harness.checkPoint("(Border)");
@@ -77,7 +82,7 @@
     harness.check(tb.getTitleColor(), c);
     Font f = UIManager.getLookAndFeelDefaults().getFont("TitledBorder.font");
     harness.check(tb.getTitleFont(), f);
-    harness.check(tb.getTitlePosition(), TitledBorder.TOP);
+    checkTitledBorderDefaultPosition(harness, tb.getTitlePosition());
     harness.check(tb.getTitleJustification(), TitledBorder.LEADING);

     tb = new TitledBorder((Border) null);
@@ -98,7 +103,7 @@
     harness.check(tb.getTitleColor(), c);
     Font f = UIManager.getLookAndFeelDefaults().getFont("TitledBorder.font");
     harness.check(tb.getTitleFont(), f);
-    harness.check(tb.getTitlePosition(), TitledBorder.TOP);
+    checkTitledBorderDefaultPosition(harness, tb.getTitlePosition());
     harness.check(tb.getTitleJustification(), TitledBorder.LEADING);

     tb = new TitledBorder((Border) null, "XYZ");
@@ -202,7 +207,7 @@
     harness.check(tb.getTitleColor(), c);
     Font f = UIManager.getLookAndFeelDefaults().getFont("TitledBorder.font");
     harness.check(tb.getTitleFont(), f);
-    harness.check(tb.getTitlePosition(), TitledBorder.TOP);
+    checkTitledBorderDefaultPosition(harness, tb.getTitlePosition());
     harness.check(tb.getTitleJustification(), TitledBorder.LEADING);

     tb = new TitledBorder((String) null);

Any comments are welcome.

Pavel


Pavel Tisnovsky wrote:
> Dr Andrew John Hughes wrote:
>> On 17:33 Fri 02 Sep     , Pavel Tisnovsky wrote:
>>> Greetings,
>>>
>>> here's fix for another Mauve test. In the test
>>> "javax/swing/Border/TitledBorder/constructors.java" the position of the title in
>>> the border should be compared with the constant TitledBorder.DEFAULT_POSITION,
>>> not with TitledBorder.TOP, at least in cases when following constructors are used:
>>>
>>> TitledBorder(Border border)
>>> TitledBorder(Border border, String title)
>>> TitledBorder(String title)
>>>
>>> (there exist three other constructors where title position can be explicitly
>>> specified, but the fix changes only test cases which use the previous three
>>> constructors).
>>>
>> Can you explain why and note whether this affects current test results?
>>
> 
> Hi Andrew,
> 
> well, I've gone deeper into this issue and it seems, that GNU Classpath and
> OpenJDK have quite different behaviour, but both are AFAIK correct due to
> imprecise specification. When one of the following constructors are used to
> create TitledBorder:
> 
> TitledBorder(Border border)
> TitledBorder(Border border, String title)
> TitledBorder(String title)
> 
> the text position is considered as DEFAULT_POSITION which * is equals * to TOP.
> The only difference between GNU Classpath / OpenJDK is that GNU Classpath
> explicitly sets text position to TOP in the constructor's body:
> 
> public TitledBorder(String title)
> {
>      this(/* border */ null, title, LEADING, TOP, /* titleFont */ null, /*
> titleColor */ null);
> }
> 
> but OpenJDK still stores DEFAULT_POSITION in following types of code:
> 
> switch (position) {
>    case TOP:
>    case DEFAULT:
>        the same code block for TOP & DEFAULT
> 
> (the same pattern are used for text justification, ie for constants LEFT and
> DEFAULT_JUSTIFICATION).
> 
> This means that TitledBorder("title").getTitlePosition()==TitledBorder.TOP on
> GNU Classpath but
> TitledBorder("title").getTitlePosition()==TitledBorder.DEFAULT_POSITION on
> OpenJDK and proprietary JDKs too.
> 
> So it's IMHO better to change the test to check for both possibilities.
> 
> Pavel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: RFC: fix for Mauve test javax/swing/Border/TitledBorder/constructors.java
  2011-09-14  8:55     ` Pavel Tisnovsky
@ 2011-09-14  9:11       ` Mark Wielaard
  0 siblings, 0 replies; 5+ messages in thread
From: Mark Wielaard @ 2011-09-14  9:11 UTC (permalink / raw)
  To: Pavel Tisnovsky; +Cc: mauve-discuss

On Wed, 2011-09-14 at 10:57 +0200, Pavel Tisnovsky wrote:
> here's the new version of a patch for a Mauve test
> javax/swing/Border/TitledBorder/constructors.java. This patch make this test
> work correctly on OpenJDK & GNU Classpath too:

IMHO you just found a bug in GNU Classpath. If no position is given it
should use DEFAULT_POSITION, not TOP. The test was wrong, and I think
your original fix was correct.

Cheers,

Mark

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2011-09-14  9:11 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-02 15:31 RFC: fix for Mauve test javax/swing/Border/TitledBorder/constructors.java Pavel Tisnovsky
2011-09-07  2:39 ` Dr Andrew John Hughes
2011-09-07 12:53   ` Pavel Tisnovsky
2011-09-14  8:55     ` Pavel Tisnovsky
2011-09-14  9:11       ` Mark Wielaard

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).