<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