harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ivanov, Alexey A" <alexey.a.iva...@intel.com>
Subject RE: [classlib][test] isHarmony method in the swing tests
Date Fri, 24 Nov 2006 07:28:25 GMT
>-----Original Message-----
>From: Mikhail Loenko [mailto:mloenko@gmail.com]
>Sent: Friday, November 24, 2006 8:50 AM
>To: dev@harmony.apache.org
>Subject: Re: [classlib][test] isHarmony method in the swing tests
>
>2006/11/23, Ivanov, Alexey A <alexey.a.ivanov@intel.com>:
>> Almost all new tests discover the incompatibilities.
>>
>> I think the exception doesn't matter much here 'cause passing null
>> string causes NPE on both Harmony and RI. But because RI validates
>> offset earlier, BadLocationException is thrown.
>>
>> I see no reason to validate offset earlier because it'll lead to
>> duplication in code.
>
>According to our conventions [1] we should follow RI's behavior here.
May
>be
>it's not the biggest problem at the moment, but we should not hide it.
>
>So I'm -1 for integrating the test that hides the problem.

You are free to provide a patch to fix the problem :)

And this incompatibility is *clearly* stated in comments for the issue
[2]. 


Regards,
Alexey.

[2] https://issues.apache.org/jira/browse/HARMONY-2198


P.S. I could have concealed it from the community by not adding this
test case to the patch, but I don't want to. It's a minor
incompatibility, and the test doesn't hide it but vice versa *clearly*
describes there's one. javax.swing.text.AbstractDocument uses GapContent
as the store for the text, and it do never pass null strings to
GapContent.insertString.

>
>Thanks,
>Mikhail
>
>[1] http://wiki.apache.org/harmony/Exception-throwing_compatibility
>
>
>Or the code should be re-designed to avoid
>> duplication. Since GapContent class is very low-level implementation
>> class, I think is not a major problem right now. A volunteer may
provide
>> patches to resolve all the incompatibilities but I consider them as
very
>> low-priority at the moment, moreover the fix is not straight forward
(if
>> it was, I'd have fixed the incompatibility). That's my reasons to
keep
>> it as is right now.
>>
>> Regards,
>> --
>> Alexey A. Ivanov
>> Intel Enterprise Solutions Software Division
>>
>>
>> >-----Original Message-----
>> >From: Mikhail Loenko [mailto:mloenko@gmail.com]
>> >Sent: Thursday, November 23, 2006 3:50 PM
>> >To: dev@harmony.apache.org
>> >Subject: Re: [classlib][test] isHarmony method in the swing tests
>> >
>> >Why we expect different exceptions? I think this test
>> >discovers incompatibility and should be just fixed to expect the
same
>> >exception
>> >
>> >Thanks,
>> >Mikhail
>> >
>> >2006/11/23, Ivanov, Alexey A <alexey.a.ivanov@intel.com>:
>> >> Yeah, I remember about TestNG. Yet I think it won't solve all the
>> cases
>> >> where isHarmony used.
>> >>
>> >> For example, look at the tests in
>> >> https://issues.apache.org/jira/browse/HARMONY-2198
>> >> The isHarmony() method is used in if-else context there which
>> >> demonstrates the difference between Harmony and RI. And mostly it
is
>> >> if-else context that isHarmony() is used.
>> >>
>> >> Regards,
>> >> --
>> >> Alexey A. Ivanov
>> >> Intel Enterprise Solutions Software Division
>> >>
>> >>
>> >> >-----Original Message-----
>> >> >From: Mikhail Loenko [mailto:mloenko@gmail.com]
>> >> >Sent: Thursday, November 23, 2006 2:39 PM
>> >> >To: dev@harmony.apache.org
>> >> >Subject: Re: [classlib][test] isHarmony method in the swing tests
>> >> >
>> >> >We are going to swith to TestNG.
>> >> >
>> >> >So we will be able to handle all that stuff there, won't we?
>> >> >
>> >> >Thanks,
>> >> >Mikhail
>> >> >
>> >> >2006/11/23, Ivanov, Alexey A <alexey.a.ivanov@intel.com>:
>> >> >> Mikhail,
>> >> >>
>> >> >> Here it's not a temporary solution.
>> >> >>
>> >> >> javax.swing.text.PlainViewI18N is for bidirectional text
support.
>> It
>> >> is
>> >> >> a package-private class, and it's not present in public API
spec.
>> >> >>
>> >> >> Sun doesn't reveal its implementation of bidirectional text. I
>> guess
>> >> >> it's fully implemented yet: there are problems with it. What I
can
>> >> >> remember at once is you can't go through all the text using
right
>> or
>> >> >> left arrows on keyboard because the caret jumps back.
>> >> >>
>> >> >> In general this method is used to differentiate our
implementation
>> >> from
>> >> >> Sun. These differences are intentional. To make the tests pass
>> both
>> >> on
>> >> >> RI and Harmony, it is checked which classlib is used. Also
looking
>> at
>> >> >> the tests one sees the expected difference.
>> >> >>
>> >> >> Regards,
>> >> >> Alexey.
>> >> >>
>> >> >> P.S. We can get rid of using this method and sort out the tests
to
>> >> >> separate implementation specific tests, but it requires lots of
>> >> effort.
>> >> >> On the other hand, some tests will lose the information about
the
>> >> >> difference. Subsequent releases of Java may change the behavior
>> and
>> >> >> we'll see it because of failing tests. This way we can adjust
our
>> >> >> implementation to the new RI impl.
>> >> >>
>> >> >> --
>> >> >> Alexey A. Ivanov
>> >> >> Intel Enterprise Solutions Software Division
>> >> >>
>> >> >>
>> >> >> >-----Original Message-----
>> >> >> >From: Mikhail Loenko [mailto:mloenko@gmail.com]
>> >> >> >Sent: Thursday, November 23, 2006 10:22 AM
>> >> >> >To: dev@harmony.apache.org
>> >> >> >Subject: [classlib][test] isHarmony method in the swing tests
>> >> >> >
>> >> >> >Did I understand correctly that it's a temporary solution to
>> >> >> >differentiate between
>> >> >> >"api" and "impl" tests?
>> >> >> >
>> >> >> >package javax.swing.text;
>> >> >> ><...>
>> >> >> >public class PlainViewI18N_LineViewTest extends SwingTestCase
{
>> >> >> ><...>
>> >> >> >    public void testGetPreferredSpan01() throws Exception {
>> >> >> >        if (!isHarmony()) {
>> >> >> >            return;
>> >> >> >        }
>> >> >>
>> >>
>>

--
Alexey A. Ivanov
Intel Enterprise Solutions Software Division

Mime
View raw message