<AWT Dev> <Awt Dev> [9] Review Request for 8134732: [TEST_BUG] Test java/awt/applet/Applet/AppletFlipBuffer.java fails on Windows with AWTException

Wed Sep 9 14:11:29 UTC 2015

On 09.09.15 16:01, Semyon Sadetsky wrote:
>
>
> On 9/9/2015 3:29 PM, Sergey Bylokhov wrote:
>> On 08.09.15 18:38, Semyon Sadetsky wrote:
>>>
>>>
>>> On 9/8/2015 2:11 PM, Sergey Bylokhov wrote:
>>>> On 08.09.15 14:03, Semyon Sadetsky wrote:
>>>>>
>>>>>
>>>>> On 9/8/2015 1:37 PM, Sergey Bylokhov wrote:
>>>>>> On 08.09.15 13:31, Semyon Sadetsky wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 9/8/2015 12:38 PM, Sergey Bylokhov wrote:
>>>>>>>> On 08.09.15 9:19, Semyon Sadetsky wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 9/7/2015 6:47 PM, Sergey Bylokhov wrote:
>>>>>>>>>> On 07.09.15 16:26, Semyon Sadetsky wrote:
>>>>>>>>>>> We cannot guarantee that the used way of the creation of the
>>>>>>>>>>> buffer
>>>>>>>>>>> strategy will not cause any unchecked exceptions.
>>>>>>>>>>
>>>>>>>>>> We can, because the list of exceptions is described in its docs,
>>>>>>>>>> all
>>>>>>>>>> other exceptions will mean a bug in implementation or in
>>>>>>>>>> documentation.
>>>>>>>>> But the test is essentially synthetic and it induces a lot of
>>>>>>>>> hardware
>>>>>>>>> related code. So to state the above you need to investigate all
>>>>>>>>> exception for all possible implementations. Did you do that?
>>>>>>>>
>>>>>>>> We call the one method only, this method has a list of
>>>>>>>> exceptions. It
>>>>>>>> is not necessary to check all methods, if we write a test
>>>>>>>> correctly,
>>>>>>>> because in this case the test finds all unspecified exceptions,
>>>>>>>> which
>>>>>>>> will causes a new CR. The test suggested by you simply skip such
>>>>>>>> exceptions which means it skip the bugs, no?
>>>>>>> No. The test aim is to check the fix that constructor accepts
>>>>>>> parameter
>>>>>>> of a certain type. It cannot serve as flip buffer construction test
>>>>>>> because it is _synthetic_: all parameters are adjusted to simulate a
>>>>>>> specific lines coverage without taking into account the underlying
>>>>>>> platform features. Without the investigation it will bring yet
>>>>>>> another
>>>>>>> regression.
>>>>>>
>>>>>> Do the parameters contradict the specification of this method? If not
>>>>>> then parameters should be accepted. Moreover just look to this bug
>>>>>> again this method generated classcast exception which was not
>>>>>> specified and we fixed it, all other cases should be done in the same
>>>>>> way.
>>>>> Yes, generally you are correct this would be useful. But this requires
>>>>> the extra work that should be done otherwise it will be just a source
>>>>> for regression.
>>>>> I don't see that results of this extra efforts are worthwhile.
>>>>
>>>> The extra work for now is just to use only specified exceptions in the
>>>> catch block. All other work will be necessary if some bugs will be
>>>> reised, but this is good we will find the new bug and of course it
>>>> will require some additional work.
>>> And I'm not sure that this raised bug will be other than a test bug. I
>>> don't like such experimental tests because they are time wasting. The
>>> code you would like to test is better covered in many other test suites
>>> which are not synthetic like this one.
>>
>> I describe above why this code in the test is bad:
> But you failed to convince me.
>> 53         catch (Exception e) {
>>   54
>>   55         }
>> This webrev is rejected.
> np. I reassigned the issue on you. You can modify my fix as you wish and
> commit it on your behalf.

Sure, I'll fix it, if you have some other interesting issues assign them 
to me too, please. I'll be happy to fix them for you=)

>>
>>>>
>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> We don't need to
>>>>>>>>>>> stumble on them.
>>>>>>>>>>>
>>>>>>>>>>> On 9/7/2015 2:59 PM, Sergey Bylokhov wrote:
>>>>>>>>>>>> On 04.09.15 17:42, Semyon Sadetsky wrote:
>>>>>>>>>>>>> Yes, I thought about that. But flip buffer is potentially
>>>>>>>>>>>>> allowed to
>>>>>>>>>>>>> throw other exceptions caused by the platform.
>>>>>>>>>>>>> Wouldn't it be excessive to introduce such unspecified
>>>>>>>>>>>>> expectation?
>>>>>>>>>>>>
>>>>>>>>>>>> Actually expectations is clearly specified in
>>>>>>>>>>>> xxx.createBufferStrategy
>>>>>>>>>>>> method. There are only two exceptions AWTException and
>>>>>>>>>>>> IllegalArgumentException. It seems that we cannot get
>>>>>>>>>>>> IllegalArgumentException in the test since numBuffers>1 and
>>>>>>>>>>>> caps!=
>>>>>>>>>>>> null. All other possible exceptions are unspecified and this
>>>>>>>>>>>> is a
>>>>>>>>>>>> bug.
>>>>>>>>>>>> No?
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 9/4/2015 5:01 PM, Sergey Bylokhov wrote:
>>>>>>>>>>>>>> On 04.09.15 15:12, Semyon Sadetsky wrote:
>>>>>>>>>>>>>>> The original bug was about ClastCastException.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Then probably catch AWTException which is only expected from
>>>>>>>>>>>>>> createBufferStrategy?. this will cover old and new bug.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> With the option the test will check nothing if buffering is
>>>>>>>>>>>>>>> not
>>>>>>>>>>>>>>> supported on the test server.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On 9/4/2015 2:40 PM, Sergey Bylokhov wrote:
>>>>>>>>>>>>>>>> Hi, Semyon.
>>>>>>>>>>>>>>>> Is it really necessary to catch ClassCastException? Can you
>>>>>>>>>>>>>>>> try to
>>>>>>>>>>>>>>>> test this functionality via -Dswing.bufferPerWindow. When
>>>>>>>>>>>>>>>> this
>>>>>>>>>>>>>>>> option
>>>>>>>>>>>>>>>> is passed the backbuffer should be created automatically if
>>>>>>>>>>>>>>>> supported.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On 04.09.15 14:03, Semyon Sadetsky wrote:
>>>>>>>>>>>>>>>>> Hello,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Please review fix for JDK9:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8134732
>>>>>>>>>>>>>>>>> webrev:
>>>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~ssadetsky/8134732/webrev.00/
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> It's a test bug: when the flip buffer is not available on
>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>> platform
>>>>>>>>>>>>>>>>> its creation attempt causes exception.
>>>>>>>>>>>>>>>>> Solution: ignore the exception.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> --Semyon
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
>

-- 
Best regards, Sergey.