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

Sergey Bylokhov Sergey.Bylokhov at oracle.com
Wed Mar 25 19:33:24 UTC 2015


The fix looks good to me.

11.03.15 13:13, Semyon Sadetsky wrote:
> Hi Sergey,
>
> Thank you for your response.
> I have added max+-1 cases to the test.
>
> http://cr.openjdk.java.net/~alexsch/semyon-sadetsky/8072769/webrev.05/
>
> cases 0,1, etc are not related to the bug. I understand you worries 
> about test coverage but I think it's better to follow the process.
> You can file a ticket to increase system tray test coverage and we 
> immediately start to work on it upon it is prioritized.
>
> --Semyon
>
>
> On 3/4/2015 11:17 PM, Sergey Bylokhov wrote:
>> 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.
>


-- 
Best regards, Sergey.

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


More information about the awt-dev mailing list