<AWT Dev> [9] review request for 8072769: System tray icon title freezes java

[Sergey Bylokhov]

On 04.03.2015 16:33, Semyon Sadetsky wrote:
> Sergey,
> OK. You can file a request to improve TrayIcon test coverage but it's 
> an another story. Here we are discussing regression test for a 
> specific bug.
> I cannot agree with the approach you've proposed. In your logic we 
> need start to write regression tests for all methods in JDK classes 
> those take String parameters by iterating all possible parameters 
> lengths to discover potential bugs.
Yes, if we know that there is a high probability to crash the method.
> Tests need to be written by certain rules. Scanning all possible 
> values of input parameters is not the best rule in my opinion.
Validation of one number is useless too. Its even does not check all 
corner cases , 0,1,max-1,max,max+1. so the range is better.
>
> --Semyon
>
> On 3/4/2015 4:03 PM, Sergey Bylokhov wrote:
>> On 04.03.2015 15:08, Semyon Sadetsky wrote:
>>> I think it's more like bug regression test policy. Since I'm a 
>>> novice in the project maybe you or somebody can advise me the right 
>>> process.
>>> Lets see what we have here:
>>> The fix is connected to the windows platform native code only. It is 
>>> a platform specific corner case. I would not say it contains "magic 
>>> numbers", those numbers a not magic for the native layer. But in the 
>>> test we call native code from java where in truth string parameters 
>>> maximum lengths are not specified.
>>> I think it make sense to run the test on all other platforms test 
>>> iterating strings lengths up to 1000 or more just to ensure that 
>>> there no similar issues there. We can do this once or have an option 
>>> to switch this scenario on.
>> The goal of the test is to catch all possibly related issues. So 
>> instead of numbers use ranges, cover all platforms if the test don't 
>> use platform's specific classes, cover all look and feels, automatic 
>> test is better than manual, make the test generic. But it has of 
>> course some restrictions like performance and stability. Moreover it 
>> seems that right now displayMessage/setToolTip are used in the manual 
>> tests only.
>>
>>> But I'm not 100% sure that such test scenario should be included in 
>>> the regular regression run because sending messages to the system 
>>> tray is not very fast and the scan can take noticeable amount of 
>>> time. In my opinion such issue seeking scenario is reasonable to run 
>>> time to time or as a part of stress testing profile, but it does not 
>>> make sense to run it during the regular regression because it tests 
>>> nothing specific on platforms other than Windows and even in Windows 
>>> there is only one specific lengths combination. It's not worth to do 
>>> this potential issue seek it's time expensive but will not bring us 
>>> a lot of value really.
>>>
>>> --Semyon
>>>
>>> On 3/4/2015 2:13 PM, Sergey Bylokhov wrote:
>>>> Hi, Semyon.
>>>> I suggest to remove stuff related to windows platform from the 
>>>> test. Also it will be good to test some reasonable range of data 
>>>> instead of magic constant, wrap setTooltip/displayMessage in a loop.
>>>> Please add "tray.remove(trayIcon);" at the end of the test, 
>>>> otherwise it can hang, when will be run w/o jtreg.
>>>>
>>>> On 03.03.2015 15:32, Semyon Sadetsky wrote:
>>>>> accepted.
>>>>> http://cr.openjdk.java.net/~alexsch/semyon-sadetsky/8072769/webrev.03/ 
>>>>>
>>>>>
>>>>>
>>>>> On 3/3/2015 1:16 PM, Alexander Scherbatiy wrote:
>>>>>>
>>>>>>  Just few comments about the test:
>>>>>>   - The test sets WindowsLookAndFeel and seems fails on non 
>>>>>> Windows platforms.
>>>>>>   - There is the second SystemTray.isSupported() check on line 
>>>>>> 49. Does it depends on L&F?
>>>>>>   - The copyright should be updated to 2015.
>>>>>>
>>>>>>  Thanks,
>>>>>>  Alexandr.
>>>>>>
>>>>>> On 3/2/2015 4:01 PM, Semyon Sadetsky wrote:
>>>>>>> Hello,
>>>>>>>
>>>>>>> Test was added. Please review.
>>>>>>> webrev: 
>>>>>>> http://cr.openjdk.java.net/~alexsch/semyon-sadetsky/8072769/webrev.02/ 
>>>>>>>
>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8072769
>>>>>>>
>>>>>>>
>>>>>>> On 2/26/2015 10:46 AM, Semyon Sadetsky wrote:
>>>>>>>> fix updated: 
>>>>>>>> http://cr.openjdk.java.net/~alexsch/semyon-sadetsky/8072769/webrev.01/ 
>>>>>>>>
>>>>>>>>
>>>>>>>> On 2/24/2015 12:12 PM, Semyon Sadetsky wrote:
>>>>>>>>> Hello,
>>>>>>>>> <AWT Dev> [9] review request for 8061636: Fix for JDK-7079254 
>>>>>>>>> changes behavior of MouseListener, MouseMotionListener
>>>>>>>>> please review the 
>>>>>>>>> fix:http://cr.openjdk.java.net/~alexsch/semyon-sadetsky/8072769/webrev/ 
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> for the issue:https://bugs.openjdk.java.net/browse/JDK-8072769
>>>>>>>>>
>>>>>>>>> System tray icon title freezes java
>>>>>>>>>
>>>>>>>>> This fix contains:
>>>>>>>>> fix corner case: <buffer size> == <length of the string>
>>>>>>>>> for two string parameters:
>>>>>>>>> 1. balloon title string
>>>>>>>>> 2. balloon text string
>>>>>>>>>
>>>>>>>>> -- 
>>>>>>>>> Thanks,
>>>>>>>>>
>>>>>>>>> Semyon.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>
>>
>>
>> -- 
>> Best regards, Sergey.
>


-- 
Best regards, Sergey.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20150304/cf5567d5/attachment-0001.html>