[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-swing-dev
Subject: Re: <Swing Dev> RFR: 8146330 [macosx] UIDefaults.keys() different size than UIDefaults.keySet()
From: Pankaj Bansal <pankaj.b.bansal () oracle ! com>
Date: 2020-03-13 11:53:05
Message-ID: 9364074c-36d2-4d34-924a-4ae2f4cfeeca () default
[Download RAW message or body]
Looks good to me though the webrev02 was also good enough and easy to read as second \
case is anyway going to fail if first case fails. You can push webrev03 as well.
Regards,
Pankaj
From: Tejpal Rebari
Sent: Friday, March 13, 2020 4:31 PM
To: Jayathirth D v <jayathirth.d.v@oracle.com>
Cc: swing-dev@openjdk.java.net
Subject: Re: <Swing Dev> RFR: 8146330 [macosx] UIDefaults.keys() different size than \
UIDefaults.keySet()
Hi Jay,
On 13-Mar-2020, at 3:55 PM, Jayathirth D v <HYPERLINK \
"mailto:JAYATHIRTH.D.V@ORACLE.COM"JAYATHIRTH.D.V@ORACLE.COM> wrote:
Hi Tejpal,
Test case is not verifying all failure scenarios properly (If both test cases fail \
exception will be thrown only for first test failure)
Make sure that you verify each test case failure and print appropriate message.
Source change looks good to me.
Thanks,
Jay
I have updated the test to properly print the error message according to test cases \
failure.
Updated webrev : http://cr.openjdk.java.net/~trebari/swing/8146330/webrev3/
Thanks
Tejpal
On 13-Mar-2020, at 3:36 PM, Sergey Bylokhov <HYPERLINK \
"mailto:Sergey.Bylokhov@oracle.com"Sergey.Bylokhov@oracle.com> wrote:
Looks fine.
On 3/13/20 2:30 am, Tejpal Rebari wrote:
Hi Sergey,
On 11-Mar-2020, at 5:27 AM, Sergey Bylokhov <HYPERLINK \
"mailto:Sergey.Bylokhov@oracle.com"Sergey.Bylokhov@oracle.com<mailto:Sergey.Bylokhov@oracle.com>> \
wrote:
On 3/10/20 1:04 am, Tejpal Rebari wrote:
I am not getting how to cover this in the test.
I that additional call is necessary, then it should be possible to trigger it by the \
test.
--
Best regards, Sergey.
I have updated the test to check for super.keySet().
Now the test will check for
1. defaults key size returned by the UIManager.getDefaults()
2. key size after writing an additional value to the UIManager.getDefaults()
Verified that the tests fails after the fix of \
http://cr.openjdk.java.net/~trebari/swing/8146330/webrev1/ and passes after adding \
set.addAll(super.keySet()); Updated webrev : \
http://cr.openjdk.java.net/~trebari/swing/8146330/webrev2/ Thanks
Tejpal
--
Best regards, Sergey.
[Attachment #3 (text/html)]
<html xmlns:v="urn:schemas-microsoft-com:vml" \
xmlns:o="urn:schemas-microsoft-com:office:office" \
xmlns:w="urn:schemas-microsoft-com:office:word" \
xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" \
xmlns="http://www.w3.org/TR/REC-html40"><head><meta http-equiv=Content-Type \
content="text/html; charset=us-ascii"><meta name=Generator content="Microsoft Word 15 \
(filtered medium)"><style><!-- /* Font Definitions */
@font-face
{font-family:Helvetica;
panose-1:2 11 6 4 2 2 2 2 2 4;}
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman",serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p.msonormal0, li.msonormal0, div.msonormal0
{mso-style-name:msonormal;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:12.0pt;
font-family:"Times New Roman",serif;}
span.apple-converted-space
{mso-style-name:apple-converted-space;}
span.EmailStyle19
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]--></head><body lang=EN-US link=blue vlink=purple><div \
class=WordSection1><p class=MsoNormal><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Looks good to \
me though the webrev02 was also good enough and easy to read as second case is anyway \
going to fail if first case fails. You can push webrev03 as \
well.<o:p></o:p></span></p><p class=MsoNormal><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'><o:p> </o:p></span></p><p \
class=MsoNormal><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Regards,<o:p></o:p></span></p><p \
class=MsoNormal><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'>Pankaj<o:p></o:p></span></p><p \
class=MsoNormal><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif;color:#1F497D'><o:p> </o:p></span></p><div><div \
style='border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in'><p \
class=MsoNormal><b><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif'>From:</span></b><span \
style='font-size:11.0pt;font-family:"Calibri",sans-serif'> Tejpal Rebari \
<br><b>Sent:</b> Friday, March 13, 2020 4:31 PM<br><b>To:</b> Jayathirth D v \
<jayathirth.d.v@oracle.com><br><b>Cc:</b> \
swing-dev@openjdk.java.net<br><b>Subject:</b> Re: <Swing Dev> RFR: 8146330 \
[macosx] UIDefaults.keys() different size than \
UIDefaults.keySet()<o:p></o:p></span></p></div></div><p \
class=MsoNormal><o:p> </o:p></p><p class=MsoNormal>Hi \
Jay,<o:p></o:p></p><div><div><blockquote \
style='margin-top:5.0pt;margin-bottom:5.0pt'><div><p class=MsoNormal>On 13-Mar-2020, \
at 3:55 PM, Jayathirth D v <<a \
href="mailto:JAYATHIRTH.D.V@ORACLE.COM">JAYATHIRTH.D.V@ORACLE.COM</a>> \
wrote:<o:p></o:p></p></div><p class=MsoNormal><o:p> </o:p></p><div><div><p \
class=MsoNormal>Hi Tejpal,<o:p></o:p></p><div><p \
class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>Test case is not \
verifying all failure scenarios properly (If both test cases fail exception will be \
thrown only for first test failure)<o:p></o:p></p></div><div><p class=MsoNormal>Make \
sure that you verify each test case failure and print appropriate \
message.<o:p></o:p></p></div><div><p \
class=MsoNormal><o:p> </o:p></p></div><div><p class=MsoNormal>Source change \
looks good to me.<o:p></o:p></p></div><div><p \
class=MsoNormal><o:p> </o:p></p></div><div><p \
class=MsoNormal>Thanks,<o:p></o:p></p></div><div><p \
class=MsoNormal>Jay<o:p></o:p></p><div><p \
class=MsoNormal><o:p> </o:p></p></div></div></div></div></blockquote><div><div><p \
class=MsoNormal>I have updated the test to properly print the error message according \
to test cases failure.<o:p></o:p></p></div><div><p class=MsoNormal>Updated webrev \
: <a href="http://cr.openjdk.java.net/~trebari/swing/8146330/webrev3/">http://cr. \
openjdk.java.net/~trebari/swing/8146330/webrev3/</a><o:p></o:p></p></div></div><div><p \
class=MsoNormal><o:p> </o:p></p></div><div><p \
class=MsoNormal>Thanks<o:p></o:p></p></div><div><p \
class=MsoNormal>Tejpal<o:p></o:p></p></div><div><p \
class=MsoNormal><o:p> </o:p></p></div><p \
class=MsoNormal><br><br><o:p></o:p></p><blockquote \
style='margin-top:5.0pt;margin-bottom:5.0pt'><div><div><div><div><blockquote \
style='margin-top:5.0pt;margin-bottom:5.0pt'><div><p class=MsoNormal>On 13-Mar-2020, \
at 3:36 PM, Sergey Bylokhov <<a \
href="mailto:Sergey.Bylokhov@oracle.com">Sergey.Bylokhov@oracle.com</a>> \
wrote:<o:p></o:p></p></div><p class=MsoNormal><o:p> </o:p></p><div><p \
class=MsoNormal><span \
style='font-size:9.0pt;font-family:"Helvetica",sans-serif'>Looks fine.<br><br>On \
3/13/20 2:30 am, Tejpal Rebari wrote:<br style='caret-color: rgb(0, 0, \
0);font-variant-caps: normal;text-align:start;-webkit-text-stroke-width: \
0px;word-spacing:0px'><br></span><o:p></o:p></p><blockquote \
style='margin-top:5.0pt;margin-bottom:5.0pt'><p class=MsoNormal><span \
style='font-size:9.0pt;font-family:"Helvetica",sans-serif'>Hi \
Sergey,<br><br><o:p></o:p></span></p><blockquote \
style='margin-top:5.0pt;margin-bottom:5.0pt'><p class=MsoNormal><span \
style='font-size:9.0pt;font-family:"Helvetica",sans-serif'>On 11-Mar-2020, at 5:27 \
AM, Sergey Bylokhov <<a \
href="mailto:Sergey.Bylokhov@oracle.com">Sergey.Bylokhov@oracle.com</a><<a \
href="mailto:Sergey.Bylokhov@oracle.com">mailto:Sergey.Bylokhov@oracle.com</a>>> \
wrote:<br><br>On 3/10/20 1:04 am, Tejpal Rebari \
wrote:<br><br><o:p></o:p></span></p><blockquote \
style='margin-top:5.0pt;margin-bottom:5.0pt'><p class=MsoNormal><span \
style='font-size:9.0pt;font-family:"Helvetica",sans-serif'>I am not getting how to \
cover this in the test.<o:p></o:p></span></p></blockquote><p class=MsoNormal><span \
style='font-size:9.0pt;font-family:"Helvetica",sans-serif'><br>I that additional call \
is necessary, then it should be possible to trigger it by the test.<br><br>--<span \
class=apple-converted-space> </span><br>Best regards, \
Sergey.<o:p></o:p></span></p></blockquote><p class=MsoNormal><span \
style='font-size:9.0pt;font-family:"Helvetica",sans-serif'>I have updated the test to \
check for super.keySet().<br>Now the test will check for<br>1. defaults key size \
returned by the UIManager.getDefaults()<br>2. key size after writing an \
additional value to the UIManager.getDefaults()<br>Verified that the tests fails \
after the fix of <a href="http://cr.openjdk.java.net/~trebari/swing/8146330/webrev1/">http://cr.openjdk.java.net/~trebari/swing/8146330/webrev1/</a><br>and \
passes after adding set.addAll(super.keySet());<br>Updated webrev : <a \
href="http://cr.openjdk.java.net/~trebari/swing/8146330/webrev2/">http://cr.openjdk.ja \
va.net/~trebari/swing/8146330/webrev2/</a><br>Thanks<br>Tejpal<o:p></o:p></span></p></blockquote><p \
class=MsoNormal><span \
style='font-size:9.0pt;font-family:"Helvetica",sans-serif'><br><br>--<span \
class=apple-converted-space> </span><br>Best regards, \
Sergey.</span><o:p></o:p></p></div></blockquote></div><p \
class=MsoNormal><o:p> </o:p></p></div></div></div></blockquote></div><p \
class=MsoNormal><o:p> </o:p></p></div></div></body></html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic