<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">Thanks Bengt - comments below...<br>
<br>
On 12/15/14 7:51 AM, Bengt Rutisson wrote:<br>
</div>
<blockquote cite="mid:548ED94B.6090506@oracle.com" type="cite">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
Hi Derek,<br>
<br>
<div class="moz-cite-prefix">On 2014-12-12 15:35, Derek White
wrote:<br>
</div>
<blockquote cite="mid:548AFD2A.8060002@oracle.com" type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
<div class="moz-cite-prefix">This is a request for final
re-review.<br>
<br>
Updated for comments, and added regression test for arguments.
This test is a bit overkill for this bug, but it's extensible
for other deprecated and aliased options.<br>
<br>
Re-merged with tree. <br>
<br>
Webrev: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Edrwhite/8061611/webrev.01/">http://cr.openjdk.java.net/~drwhite/8061611/webrev.01/</a><br>
</div>
</blockquote>
<br>
The code changes look good to me.<br>
<br>
A few comments about the test.<br>
<br>
I don't think it is worth testing that the removed flags produce
error messages. Starting a VM takes some time and doing it 13
times just to check that no one by mistake added the flags back
seems like a waste of resources to me. The risk that the flags are
suddenly added back is almost zero, right?<br>
</blockquote>
OK.<br>
<blockquote cite="mid:548ED94B.6090506@oracle.com" type="cite"> I am
also not convinced that a general and extensible test is the way
to go here. I think I would prefer a more specific test for the
flags that were deprecated now. For tests I think it is more
important that the tests are easily readable and understandable
than that code duplication is avoided. Thus, I strongly prefer
small but specific tests that clearly communicates what went wrong
when they fail and are easy to understand how they failed. So, in
this case maybe we should even have two tests?
TestDeprecatedMarkStackFlags and TestDeprecatedConcGCThreadsFlags.<br>
</blockquote>
<br>
OK. I'm trying to calibrate how much testing is appropriate and how
it should be organized. In fact, the other organizing principle was
to be more "process oriented" - one (or more) regression tests per
bug fixed. And regression tests would be essentially immutable -
never expanded to test new cases. Is that about right?<br>
<br>
So to make the tests simpler, but bug specific, how about I break
them up for one test to test aliases, and one test to test
deprecation warnings?<br>
<blockquote cite="mid:548ED94B.6090506@oracle.com" type="cite">
Instead of parsing the PrintFlagsFinal output you could use
ManagementFactoryHelper.getDiagnosticMXBean().getVMOption() to get
the option values and check if they are set correctly. I think
that will reduce the number of lines in the test further.<br>
</blockquote>
<br>
PrintFlagsFinal has the added benefit of noting that a flag was
explicitly set vs. has a default value. (":=" vs "="). I first used
bizzare option values to try to test that, but ":=" is definitive.<br>
<br>
BTW, a motivation for the alias tests was thinking ahead to the redo
of "alias" options that I'm planning. I'll want to test that the new
alias handling code actually works for the remaining aliased
options. But that can get it's own test file.<br>
<br>
Thanks again!<br>
<br>
 - Derek<br>
</body>
</html>