RFR (smallish?) 8061611: Remove deprecated command line flags

[Derek White]

FYI -

New webrev shows splitting regression test into simpler alias and 
deprecation tests...

http://cr.openjdk.java.net/~drwhite/8061611/webrev.03/

  - Derek

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?
> OK.
>> 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.
>
> 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 
> warnings?
>> 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...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20141215/96e8718c/attachment.htm>