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.
