On 12/11/17, 11:18 AM, Roger Riggs wrote:
Hi Joe,
The new tests using testng look good, a few comments:
- BAOS/EncodingTest.java
70: The message gets printed if the condition is not true; so
"results do not might" might read better
(also, there is Assert.assertEquals(str1, str2, msg) - that
compares objects with .equals())
A message in nice, though in many cases the testng generated
exception/message is sufficient
especially for equals.
Assert.assertEquals it is! Removed other debug printout.
- PrintStream/EncodingTest.java
92: out should not be null; its probably a test bug; the test
should be removed
Done.
- PrintWriter/EncodingTest.java
93: out should not be null; its probably a test bug; the test
should be removed
Done.
Updated webrev:
http://cr.openjdk.java.net/~joehw/jdk10/8183743/webrev/index.html
Thanks,
Joe
Thanks, Roger
On 12/8/2017 7:29 PM, Joe Wang wrote:
Thanks Roger, Alan for the comments!
I've updated the webrev accordingly. Here's the current webrev:
http://cr.openjdk.java.net/~joehw/jdk10/8183743/webrev/index.html
-Joe
On 12/4/17, 7:52 PM, Joe Wang wrote:
On 12/3/17, 11:34 AM, Alan Bateman wrote:
On 01/12/2017 20:35, Joe Wang wrote:
:
I think URLDecoder.decode methods should have @throws
IllegalArgumentException. I realize it's implementation specific
as to whether IAE is thrown with bad input but given that the RI
does throw IAE then consumers of the API should be prepared for
it. The @implNote can stay and probably should be copied into the
legacy decode method too.
I added @throws IAE. On a 2nd thought, would that give no
flexibility to alternative impls as the general (class) spec had
given? With this addition, it becomes a requirement.
If the @throws description starts with "if the implementation ..."
then it should be clear that it is optional.
Thanks Alan. I made the change accordingly.
http://cr.openjdk.java.net/~joehw/jdk10/8183743/webrev/index.html
-Joe
-Alan