RFC: fix for Mauve test javax/swing/Border/TitledBorder/constructors.java

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

RFC: fix for Mauve test javax/swing/Border/TitledBorder/constructors.java

Pavel Tisnovsky
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
Reply | Threaded
Open this post in threaded view
|

Re: RFC: fix for Mauve test javax/swing/Border/TitledBorder/constructors.java

ahughes-2
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
Reply | Threaded
Open this post in threaded view
|

Re: RFC: fix for Mauve test javax/swing/Border/TitledBorder/constructors.java

Pavel Tisnovsky
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
Reply | Threaded
Open this post in threaded view
|

Re: RFC: fix for Mauve test javax/swing/Border/TitledBorder/constructors.java

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

Reply | Threaded
Open this post in threaded view
|

Re: RFC: fix for Mauve test javax/swing/Border/TitledBorder/constructors.java

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