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

[Semyon Sadetsky]

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