RFR (smallish?) 8061611: Remove deprecated command line flags
derek.white at oracle.com
Mon Dec 15 22:24:38 UTC 2014
New webrev shows splitting regression test into simpler alias and
On 12/15/14 1:04 PM, Derek White wrote:
> Thanks Bengt - comments below...
> On 12/15/14 7:51 AM, Bengt Rutisson wrote:
>> Hi Derek,
>> On 2014-12-12 15:35, Derek White wrote:
>>> This is a request for final re-review.
>>> 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.
>>> Re-merged with tree.
>>> Webrev: http://cr.openjdk.java.net/~drwhite/8061611/webrev.01/
>> The code changes look good to me.
>> A few comments about the test.
>> 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?
>> 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
> 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?
> 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
>> 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.
> 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.
> 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.
> Thanks again!
> - Derek
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the hotspot-gc-dev