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

[Derek White]

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/dbdc4e18/attachment.htm>