RFR (smallish?) 8061611: Remove deprecated command line flags
Derek White
derek.white at oracle.com
Mon Dec 15 18:04:21 UTC 2014
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>
More information about the hotspot-gc-dev
mailing list