RFC: fix for Mauve test javax/swing/JTextArea/prefferedSize.java

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

RFC: fix for Mauve test javax/swing/JTextArea/prefferedSize.java

Pavel Tisnovsky
Greetings,

I think that the last 6 checks contained in Mauve test
"javax/swing/JTextArea/prefferedSize.java" should be changed. In the original
test, the preferred width of the JTextArea is compared with 100 pixels after
word line wrapping is enabled.

But it does not make sense (IMHO) because the JTextArea contains only empty text
or some NL characters ('\n'), which means, that preferred and minimum width of
JTextArea should be 0 pixels, not 100 in both cases because empty text and NL
sequence has only some non-zero height, but zero width. This behaviour is
independent of auto wrapping and/or word wrapping.

It's probably worth to add some check for preferred height.

Here is proposed change to this test:

--- preferredSize.java  2005-12-07 20:23:38.000000000 +0100
+++ prefferedsize.java  2011-09-02 16:02:00.000000000 +0200
@@ -51,14 +51,14 @@
         ta2.setLineWrap(true);
         ta2.setWrapStyleWord(true);

-        harness.check (ta2.getPreferredSize().width == 100);
-        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 100);
+        harness.check (ta2.getPreferredSize().width == 0);
+        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 0);
         ta2.setText("");
-        harness.check (ta2.getPreferredSize().width == 100);
-        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 100);
+        harness.check (ta2.getPreferredSize().width == 0);
+        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 0);
         ta2.setText("\n\n\n\n\n\n\n\n\n");
-        harness.check (ta2.getPreferredSize().width == 100);
-        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 100);
+        harness.check (ta2.getPreferredSize().width == 0);
+        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 0);
       }
     catch (Exception e)
       {
~

Is it possible to change this test case please?

Cheers,
Pavel
Reply | Threaded
Open this post in threaded view
|

Re: RFC: fix for Mauve test javax/swing/JTextArea/prefferedSize.java

ahughes-2
On 16:17 Fri 02 Sep     , Pavel Tisnovsky wrote:

> Greetings,
>
> I think that the last 6 checks contained in Mauve test
> "javax/swing/JTextArea/prefferedSize.java" should be changed. In the original
> test, the preferred width of the JTextArea is compared with 100 pixels after
> word line wrapping is enabled.
>
> But it does not make sense (IMHO) because the JTextArea contains only empty text
> or some NL characters ('\n'), which means, that preferred and minimum width of
> JTextArea should be 0 pixels, not 100 in both cases because empty text and NL
> sequence has only some non-zero height, but zero width. This behaviour is
> independent of auto wrapping and/or word wrapping.
>
> It's probably worth to add some check for preferred height.
>
> Here is proposed change to this test:
>
> --- preferredSize.java  2005-12-07 20:23:38.000000000 +0100
> +++ prefferedsize.java  2011-09-02 16:02:00.000000000 +0200
> @@ -51,14 +51,14 @@
>          ta2.setLineWrap(true);
>          ta2.setWrapStyleWord(true);
>
> -        harness.check (ta2.getPreferredSize().width == 100);
> -        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 100);
> +        harness.check (ta2.getPreferredSize().width == 0);
> +        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 0);
>          ta2.setText("");
> -        harness.check (ta2.getPreferredSize().width == 100);
> -        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 100);
> +        harness.check (ta2.getPreferredSize().width == 0);
> +        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 0);
>          ta2.setText("\n\n\n\n\n\n\n\n\n");
> -        harness.check (ta2.getPreferredSize().width == 100);
> -        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 100);
> +        harness.check (ta2.getPreferredSize().width == 0);
> +        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 0);
>        }
>      catch (Exception e)
>        {
> ~
>
> Is it possible to change this test case please?
>
> Cheers,
> Pavel

Do tests still pass with these changes?
--
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/JTextArea/prefferedSize.java

Pavel Tisnovsky
Dr Andrew John Hughes wrote:

> On 16:17 Fri 02 Sep     , Pavel Tisnovsky wrote:
>> Greetings,
>>
>> I think that the last 6 checks contained in Mauve test
>> "javax/swing/JTextArea/prefferedSize.java" should be changed. In the original
>> test, the preferred width of the JTextArea is compared with 100 pixels after
>> word line wrapping is enabled.
>>
>> But it does not make sense (IMHO) because the JTextArea contains only empty text
>> or some NL characters ('\n'), which means, that preferred and minimum width of
>> JTextArea should be 0 pixels, not 100 in both cases because empty text and NL
>> sequence has only some non-zero height, but zero width. This behaviour is
>> independent of auto wrapping and/or word wrapping.
>>
>> It's probably worth to add some check for preferred height.
>>
>> Here is proposed change to this test:
>>
>> --- preferredSize.java  2005-12-07 20:23:38.000000000 +0100
>> +++ prefferedsize.java  2011-09-02 16:02:00.000000000 +0200
>> @@ -51,14 +51,14 @@
>>          ta2.setLineWrap(true);
>>          ta2.setWrapStyleWord(true);
>>
>> -        harness.check (ta2.getPreferredSize().width == 100);
>> -        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 100);
>> +        harness.check (ta2.getPreferredSize().width == 0);
>> +        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 0);
>>          ta2.setText("");
>> -        harness.check (ta2.getPreferredSize().width == 100);
>> -        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 100);
>> +        harness.check (ta2.getPreferredSize().width == 0);
>> +        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 0);
>>          ta2.setText("\n\n\n\n\n\n\n\n\n");
>> -        harness.check (ta2.getPreferredSize().width == 100);
>> -        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 100);
>> +        harness.check (ta2.getPreferredSize().width == 0);
>> +        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 0);
>>        }
>>      catch (Exception e)
>>        {
>> ~
>>
>> Is it possible to change this test case please?
>>
>> Cheers,
>> Pavel
>
> Do tests still pass with these changes?

yes
Reply | Threaded
Open this post in threaded view
|

Re: RFC: fix for Mauve test javax/swing/JTextArea/prefferedSize.java

ahughes-2
On 17:09 Fri 02 Sep     , Pavel Tisnovsky wrote:

> Dr Andrew John Hughes wrote:
> > On 16:17 Fri 02 Sep     , Pavel Tisnovsky wrote:
> >> Greetings,
> >>
> >> I think that the last 6 checks contained in Mauve test
> >> "javax/swing/JTextArea/prefferedSize.java" should be changed. In the original
> >> test, the preferred width of the JTextArea is compared with 100 pixels after
> >> word line wrapping is enabled.
> >>
> >> But it does not make sense (IMHO) because the JTextArea contains only empty text
> >> or some NL characters ('\n'), which means, that preferred and minimum width of
> >> JTextArea should be 0 pixels, not 100 in both cases because empty text and NL
> >> sequence has only some non-zero height, but zero width. This behaviour is
> >> independent of auto wrapping and/or word wrapping.
> >>
> >> It's probably worth to add some check for preferred height.
> >>
> >> Here is proposed change to this test:
> >>
> >> --- preferredSize.java  2005-12-07 20:23:38.000000000 +0100
> >> +++ prefferedsize.java  2011-09-02 16:02:00.000000000 +0200
> >> @@ -51,14 +51,14 @@
> >>          ta2.setLineWrap(true);
> >>          ta2.setWrapStyleWord(true);
> >>
> >> -        harness.check (ta2.getPreferredSize().width == 100);
> >> -        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 100);
> >> +        harness.check (ta2.getPreferredSize().width == 0);
> >> +        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 0);
> >>          ta2.setText("");
> >> -        harness.check (ta2.getPreferredSize().width == 100);
> >> -        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 100);
> >> +        harness.check (ta2.getPreferredSize().width == 0);
> >> +        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 0);
> >>          ta2.setText("\n\n\n\n\n\n\n\n\n");
> >> -        harness.check (ta2.getPreferredSize().width == 100);
> >> -        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 100);
> >> +        harness.check (ta2.getPreferredSize().width == 0);
> >> +        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 0);
> >>        }
> >>      catch (Exception e)
> >>        {
> >> ~
> >>
> >> Is it possible to change this test case please?
> >>
> >> Cheers,
> >> Pavel
> >
> > Do tests still pass with these changes?
>
> yes

On both OpenJDK and GNU Classpath?
--
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/JTextArea/prefferedSize.java

Pavel Tisnovsky
Dr Andrew John Hughes wrote:

> On 17:09 Fri 02 Sep     , Pavel Tisnovsky wrote:
>> Dr Andrew John Hughes wrote:
>>> On 16:17 Fri 02 Sep     , Pavel Tisnovsky wrote:
>>>> Greetings,
>>>>
>>>> I think that the last 6 checks contained in Mauve test
>>>> "javax/swing/JTextArea/prefferedSize.java" should be changed. In the original
>>>> test, the preferred width of the JTextArea is compared with 100 pixels after
>>>> word line wrapping is enabled.
>>>>
>>>> But it does not make sense (IMHO) because the JTextArea contains only empty text
>>>> or some NL characters ('\n'), which means, that preferred and minimum width of
>>>> JTextArea should be 0 pixels, not 100 in both cases because empty text and NL
>>>> sequence has only some non-zero height, but zero width. This behaviour is
>>>> independent of auto wrapping and/or word wrapping.
>>>>
>>>> It's probably worth to add some check for preferred height.
>>>>
>>>> Here is proposed change to this test:
>>>>
>>>> --- preferredSize.java  2005-12-07 20:23:38.000000000 +0100
>>>> +++ prefferedsize.java  2011-09-02 16:02:00.000000000 +0200
>>>> @@ -51,14 +51,14 @@
>>>>          ta2.setLineWrap(true);
>>>>          ta2.setWrapStyleWord(true);
>>>>
>>>> -        harness.check (ta2.getPreferredSize().width == 100);
>>>> -        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 100);
>>>> +        harness.check (ta2.getPreferredSize().width == 0);
>>>> +        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 0);
>>>>          ta2.setText("");
>>>> -        harness.check (ta2.getPreferredSize().width == 100);
>>>> -        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 100);
>>>> +        harness.check (ta2.getPreferredSize().width == 0);
>>>> +        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 0);
>>>>          ta2.setText("\n\n\n\n\n\n\n\n\n");
>>>> -        harness.check (ta2.getPreferredSize().width == 100);
>>>> -        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 100);
>>>> +        harness.check (ta2.getPreferredSize().width == 0);
>>>> +        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 0);
>>>>        }
>>>>      catch (Exception e)
>>>>        {
>>>> ~
>>>>
>>>> Is it possible to change this test case please?
>>>>
>>>> Cheers,
>>>> Pavel
>>> Do tests still pass with these changes?
>> yes
>
> On both OpenJDK and GNU Classpath?

They pass only on OpenJDK. On GNU Classpath (4.4.4), some tests failed
regardless of whether the patch has been applied or not. But the failures
occurred on previous test not touched by the patch:

        ta2.setText("");
        harness.check (ta2.getPreferredSize().width == 0);
        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 0);
        ta2.setText("\n\n\n\n\n\n\n\n\n");
        harness.check (ta2.getPreferredSize().width == 0);
        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 0);

before the line wrap & word wrap is enabled.

The proposed patch fixes other failures on OpenJDK & GNU Classpath too, ie:
OpenJDK:       6 failures -> 0 failures
GNU Classpath: 8 failures -> 2 failures

Pavel
Reply | Threaded
Open this post in threaded view
|

Re: RFC: fix for Mauve test javax/swing/JTextArea/prefferedSize.java

ahughes-2
On 13:35 Wed 07 Sep     , Pavel Tisnovsky wrote:

> Dr Andrew John Hughes wrote:
> > On 17:09 Fri 02 Sep     , Pavel Tisnovsky wrote:
> >> Dr Andrew John Hughes wrote:
> >>> On 16:17 Fri 02 Sep     , Pavel Tisnovsky wrote:
> >>>> Greetings,
> >>>>
> >>>> I think that the last 6 checks contained in Mauve test
> >>>> "javax/swing/JTextArea/prefferedSize.java" should be changed. In the original
> >>>> test, the preferred width of the JTextArea is compared with 100 pixels after
> >>>> word line wrapping is enabled.
> >>>>
> >>>> But it does not make sense (IMHO) because the JTextArea contains only empty text
> >>>> or some NL characters ('\n'), which means, that preferred and minimum width of
> >>>> JTextArea should be 0 pixels, not 100 in both cases because empty text and NL
> >>>> sequence has only some non-zero height, but zero width. This behaviour is
> >>>> independent of auto wrapping and/or word wrapping.
> >>>>
> >>>> It's probably worth to add some check for preferred height.
> >>>>
> >>>> Here is proposed change to this test:
> >>>>
> >>>> --- preferredSize.java  2005-12-07 20:23:38.000000000 +0100
> >>>> +++ prefferedsize.java  2011-09-02 16:02:00.000000000 +0200
> >>>> @@ -51,14 +51,14 @@
> >>>>          ta2.setLineWrap(true);
> >>>>          ta2.setWrapStyleWord(true);
> >>>>
> >>>> -        harness.check (ta2.getPreferredSize().width == 100);
> >>>> -        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 100);
> >>>> +        harness.check (ta2.getPreferredSize().width == 0);
> >>>> +        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 0);
> >>>>          ta2.setText("");
> >>>> -        harness.check (ta2.getPreferredSize().width == 100);
> >>>> -        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 100);
> >>>> +        harness.check (ta2.getPreferredSize().width == 0);
> >>>> +        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 0);
> >>>>          ta2.setText("\n\n\n\n\n\n\n\n\n");
> >>>> -        harness.check (ta2.getPreferredSize().width == 100);
> >>>> -        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 100);
> >>>> +        harness.check (ta2.getPreferredSize().width == 0);
> >>>> +        harness.check (view.getPreferredSpan(View.HORIZONTAL) == 0);
> >>>>        }
> >>>>      catch (Exception e)
> >>>>        {
> >>>> ~
> >>>>
> >>>> Is it possible to change this test case please?
> >>>>
> >>>> Cheers,
> >>>> Pavel
> >>> Do tests still pass with these changes?
> >> yes
> >
> > On both OpenJDK and GNU Classpath?
>
> They pass only on OpenJDK. On GNU Classpath (4.4.4), some tests failed
> regardless of whether the patch has been applied or not. But the failures
> occurred on previous test not touched by the patch:
>
>         ta2.setText("");
>         harness.check (ta2.getPreferredSize().width == 0);
>         harness.check (view.getPreferredSpan(View.HORIZONTAL) == 0);
>         ta2.setText("\n\n\n\n\n\n\n\n\n");
>         harness.check (ta2.getPreferredSize().width == 0);
>         harness.check (view.getPreferredSpan(View.HORIZONTAL) == 0);
>
> before the line wrap & word wrap is enabled.
>
> The proposed patch fixes other failures on OpenJDK & GNU Classpath too, ie:
> OpenJDK:       6 failures -> 0 failures
> GNU Classpath: 8 failures -> 2 failures
>
> Pavel

You seem to have sent this twice...

I guess this is ok, but I have to admit I don't really know this GUI stuff that
well.  Would be good if someone else could take a look.
--
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/JTextArea/prefferedSize.java

Mark Wielaard
On Wed, 2011-09-07 at 16:35 +0100, Dr Andrew John Hughes wrote:
> On 13:35 Wed 07 Sep     , Pavel Tisnovsky wrote:
> I guess this is ok, but I have to admit I don't really know this GUI stuff that
> well.  Would be good if someone else could take a look.

It does make sense to me to change the tests as Pavel suggests.

Cheers,

Mark