<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">FYI - <br>
<br>
New webrev shows splitting regression test into simpler alias and
deprecation tests...<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~drwhite/8061611/webrev.03/">http://cr.openjdk.java.net/~drwhite/8061611/webrev.03/</a><br>
<br>
- Derek<br>
<br>
On 12/15/14 1:04 PM, Derek White wrote:<br>
</div>
<blockquote cite="mid:548F22A5.6020308@oracle.com" type="cite">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
<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>
</blockquote>
<br>
</body>
</html>